nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: tornadoweb/tornado
Pull request #3412 (2024-07-08 19:36:16)

web: do not send 500 on invalid If-Modified-Since header
Fixing https://github.com/tornadoweb/tornado/issues/3408

bdarnell: Looks good, but it could use a simple test in web_test.py. 
rgajrawala: @bdarnell done!
rgajrawala: @bdarnell @mcg1969 Can we please merge this if it's good? We're getting pinged for 500s every time our security scan runs and unfortunately we can't override `StaticFileHandler` since tornado is not a direct dependency.
bdarnell: Looks like the catch should be `except Exception`, since there no strong guarantees about what type of error you might see here.

I can merge this once the tests are passing but I'm not planning another release for a while (months) so you'll probably want to make your own custom build and put it in a private package index in the meantime. 
rgajrawala: Updated per your suggestion.

> I'm not planning another release for a while

Even for a patch? I'll try to look into private package indexes...
bdarnell: > Even for a patch? 

I can do patches for severe bugs, but this doesn't seem like it reaches that level since it concerns the handling of deliberately-invalid requests (and the failure is a spurious 500 response instead of doing work that shouldn't be done).

> I'll try to look into private package indexes...

Some folks like to use tools like `devpi` for this but I prefer to keep it simple and use `PIP_FIND_LINKS` as in this ancient example:  https://github.com/viewfinderco/viewfinder/blob/master/scripts/update-environment

Patch:
diff --git a/tornado/web.py b/tornado/web.py
index 93ce3501..357c5f1a 100644
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -2861,7 +2861,10 @@ class StaticFileHandler(RequestHandler):
         # content has not been modified
         ims_value = self.request.headers.get("If-Modified-Since")
         if ims_value is not None:
-            if_since = email.utils.parsedate_to_datetime(ims_value)
+            try:
+                if_since = email.utils.parsedate_to_datetime(ims_value)
+            except Exception:
+                return False
             if if_since.tzinfo is None:
                 if_since = if_since.replace(tzinfo=datetime.timezone.utc)
             assert self.modified is not None


Test patch:
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index 32a12bad..42c66171 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -1230,6 +1230,13 @@ class StaticFileTest(WebTestCase):
         )
         self.assertEqual(response2.code, 200)
 
+    def test_static_304_if_modified_since_invalid(self):
+        response = self.get_and_head(
+            "/static/robots.txt",
+            headers={"If-Modified-Since": "!nv@l!d"},
+        )
+        self.assertEqual(response.code, 200)
+
     def test_static_if_modified_since_pre_epoch(self):
         # On windows, the functions that work with time_t do not accept
         # negative values, and at least one client (processing.js) seems
Repository: tornadoweb/tornado
Pull request #3451 (2024-12-18 18:38:14)

Allow horizontal tabs in header values
Addresses https://github.com/tornadoweb/tornado/issues/3450


Patch:
diff --git a/tornado/web.py b/tornado/web.py
index 357c5f1a..a0949572 100644
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -399,7 +399,8 @@ class RequestHandler:
         if name in self._headers:
             del self._headers[name]
 
-    _INVALID_HEADER_CHAR_RE = re.compile(r"[\x00-\x1f]")
+    # https://www.rfc-editor.org/rfc/rfc9110#name-field-values
+    _VALID_HEADER_CHARS = re.compile(r"[\x09\x20-\x7e\x80-\xff]*")
 
     def _convert_header_value(self, value: _HeaderTypes) -> str:
         # Convert the input value to a str. This type check is a bit
@@ -421,7 +422,7 @@ class RequestHandler:
             raise TypeError("Unsupported header value %r" % value)
         # If \n is allowed into the header, it is possible to inject
         # additional headers or split the request.
-        if RequestHandler._INVALID_HEADER_CHAR_RE.search(retval):
+        if RequestHandler._VALID_HEADER_CHARS.fullmatch(retval) is None:
             raise ValueError("Unsafe header value %r", retval)
         return retval
 


Test patch:
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index a829657a..45bf211b 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -706,6 +706,26 @@ class HeaderInjectionHandler(RequestHandler):
                 raise
 
 
+class SetHeaderHandler(RequestHandler):
+    def get(self):
+        # tests the validity of web.RequestHandler._VALID_HEADER_CHARS
+        illegal_chars = [chr(o) for o in range(0, 0x20)]
+        illegal_chars.append(chr(0x7f))
+        illegal_chars.remove('\t')
+        for char in illegal_chars:
+            try:
+                self.set_header("X-Foo", "foo" + char + "bar")
+                raise Exception("Didn't get expected exception")
+            except ValueError as e:
+                if "Unsafe header value" not in str(e):
+                    raise
+
+        # an empty header value is valid as well
+        self.set_header("X-Foo", "")
+
+        self.finish(b"ok")
+
+
 class GetArgumentHandler(RequestHandler):
     def prepare(self):
         if self.get_argument("source", None) == "query":
@@ -790,6 +810,7 @@ class WSGISafeWebTest(WebTestCase):
             url("/header_injection", HeaderInjectionHandler),
             url("/get_argument", GetArgumentHandler),
             url("/get_arguments", GetArgumentsHandler),
+            url("/set_header", SetHeaderHandler),
         ]
         return urls
 
@@ -938,6 +959,10 @@ js_embed()
         response = self.fetch("/header_injection")
         self.assertEqual(response.body, b"ok")
 
+    def test_set_header(self):
+        response = self.fetch("/set_header")
+        self.assertEqual(response.body, b"ok")
+
     def test_get_argument(self):
         response = self.fetch("/get_argument?foo=bar")
         self.assertEqual(response.body, b"bar")