From dc1c24064de18ea7f87db4913dbe15ac94a69e97 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 8 Aug 2024 13:01:01 -0700 Subject: [PATCH 1/5] handle lower-cased http headers --- synapse/http/client.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 56ad28eabf3..910e0adacb1 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -1057,11 +1057,17 @@ def dataReceived(self, incoming_data: bytes) -> None: if not self.parser: def on_header_field(data: bytes, start: int, end: int) -> None: - if data[start:end] == b"Location": + if data[start:end] == b"Location" or data[start:end] == b"location": self.has_redirect = True - if data[start:end] == b"Content-Disposition": + if ( + data[start:end] == b"Content-Disposition" + or data[start:end] == b"content-disposition" + ): self.in_disposition = True - if data[start:end] == b"Content-Type": + if ( + data[start:end] == b"Content-Type" + or data[start:end] == b"content-type" + ): self.in_content_type = True def on_header_value(data: bytes, start: int, end: int) -> None: From 3f167d5fcd096edce7cd63743d784e75d436d08d Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 8 Aug 2024 13:01:07 -0700 Subject: [PATCH 2/5] test --- tests/http/test_client.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/http/test_client.py b/tests/http/test_client.py index 721917f957c..6447635f817 100644 --- a/tests/http/test_client.py +++ b/tests/http/test_client.py @@ -51,6 +51,7 @@ class ReadMultipartResponseTests(TestCase): data1 = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: text/plain\r\nContent-Disposition: inline; filename=test_upload\r\n\r\nfile_" data2 = b"to_stream\r\n--6067d4698f8d40a0a794ea7d7379d53a--\r\n\r\n" + data3 = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\ncontent-type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\ncontent-type: text/plain\r\ncontent-disposition: inline; filename=test_upload\r\n\r\nfile_" redirect_data = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nLocation: https://cdn.example.org/ab/c1/2345.txt\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a--\r\n\r\n" @@ -118,6 +119,29 @@ def test_parse_file(self) -> None: multipart_response.disposition, b"inline; filename=test_upload" ) + def test_parse_file_lowercase_headers(self) -> None: + """ + Check that a multipart response containing a file is properly parsed + into the json/file parts, and the json and file are properly captured if the http headers are lowercased + """ + result, deferred, protocol = self._build_multipart_response(249, 250) + + # Start sending data. + protocol.dataReceived(self.data3) + protocol.dataReceived(self.data2) + # Close the connection. + protocol.connectionLost(Failure(ResponseDone())) + + multipart_response: MultipartResponse = deferred.result # type: ignore[assignment] + + self.assertEqual(multipart_response.json, b"{}") + self.assertEqual(result.getvalue(), b"file_to_stream") + self.assertEqual(multipart_response.length, len(b"file_to_stream")) + self.assertEqual(multipart_response.content_type, b"text/plain") + self.assertEqual( + multipart_response.disposition, b"inline; filename=test_upload" + ) + def test_parse_redirect(self) -> None: """ check that a multipart response containing a redirect is properly parsed and redirect url is From 02e5752d83c6bbcce3d7d924125c8e457809e567 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 8 Aug 2024 13:05:19 -0700 Subject: [PATCH 3/5] newsfragment --- changelog.d/17545.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17545.bugfix diff --git a/changelog.d/17545.bugfix b/changelog.d/17545.bugfix new file mode 100644 index 00000000000..9e9e10f3cba --- /dev/null +++ b/changelog.d/17545.bugfix @@ -0,0 +1 @@ +Handle lower-case http headers in _Mulitpart_Parser_Protocol. \ No newline at end of file From d1fe7f2b2006b780efb68c74871ad90d4149a184 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 8 Aug 2024 16:35:36 -0700 Subject: [PATCH 4/5] handle all case possibilities --- synapse/http/client.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 910e0adacb1..391102fe6a0 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -1057,17 +1057,11 @@ def dataReceived(self, incoming_data: bytes) -> None: if not self.parser: def on_header_field(data: bytes, start: int, end: int) -> None: - if data[start:end] == b"Location" or data[start:end] == b"location": + if data[start:end].lower() == b"location": self.has_redirect = True - if ( - data[start:end] == b"Content-Disposition" - or data[start:end] == b"content-disposition" - ): + if data[start:end].lower() == b"content-disposition": self.in_disposition = True - if ( - data[start:end] == b"Content-Type" - or data[start:end] == b"content-type" - ): + if data[start:end].lower() == b"content-type": self.in_content_type = True def on_header_value(data: bytes, start: int, end: int) -> None: From c65afefcf091f903dd6f5175c05464f59f7b41c4 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 13 Aug 2024 10:27:55 -0700 Subject: [PATCH 5/5] update tests --- changelog.d/17545.bugfix | 2 +- tests/http/test_client.py | 24 +++++++++++++----------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/changelog.d/17545.bugfix b/changelog.d/17545.bugfix index 9e9e10f3cba..31e22d873e9 100644 --- a/changelog.d/17545.bugfix +++ b/changelog.d/17545.bugfix @@ -1 +1 @@ -Handle lower-case http headers in _Mulitpart_Parser_Protocol. \ No newline at end of file +Handle lower-case http headers in `_Mulitpart_Parser_Protocol`. \ No newline at end of file diff --git a/tests/http/test_client.py b/tests/http/test_client.py index 6447635f817..f2abec190bd 100644 --- a/tests/http/test_client.py +++ b/tests/http/test_client.py @@ -49,9 +49,11 @@ class ReadMultipartResponseTests(TestCase): - data1 = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: text/plain\r\nContent-Disposition: inline; filename=test_upload\r\n\r\nfile_" - data2 = b"to_stream\r\n--6067d4698f8d40a0a794ea7d7379d53a--\r\n\r\n" - data3 = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\ncontent-type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\ncontent-type: text/plain\r\ncontent-disposition: inline; filename=test_upload\r\n\r\nfile_" + multipart_response_data1 = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: text/plain\r\nContent-Disposition: inline; filename=test_upload\r\n\r\nfile_" + multipart_response_data2 = ( + b"to_stream\r\n--6067d4698f8d40a0a794ea7d7379d53a--\r\n\r\n" + ) + multipart_response_data_cased = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\ncOntEnt-type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-tyPe: text/plain\r\nconTent-dispOsition: inline; filename=test_upload\r\n\r\nfile_" redirect_data = b"\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nContent-Type: application/json\r\n\r\n{}\r\n--6067d4698f8d40a0a794ea7d7379d53a\r\nLocation: https://cdn.example.org/ab/c1/2345.txt\r\n\r\n--6067d4698f8d40a0a794ea7d7379d53a--\r\n\r\n" @@ -104,8 +106,8 @@ def test_parse_file(self) -> None: result, deferred, protocol = self._build_multipart_response(249, 250) # Start sending data. - protocol.dataReceived(self.data1) - protocol.dataReceived(self.data2) + protocol.dataReceived(self.multipart_response_data1) + protocol.dataReceived(self.multipart_response_data2) # Close the connection. protocol.connectionLost(Failure(ResponseDone())) @@ -127,8 +129,8 @@ def test_parse_file_lowercase_headers(self) -> None: result, deferred, protocol = self._build_multipart_response(249, 250) # Start sending data. - protocol.dataReceived(self.data3) - protocol.dataReceived(self.data2) + protocol.dataReceived(self.multipart_response_data_cased) + protocol.dataReceived(self.multipart_response_data2) # Close the connection. protocol.connectionLost(Failure(ResponseDone())) @@ -167,7 +169,7 @@ def test_too_large(self) -> None: result, deferred, protocol = self._build_multipart_response(UNKNOWN_LENGTH, 180) # Start sending data. - protocol.dataReceived(self.data1) + protocol.dataReceived(self.multipart_response_data1) self.assertEqual(result.getvalue(), b"file_") self._assert_error(deferred, protocol) @@ -178,11 +180,11 @@ def test_additional_data(self) -> None: result, deferred, protocol = self._build_multipart_response(UNKNOWN_LENGTH, 180) # Start sending data. - protocol.dataReceived(self.data1) + protocol.dataReceived(self.multipart_response_data1) self._assert_error(deferred, protocol) # More data might have come in. - protocol.dataReceived(self.data2) + protocol.dataReceived(self.multipart_response_data2) self.assertEqual(result.getvalue(), b"file_") self._assert_error(deferred, protocol) @@ -196,7 +198,7 @@ def test_content_length(self) -> None: self.assertFalse(deferred.called) # Start sending data. - protocol.dataReceived(self.data1) + protocol.dataReceived(self.multipart_response_data1) self._assert_error(deferred, protocol) self._cleanup_error(deferred)