[issue36274] http.client cannot send non-ASCII request lines

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan <tir.karthi at gmail.com> added the comment:

Is there a reason why cherrypy doesn't URL encode the null byte in this test as per comment : https://github.com/cherrypy/cherrypy/issues/1781#issuecomment-504476658 ?

The original fix was adopted from golang (https://github.com/golang/go/commit/829c5df58694b3345cb5ea41206783c8ccf5c3ca) and as per the cherrypy test the golang client also should be forbidding invalid control character : https://play.golang.org/p/FTNtRmvlWrn

Also as per the discussion one of our own stdlib tests were depending on this behavior in the http client and had to be changed : https://github.com/python/cpython/pull/12755#issuecomment-481617878

The change made in Issue30458 also affects urllib3 test in 3.7.4 causing CI failure and the related discussion to encode URLs : https://github.com/urllib3/urllib3/pull/1671#discussion_r318804155 . urllib3 issue : https://github.com/urllib3/urllib3/issues/1664 . 

IIRC urllib3 was also patched for this vulnerability in similar manner as part of larger refactor : https://github.com/urllib3/urllib3/issues/1553 . I didn't verify yet, it's unreleased and how urllib3 client behaves before and after patch for the CRLF characters and similar tests that use urllib3 as client.

Applying the URL encoding patch to cherrypy I can verify that the behavior is also tested

diff --git a/cherrypy/test/test_static.py b/cherrypy/test/test_static.py
index cdd821ae..85a51cf3 100644
--- a/cherrypy/test/test_static.py
+++ b/cherrypy/test/test_static.py
@@ -398,7 +398,8 @@ class StaticTest(helper.CPWebCase):
         self.assertInBody("I couldn't find that thing")

     def test_null_bytes(self):
-        self.getPage('/static/\x00')
+        from urllib.parse import quote
+        self.getPage(quote('/static/\x00'))
         self.assertStatus('404 Not Found')

     @classmethod


After patch the test passes and ValueError is also raised properly as the null byte is decoded in the server and using it in os.stat to resolve null byte path.

Breakages were expected in the discussion as adopting the fix from golang. Giving an option like a parameter to bypass the validation was also discussed in the linked but giving an option would also mean it could be abused or missed.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue36274>
_______________________________________