From 40d60c8976dca6a685ad1d1ac53d477bb6144978 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 21 Nov 2025 10:07:47 -0700 Subject: [PATCH 01/36] Add HTTP 413 response when incoming request is too large --- synapse/http/site.py | 55 +++++++++++++++++++++++++++++++++-------- tests/http/test_site.py | 34 +++++++++++++++++++++++++ tests/server.py | 10 ++++++-- 3 files changed, 87 insertions(+), 12 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 03d5d048b11..9ed0dec1c50 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -146,17 +146,52 @@ def __repr__(self) -> str: # Twisted machinery: this method is called by the Channel once the full request has # been received, to dispatch the request to a resource. - # - # We're patching Twisted to bail/abort early when we see someone trying to upload - # `multipart/form-data` so we can avoid Twisted parsing the entire request body into - # in-memory (specific problem of this specific `Content-Type`). This protects us - # from an attacker uploading something bigger than the available RAM and crashing - # the server with a `MemoryError`, or carefully block just enough resources to cause - # all other requests to fail. - # - # FIXME: This can be removed once we Twisted releases a fix and we update to a - # version that is patched def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: + # In the case of a Content-Length header being present, and it's value being too + # large, this will make debugging issues due to overly large requests much + # easier. Currently we handle such cases in `handleContentChunk` and abort the + # connection without providing a proper HTTP response. + # + # Attempting to write an HTTP response from within `handleContentChunk` does not + # work, so the code here has been added to at least provide a response in the + # case of the Content-Length header being present. + content_length_headers = self.requestHeaders.getRawHeaders(b"content-length") + if content_length_headers is not None: + try: + content_length = int(content_length_headers[0]) + if content_length > self._max_request_body_size: + self.method, self.uri = command, path + self.clientproto = version + self.code = HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value + self.code_message = bytes( + HTTPStatus.REQUEST_ENTITY_TOO_LARGE.phrase, "ascii" + ) + self.responseHeaders.setRawHeaders(b"content-length", [b"0"]) + + logger.warning( + "Rejecting request from %s because Content-Length %d exceeds maximum size %d: %s %s", + self.client, + content_length, + self._max_request_body_size, + command.decode("ascii", errors="replace"), + path.decode("ascii", errors="replace"), + ) + self.write(b"") + self.loseConnection() + return + except (ValueError, IndexError): + # Invalid Content-Length header, let it proceed + pass + + # We're patching Twisted to bail/abort early when we see someone trying to upload + # `multipart/form-data` so we can avoid Twisted parsing the entire request body into + # in-memory (specific problem of this specific `Content-Type`). This protects us + # from an attacker uploading something bigger than the available RAM and crashing + # the server with a `MemoryError`, or carefully block just enough resources to cause + # all other requests to fail. + # + # FIXME: This can be removed once we Twisted releases a fix and we update to a + # version that is patched if command == b"POST": ctype = self.requestHeaders.getRawHeaders(b"content-type") if ctype and b"multipart/form-data" in ctype[0]: diff --git a/tests/http/test_site.py b/tests/http/test_site.py index 9e6d929c9ef..04ac7196201 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -143,3 +143,37 @@ def test_content_type_multipart(self) -> None: # we should get a 415 self.assertRegex(transport.value().decode(), r"^HTTP/1\.1 415 ") + + def test_content_length_too_large(self) -> None: + """HTTP requests with Content-Length exceeding max size should be rejected with 413""" + self.hs.start_listening() + + # find the HTTP server which is configured to listen on port 0 + (port, factory, _backlog, interface) = self.reactor.tcpServers[0] + self.assertEqual(interface, "::") + self.assertEqual(port, 0) + + # complete the connection and wire it up to a fake transport + client_address = IPv6Address("TCP", "::1", 2345) + protocol = factory.buildProtocol(client_address) + transport = StringTransport() + protocol.makeConnection(transport) + + # Send a request with Content-Length header that exceeds the limit. + # Default max is 50MB (from media max_upload_size), so send something larger. + oversized_length = 60 * 1024 * 1024 + protocol.dataReceived( + b"POST / HTTP/1.1\r\n" + b"Connection: close\r\n" + b"Content-Length: " + str(oversized_length).encode() + b"\r\n" + b"\r\n" + ) + protocol.dataReceived(b"x" * oversized_length) + + # Advance the reactor to process the request + while not transport.disconnecting: + self.reactor.advance(1) + + # we should get a 413 Payload Too Large + response = transport.value().decode() + self.assertRegex(response, r"^HTTP/1\.1 413 ") diff --git a/tests/server.py b/tests/server.py index 30337f3e38e..b432e05306f 100644 --- a/tests/server.py +++ b/tests/server.py @@ -81,6 +81,7 @@ from twisted.web.resource import IResource from twisted.web.server import Request, Site +from synapse.api.constants import MAX_PDU_SIZE from synapse.config.database import DatabaseConnectionConfig from synapse.config.homeserver import HomeServerConfig from synapse.events.auto_accept_invites import InviteAutoAccepter @@ -241,7 +242,6 @@ def writeSequence(self, data: Iterable[bytes]) -> None: def loseConnection(self) -> None: self.unregisterProducer() - self.transport.loseConnection() # Type ignore: mypy doesn't like the fact that producer isn't an IProducer. def registerProducer(self, producer: IProducer, streaming: bool) -> None: @@ -428,7 +428,13 @@ def make_request( channel = FakeChannel(site, reactor, ip=client_ip) - req = request(channel, site, our_server_name="test_server") + # `max_request_body_size` copied from `synapse/app/_base.py -> max_request_body_size()` + req = request( + channel, + site, + our_server_name="test_server", + max_request_body_size=200 * MAX_PDU_SIZE, + ) channel.request = req req.content = BytesIO(content) From 1e8eaa1d470e524fd062bd2b557ad67240bd4eb7 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 21 Nov 2025 10:15:04 -0700 Subject: [PATCH 02/36] Add changelog entry --- changelog.d/19212.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19212.misc diff --git a/changelog.d/19212.misc b/changelog.d/19212.misc new file mode 100644 index 00000000000..4a5fd4d210d --- /dev/null +++ b/changelog.d/19212.misc @@ -0,0 +1 @@ +Add HTTP 413 response when incoming request is too large. From c3be0742b3e7353e4d3a2eb880a57ccd53cb3e07 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 21 Nov 2025 12:11:24 -0700 Subject: [PATCH 03/36] Log redacted uri --- synapse/http/site.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 9ed0dec1c50..e0a4ab0063b 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -174,7 +174,7 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: content_length, self._max_request_body_size, command.decode("ascii", errors="replace"), - path.decode("ascii", errors="replace"), + self.get_redacted_uri(), ) self.write(b"") self.loseConnection() From 26a2c8f0fa69f4aba21ca80ded15c032338d3338 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 21 Nov 2025 12:12:06 -0700 Subject: [PATCH 04/36] Log redacted uri in content-type check --- synapse/http/site.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index e0a4ab0063b..5868a530989 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -206,8 +206,8 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: logger.warning( "Aborting connection from %s because `content-type: multipart/form-data` is unsupported: %s %s", self.client, - command, - path, + command.decode("ascii", errors="replace"), + self.get_redacted_uri(), ) self.write(b"") self.loseConnection() From 5cf7b962ebaf8a86f3c62dcb3e396b929e13c484 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 26 Nov 2025 13:56:20 -0700 Subject: [PATCH 05/36] Address review comments --- synapse/http/site.py | 59 +++++++++++++++++++++++++++++------------ tests/http/test_site.py | 1 + 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 5868a530989..7778e37c105 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -19,6 +19,7 @@ # # import contextlib +import json import logging import time from http import HTTPStatus @@ -36,6 +37,7 @@ from twisted.web.resource import IResource, Resource from twisted.web.server import Request +from synapse.api.errors import Codes from synapse.config.server import ListenerConfig from synapse.http import get_request_user_agent, redact_uri from synapse.http.proxy import ProxySite @@ -155,31 +157,53 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: # Attempting to write an HTTP response from within `handleContentChunk` does not # work, so the code here has been added to at least provide a response in the # case of the Content-Length header being present. - content_length_headers = self.requestHeaders.getRawHeaders(b"content-length") + content_length_headers = self.requestHeaders.getRawHeaders(b"Content-Length") if content_length_headers is not None: + if len(content_length_headers) != 1: + logger.warning( + "Dropping request from %s because it contains multiple Content-Length headers: %s %s", + self.client, + command.decode("ascii", errors="replace"), + self.get_redacted_uri(), + ) + self.loseConnection() + return + try: content_length = int(content_length_headers[0]) if content_length > self._max_request_body_size: self.method, self.uri = command, path self.clientproto = version - self.code = HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value - self.code_message = bytes( - HTTPStatus.REQUEST_ENTITY_TOO_LARGE.phrase, "ascii" - ) - self.responseHeaders.setRawHeaders(b"content-length", [b"0"]) - logger.warning( "Rejecting request from %s because Content-Length %d exceeds maximum size %d: %s %s", self.client, content_length, self._max_request_body_size, - command.decode("ascii", errors="replace"), + self.get_method(), self.get_redacted_uri(), ) - self.write(b"") + + self.code = HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value + self.code_message = bytes( + HTTPStatus.REQUEST_ENTITY_TOO_LARGE.phrase, "ascii" + ) + + error_response_json = { + "errcode": Codes.TOO_LARGE, + "error": "Request content is too large", + } + error_response_bytes = (json.dumps(error_response_json)).encode() + + self.responseHeaders.setRawHeaders( + b"Content-Type", [b"application/json"] + ) + self.responseHeaders.setRawHeaders( + b"content-length", [f"{len(error_response_bytes)}"] + ) + self.write(error_response_bytes) self.loseConnection() return - except (ValueError, IndexError): + except ValueError: # Invalid Content-Length header, let it proceed pass @@ -197,18 +221,19 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: if ctype and b"multipart/form-data" in ctype[0]: self.method, self.uri = command, path self.clientproto = version - self.code = HTTPStatus.UNSUPPORTED_MEDIA_TYPE.value - self.code_message = bytes( - HTTPStatus.UNSUPPORTED_MEDIA_TYPE.phrase, "ascii" - ) - self.responseHeaders.setRawHeaders(b"content-length", [b"0"]) - logger.warning( "Aborting connection from %s because `content-type: multipart/form-data` is unsupported: %s %s", self.client, - command.decode("ascii", errors="replace"), + self.get_method(), self.get_redacted_uri(), ) + + self.code = HTTPStatus.UNSUPPORTED_MEDIA_TYPE.value + self.code_message = bytes( + HTTPStatus.UNSUPPORTED_MEDIA_TYPE.phrase, "ascii" + ) + + self.responseHeaders.setRawHeaders(b"content-length", [b"0"]) self.write(b"") self.loseConnection() return diff --git a/tests/http/test_site.py b/tests/http/test_site.py index 04ac7196201..a34126eb42e 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -177,3 +177,4 @@ def test_content_length_too_large(self) -> None: # we should get a 413 Payload Too Large response = transport.value().decode() self.assertRegex(response, r"^HTTP/1\.1 413 ") + self.assertSubstring("M_TOO_LARGE", response) From 85d84f5236d9901bb3c04f049f86bd0f1369601a Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 26 Nov 2025 13:59:18 -0700 Subject: [PATCH 06/36] Upade comment wording --- tests/http/test_site.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/http/test_site.py b/tests/http/test_site.py index a34126eb42e..f8824a91477 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -174,7 +174,7 @@ def test_content_length_too_large(self) -> None: while not transport.disconnecting: self.reactor.advance(1) - # we should get a 413 Payload Too Large + # We should get a 413 Content Too Large response = transport.value().decode() self.assertRegex(response, r"^HTTP/1\.1 413 ") self.assertSubstring("M_TOO_LARGE", response) From 9c4536bc3f65e0d091ff2d8aaa6bbb692096b636 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 26 Nov 2025 14:09:24 -0700 Subject: [PATCH 07/36] Rejig max_request_size usage to use common constants everywhere --- synapse/api/constants.py | 13 +++++++++++++ synapse/app/_base.py | 14 ++------------ tests/http/test_site.py | 3 ++- tests/server.py | 4 ++-- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 7a8f546d6bf..92f63110522 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -29,6 +29,19 @@ # the max size of a (canonical-json-encoded) event MAX_PDU_SIZE = 65536 +# The maximum allowed size of an HTTP request. +# Other than media uploads, the biggest request we expect to see is a fully-loaded +# /federation/v1/send request. +# +# The main thing in such a request is up to 50 PDUs, and up to 100 EDUs. PDUs are +# limited to 65536 bytes (possibly slightly more if the sender didn't use canonical +# json encoding); there is no specced limit to EDUs (see +# https://github.com/matrix-org/matrix-doc/issues/3121). +# +# in short, we somewhat arbitrarily limit requests to 200 * 64K (about 12.5M) +# +MAX_REQUEST_SIZE = 200 * MAX_PDU_SIZE + # Max/min size of ints in canonical JSON CANONICALJSON_MAX_INT = (2**53) - 1 CANONICALJSON_MIN_INT = -CANONICALJSON_MAX_INT diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 52bdb9e0d71..6a87590b2be 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -59,7 +59,7 @@ from twisted.web.resource import Resource import synapse.util.caches -from synapse.api.constants import MAX_PDU_SIZE +from synapse.api.constants import MAX_REQUEST_SIZE from synapse.app import check_bind_error from synapse.config import ConfigError from synapse.config._base import format_config_error @@ -843,17 +843,7 @@ def sdnotify(state: bytes) -> None: def max_request_body_size(config: HomeServerConfig) -> int: """Get a suitable maximum size for incoming HTTP requests""" - # Other than media uploads, the biggest request we expect to see is a fully-loaded - # /federation/v1/send request. - # - # The main thing in such a request is up to 50 PDUs, and up to 100 EDUs. PDUs are - # limited to 65536 bytes (possibly slightly more if the sender didn't use canonical - # json encoding); there is no specced limit to EDUs (see - # https://github.com/matrix-org/matrix-doc/issues/3121). - # - # in short, we somewhat arbitrarily limit requests to 200 * 64K (about 12.5M) - # - max_request_size = 200 * MAX_PDU_SIZE + max_request_size = MAX_REQUEST_SIZE # if we have a media repo enabled, we may need to allow larger uploads than that if config.media.can_load_media_repo: diff --git a/tests/http/test_site.py b/tests/http/test_site.py index f8824a91477..1ae6d1ca992 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -22,6 +22,7 @@ from twisted.internet.address import IPv6Address from twisted.internet.testing import MemoryReactor, StringTransport +from synapse.app._base import max_request_body_size from synapse.app.homeserver import SynapseHomeServer from synapse.server import HomeServer from synapse.util.clock import Clock @@ -161,7 +162,7 @@ def test_content_length_too_large(self) -> None: # Send a request with Content-Length header that exceeds the limit. # Default max is 50MB (from media max_upload_size), so send something larger. - oversized_length = 60 * 1024 * 1024 + oversized_length = 1 + max_request_body_size(self.hs.config) protocol.dataReceived( b"POST / HTTP/1.1\r\n" b"Connection: close\r\n" diff --git a/tests/server.py b/tests/server.py index b432e05306f..e27ecc92d99 100644 --- a/tests/server.py +++ b/tests/server.py @@ -81,7 +81,7 @@ from twisted.web.resource import IResource from twisted.web.server import Request, Site -from synapse.api.constants import MAX_PDU_SIZE +from synapse.api.constants import MAX_REQUEST_SIZE from synapse.config.database import DatabaseConnectionConfig from synapse.config.homeserver import HomeServerConfig from synapse.events.auto_accept_invites import InviteAutoAccepter @@ -433,7 +433,7 @@ def make_request( channel, site, our_server_name="test_server", - max_request_body_size=200 * MAX_PDU_SIZE, + max_request_body_size=MAX_REQUEST_SIZE, ) channel.request = req From 5f4a097d1ac1774f3acca4aa5f5687815d9dc355 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 26 Nov 2025 16:22:00 -0700 Subject: [PATCH 08/36] Add ref to upstream twisted changes needed --- synapse/http/site.py | 3 ++- tests/http/test_site.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 7778e37c105..a46bd7e0ec9 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -214,8 +214,9 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: # the server with a `MemoryError`, or carefully block just enough resources to cause # all other requests to fail. # - # FIXME: This can be removed once we Twisted releases a fix and we update to a + # FIXME: This can be removed once Twisted releases a fix and we update to a # version that is patched + # See: https://github.com/element-hq/synapse/security/advisories/GHSA-rfq8-j7rh-8hf2 if command == b"POST": ctype = self.requestHeaders.getRawHeaders(b"content-type") if ctype and b"multipart/form-data" in ctype[0]: diff --git a/tests/http/test_site.py b/tests/http/test_site.py index 1ae6d1ca992..1597afc805e 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -168,8 +168,9 @@ def test_content_length_too_large(self) -> None: b"Connection: close\r\n" b"Content-Length: " + str(oversized_length).encode() + b"\r\n" b"\r\n" + b"" + b"x" * oversized_length + b"\r\n" + b"\r\n" ) - protocol.dataReceived(b"x" * oversized_length) # Advance the reactor to process the request while not transport.disconnecting: From 21b114c9a6131f566055bcd0977e5996c50a643a Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 26 Nov 2025 16:55:04 -0700 Subject: [PATCH 09/36] Don't add duplicate Content-Length header in tests --- tests/rest/client/test_login.py | 6 ------ tests/rest/client/test_media.py | 4 ---- tests/rest/client/utils.py | 1 - 3 files changed, 11 deletions(-) diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py index d599351df78..d83604a6961 100644 --- a/tests/rest/client/test_login.py +++ b/tests/rest/client/test_login.py @@ -1728,9 +1728,6 @@ def test_username_picker_use_displayname_avatar_and_email(self) -> None: content_is_form=True, custom_headers=[ ("Cookie", "username_mapping_session=" + session_id), - # old versions of twisted don't do form-parsing without a valid - # content-length header. - ("Content-Length", str(len(content))), ], ) self.assertEqual(chan.code, 302, chan.result) @@ -1818,9 +1815,6 @@ def test_username_picker_dont_use_displayname_avatar_or_email(self) -> None: content_is_form=True, custom_headers=[ ("Cookie", "username_mapping_session=" + session_id), - # old versions of twisted don't do form-parsing without a valid - # content-length header. - ("Content-Length", str(len(content))), ], ) self.assertEqual(chan.code, 302, chan.result) diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 33172f930ed..ec81b1413c2 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -2590,7 +2590,6 @@ def test_authenticated_media(self) -> None: self.tok, shorthand=False, content_type=b"image/png", - custom_headers=[("Content-Length", str(67))], ) self.assertEqual(channel.code, 200) res = channel.json_body.get("content_uri") @@ -2750,7 +2749,6 @@ def test_authenticated_media_etag(self) -> None: self.tok, shorthand=False, content_type=b"image/png", - custom_headers=[("Content-Length", str(67))], ) self.assertEqual(channel.code, 200) res = channel.json_body.get("content_uri") @@ -2909,7 +2907,6 @@ def upload_media(self, size: int) -> FakeChannel: access_token=self.tok, shorthand=False, content_type=b"text/plain", - custom_headers=[("Content-Length", str(size))], ) def test_upload_under_limit(self) -> None: @@ -3074,7 +3071,6 @@ def upload_media(self, size: int, tok: str) -> FakeChannel: access_token=tok, shorthand=False, content_type=b"text/plain", - custom_headers=[("Content-Length", str(size))], ) def test_upload_under_limit(self) -> None: diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index 613c317b8a6..08e25c0569f 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -621,7 +621,6 @@ def upload_media( path, content=image_data, access_token=tok, - custom_headers=[("Content-Length", str(image_length))], ) assert channel.code == expect_code, "Expected: %d, got: %d, resp: %r" % ( From 426b676f3f06c585f20c8775e87c1c06b0919a5a Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 26 Nov 2025 17:01:29 -0700 Subject: [PATCH 10/36] Remove unused variable --- tests/rest/client/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index 08e25c0569f..b3808d75bb9 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -612,7 +612,6 @@ def upload_media( filename: The filename of the media to be uploaded expect_code: The return code to expect from attempting to upload the media """ - image_length = len(image_data) path = "/_matrix/media/r0/upload?filename=%s" % (filename,) channel = make_request( self.reactor, From 296ed42ed88cfd482d3b27f7084b9aae78c72888 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Tue, 2 Dec 2025 00:11:01 +0000 Subject: [PATCH 11/36] Update synapse/http/site.py Co-authored-by: Eric Eastwood --- synapse/http/site.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index a46bd7e0ec9..def6ee0a535 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -150,7 +150,7 @@ def __repr__(self) -> str: # been received, to dispatch the request to a resource. def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: # In the case of a Content-Length header being present, and it's value being too - # large, this will make debugging issues due to overly large requests much + # large, throw a proper error to make debugging issues due to overly large requests much # easier. Currently we handle such cases in `handleContentChunk` and abort the # connection without providing a proper HTTP response. # From 7e86d3de4b5bdeba42d592f7c62ed49764d3e6fc Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Mon, 1 Dec 2025 16:00:11 -0700 Subject: [PATCH 12/36] Log rejection at info level --- synapse/http/site.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index def6ee0a535..39303dfd059 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -174,7 +174,7 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: if content_length > self._max_request_body_size: self.method, self.uri = command, path self.clientproto = version - logger.warning( + logger.info( "Rejecting request from %s because Content-Length %d exceeds maximum size %d: %s %s", self.client, content_length, From e8d8a3b79ad63797925087ad25591d72efd70b91 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Tue, 2 Dec 2025 08:17:30 -0700 Subject: [PATCH 13/36] Temp changes for incorrect Content-Length --- synapse/http/site.py | 32 ++++++++++++++++++++++++++++++++ tests/http/test_site.py | 6 ++++++ 2 files changed, 38 insertions(+) diff --git a/synapse/http/site.py b/synapse/http/site.py index 39303dfd059..8683b60c867 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -171,6 +171,38 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: try: content_length = int(content_length_headers[0]) + if content_length < self.content.tell(): + self.method, self.uri = command, path + self.clientproto = version + logger.info( + "Rejecting request from %s because Content-Length %d is smaller than the request content size %d: %s %s", + self.client, + content_length, + self.content.tell(), + self.get_method(), + self.get_redacted_uri(), + ) + + self.code = HTTPStatus.BAD_REQUEST.value + self.code_message = bytes( + HTTPStatus.BAD_REQUEST.phrase, "ascii" + ) + + error_response_json = { + "errcode": Codes.UNKNOWN, + "error": "Request content is too small", + } + error_response_bytes = (json.dumps(error_response_json)).encode() + + self.responseHeaders.setRawHeaders( + b"Content-Type", [b"application/json"] + ) + self.responseHeaders.setRawHeaders( + b"content-length", [f"{len(error_response_bytes)}"] + ) + self.write(error_response_bytes) + self.loseConnection() + return if content_length > self._max_request_body_size: self.method, self.uri = command, path self.clientproto = version diff --git a/tests/http/test_site.py b/tests/http/test_site.py index 1597afc805e..bc14d176d9b 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -24,6 +24,7 @@ from synapse.app._base import max_request_body_size from synapse.app.homeserver import SynapseHomeServer +from synapse.rest.client import capabilities, versions from synapse.server import HomeServer from synapse.util.clock import Clock @@ -31,6 +32,11 @@ class SynapseRequestTestCase(HomeserverTestCase): + servlets = [ + versions.register_servlets, + capabilities.register_servlets, + ] + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: return self.setup_test_homeserver(homeserver_to_use=SynapseHomeServer) From 3f9af8d12a993e6be77e5c52e1bac07e257886a7 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 3 Dec 2025 17:15:01 -0700 Subject: [PATCH 14/36] Remove invalid comment --- tests/server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/server.py b/tests/server.py index f9ddbc7ce8b..c33e8c5b873 100644 --- a/tests/server.py +++ b/tests/server.py @@ -428,7 +428,6 @@ def make_request( channel = FakeChannel(site, reactor, ip=client_ip) - # `max_request_body_size` copied from `synapse/app/_base.py -> max_request_body_size()` req = request( channel, site, From 536bf2d49a9ceed7fc4a14db1ecf5810c7e11e5e Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 3 Dec 2025 17:17:26 -0700 Subject: [PATCH 15/36] Address review comments --- synapse/http/site.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 2d9ca2d90c6..319b118bf33 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -158,12 +158,13 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: # work, so the code here has been added to at least provide a response in the # case of the Content-Length header being present. content_length_headers = self.requestHeaders.getRawHeaders(b"Content-Length") + self.method, self.uri = command, path if content_length_headers is not None: if len(content_length_headers) != 1: logger.warning( "Dropping request from %s because it contains multiple Content-Length headers: %s %s", self.client, - command.decode("ascii", errors="replace"), + self.get_method(), self.get_redacted_uri(), ) self.loseConnection() @@ -172,7 +173,6 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: try: content_length = int(content_length_headers[0]) if content_length < self.content.tell(): - self.method, self.uri = command, path self.clientproto = version logger.info( "Rejecting request from %s because Content-Length %d is smaller than the request content size %d: %s %s", @@ -184,9 +184,7 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: ) self.code = HTTPStatus.BAD_REQUEST.value - self.code_message = bytes( - HTTPStatus.BAD_REQUEST.phrase, "ascii" - ) + self.code_message = bytes(HTTPStatus.BAD_REQUEST.phrase, "ascii") error_response_json = { "errcode": Codes.UNKNOWN, @@ -198,7 +196,7 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: b"Content-Type", [b"application/json"] ) self.responseHeaders.setRawHeaders( - b"content-length", [f"{len(error_response_bytes)}"] + b"Content-Length", [f"{len(error_response_bytes)}"] ) self.write(error_response_bytes) self.loseConnection() @@ -230,7 +228,7 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: b"Content-Type", [b"application/json"] ) self.responseHeaders.setRawHeaders( - b"content-length", [f"{len(error_response_bytes)}"] + b"Content-Length", [f"{len(error_response_bytes)}"] ) self.write(error_response_bytes) self.loseConnection() @@ -266,7 +264,7 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: HTTPStatus.UNSUPPORTED_MEDIA_TYPE.phrase, "ascii" ) - self.responseHeaders.setRawHeaders(b"content-length", [b"0"]) + self.responseHeaders.setRawHeaders(b"Content-Length", [b"0"]) self.write(b"") self.loseConnection() return From 8c594c9daae99c6e09d7caa54d5a5a557005b1b6 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 3 Dec 2025 17:18:30 -0700 Subject: [PATCH 16/36] Add more comments --- synapse/app/_base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index edeafff67df..98d051bf04d 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -895,6 +895,7 @@ def sdnotify(state: bytes) -> None: def max_request_body_size(config: HomeServerConfig) -> int: """Get a suitable maximum size for incoming HTTP requests""" + # Baseline default for any request that isn't configured in the homeserver config max_request_size = MAX_REQUEST_SIZE # if we have a media repo enabled, we may need to allow larger uploads than that From 9e2b2a15ab4f461a72e43fe421e057efca0c85ab Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 3 Dec 2025 17:24:13 -0700 Subject: [PATCH 17/36] Add FIXME comment --- synapse/http/site.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/http/site.py b/synapse/http/site.py index 319b118bf33..1a4a6472003 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -264,6 +264,8 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: HTTPStatus.UNSUPPORTED_MEDIA_TYPE.phrase, "ascii" ) + # FIXME: Return a better error response here similar to the + # `error_response_json` returned in other code paths here. self.responseHeaders.setRawHeaders(b"Content-Length", [b"0"]) self.write(b"") self.loseConnection() From e4f21947638d1aa0e08d55455c8292165911e7c0 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Wed, 3 Dec 2025 17:54:53 -0700 Subject: [PATCH 18/36] Refactor content-length extraction --- synapse/http/site.py | 150 +++++++++++++++++++++------------------- tests/http/test_site.py | 71 +++++++++++++++++++ 2 files changed, 151 insertions(+), 70 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 1a4a6472003..75f36955981 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -34,10 +34,11 @@ from twisted.internet.protocol import Protocol from twisted.python.failure import Failure from twisted.web.http import HTTPChannel +from twisted.web.http_headers import Headers from twisted.web.resource import IResource, Resource from twisted.web.server import Request -from synapse.api.errors import Codes +from synapse.api.errors import Codes, SynapseError from synapse.config.server import ListenerConfig from synapse.http import get_request_user_agent, redact_uri from synapse.http.proxy import ProxySite @@ -157,85 +158,96 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: # Attempting to write an HTTP response from within `handleContentChunk` does not # work, so the code here has been added to at least provide a response in the # case of the Content-Length header being present. - content_length_headers = self.requestHeaders.getRawHeaders(b"Content-Length") self.method, self.uri = command, path - if content_length_headers is not None: - if len(content_length_headers) != 1: - logger.warning( - "Dropping request from %s because it contains multiple Content-Length headers: %s %s", + self.clientproto = version + + def get_content_length_from_headers(headers: Headers) -> int | None: + content_length_headers = headers.getRawHeaders(b"Content-Length") + if content_length_headers is not None: + if len(content_length_headers) != 1: + logger.warning( + "Dropping request from %s because it contains multiple Content-Length headers: %s %s", + self.client, + self.get_method(), + self.get_redacted_uri(), + ) + self.loseConnection() + raise SynapseError( + HTTPStatus.BAD_REQUEST, "Too many Content-Length headers." + ) + + try: + content_length = int(content_length_headers[0]) + return content_length + except Exception: + raise SynapseError( + HTTPStatus.BAD_REQUEST, "Content-Length value invalid." + ) + + try: + content_length = get_content_length_from_headers(self.requestHeaders) + except Exception: + return + if content_length is not None: + if content_length < self.content.tell(): + logger.info( + "Rejecting request from %s because Content-Length %d is smaller than the request content size %d: %s %s", self.client, + content_length, + self.content.tell(), self.get_method(), self.get_redacted_uri(), ) - self.loseConnection() - return - try: - content_length = int(content_length_headers[0]) - if content_length < self.content.tell(): - self.clientproto = version - logger.info( - "Rejecting request from %s because Content-Length %d is smaller than the request content size %d: %s %s", - self.client, - content_length, - self.content.tell(), - self.get_method(), - self.get_redacted_uri(), - ) + self.code = HTTPStatus.BAD_REQUEST.value + self.code_message = bytes(HTTPStatus.BAD_REQUEST.phrase, "ascii") - self.code = HTTPStatus.BAD_REQUEST.value - self.code_message = bytes(HTTPStatus.BAD_REQUEST.phrase, "ascii") + error_response_json = { + "errcode": Codes.UNKNOWN, + "error": "Request content is too small", + } + error_response_bytes = (json.dumps(error_response_json)).encode() - error_response_json = { - "errcode": Codes.UNKNOWN, - "error": "Request content is too small", - } - error_response_bytes = (json.dumps(error_response_json)).encode() + self.responseHeaders.setRawHeaders( + b"Content-Type", [b"application/json"] + ) + self.responseHeaders.setRawHeaders( + b"Content-Length", [f"{len(error_response_bytes)}"] + ) + self.write(error_response_bytes) + self.loseConnection() + return - self.responseHeaders.setRawHeaders( - b"Content-Type", [b"application/json"] - ) - self.responseHeaders.setRawHeaders( - b"Content-Length", [f"{len(error_response_bytes)}"] - ) - self.write(error_response_bytes) - self.loseConnection() - return - if content_length > self._max_request_body_size: - self.method, self.uri = command, path - self.clientproto = version - logger.info( - "Rejecting request from %s because Content-Length %d exceeds maximum size %d: %s %s", - self.client, - content_length, - self._max_request_body_size, - self.get_method(), - self.get_redacted_uri(), - ) + if content_length > self._max_request_body_size: + logger.info( + "Rejecting request from %s because Content-Length %d exceeds maximum size %d: %s %s", + self.client, + content_length, + self._max_request_body_size, + self.get_method(), + self.get_redacted_uri(), + ) - self.code = HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value - self.code_message = bytes( - HTTPStatus.REQUEST_ENTITY_TOO_LARGE.phrase, "ascii" - ) + self.code = HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value + self.code_message = bytes( + HTTPStatus.REQUEST_ENTITY_TOO_LARGE.phrase, "ascii" + ) - error_response_json = { - "errcode": Codes.TOO_LARGE, - "error": "Request content is too large", - } - error_response_bytes = (json.dumps(error_response_json)).encode() + error_response_json = { + "errcode": Codes.TOO_LARGE, + "error": "Request content is too large", + } + error_response_bytes = (json.dumps(error_response_json)).encode() - self.responseHeaders.setRawHeaders( - b"Content-Type", [b"application/json"] - ) - self.responseHeaders.setRawHeaders( - b"Content-Length", [f"{len(error_response_bytes)}"] - ) - self.write(error_response_bytes) - self.loseConnection() - return - except ValueError: - # Invalid Content-Length header, let it proceed - pass + self.responseHeaders.setRawHeaders( + b"Content-Type", [b"application/json"] + ) + self.responseHeaders.setRawHeaders( + b"Content-Length", [f"{len(error_response_bytes)}"] + ) + self.write(error_response_bytes) + self.loseConnection() + return # We're patching Twisted to bail/abort early when we see someone trying to upload # `multipart/form-data` so we can avoid Twisted parsing the entire request body into @@ -250,8 +262,6 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: if command == b"POST": ctype = self.requestHeaders.getRawHeaders(b"content-type") if ctype and b"multipart/form-data" in ctype[0]: - self.method, self.uri = command, path - self.clientproto = version logger.warning( "Aborting connection from %s because `content-type: multipart/form-data` is unsupported: %s %s", self.client, diff --git a/tests/http/test_site.py b/tests/http/test_site.py index bc14d176d9b..d4ab2c7bf26 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -186,3 +186,74 @@ def test_content_length_too_large(self) -> None: response = transport.value().decode() self.assertRegex(response, r"^HTTP/1\.1 413 ") self.assertSubstring("M_TOO_LARGE", response) + + def test_too_many_content_length_headers(self) -> None: + """HTTP requests with multiple Content-Length headers should be rejected with 400""" + self.hs.start_listening() + + # find the HTTP server which is configured to listen on port 0 + (port, factory, _backlog, interface) = self.reactor.tcpServers[0] + self.assertEqual(interface, "::") + self.assertEqual(port, 0) + + # complete the connection and wire it up to a fake transport + client_address = IPv6Address("TCP", "::1", 2345) + protocol = factory.buildProtocol(client_address) + transport = StringTransport() + protocol.makeConnection(transport) + + # Send a request with Content-Length header that exceeds the limit. + # Default max is 50MB (from media max_upload_size), so send something larger. + oversized_length = 1 + max_request_body_size(self.hs.config) + protocol.dataReceived( + b"POST / HTTP/1.1\r\n" + b"Connection: close\r\n" + b"Content-Length: " + str(oversized_length).encode() + b"\r\n" + b"Content-Length: " + str(oversized_length).encode() + b"\r\n" + b"\r\n" + b"" + b"x" * oversized_length + b"\r\n" + b"\r\n" + ) + + # Advance the reactor to process the request + while not transport.disconnecting: + self.reactor.advance(1) + + # We should get a 400 + response = transport.value().decode() + self.assertRegex(response, r"^HTTP/1\.1 400 ") + + def test_invalide_content_length_headers(self) -> None: + """HTTP requests with invalid Content-Length header should be rejected with 400""" + self.hs.start_listening() + + # find the HTTP server which is configured to listen on port 0 + (port, factory, _backlog, interface) = self.reactor.tcpServers[0] + self.assertEqual(interface, "::") + self.assertEqual(port, 0) + + # complete the connection and wire it up to a fake transport + client_address = IPv6Address("TCP", "::1", 2345) + protocol = factory.buildProtocol(client_address) + transport = StringTransport() + protocol.makeConnection(transport) + + # Send a request with Content-Length header that exceeds the limit. + # Default max is 50MB (from media max_upload_size), so send something larger. + oversized_length = 1 + max_request_body_size(self.hs.config) + protocol.dataReceived( + b"POST / HTTP/1.1\r\n" + b"Connection: close\r\n" + b"Content-Length: eight\r\n" + b"\r\n" + b"" + b"x" * oversized_length + b"\r\n" + b"\r\n" + ) + + # Advance the reactor to process the request + while not transport.disconnecting: + self.reactor.advance(1) + + # We should get a 400 + response = transport.value().decode() + self.assertRegex(response, r"^HTTP/1\.1 400 ") From c647a0acd7df468c76ac6176d8fc264482b70359 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 08:51:26 -0700 Subject: [PATCH 19/36] Fix exception handling --- synapse/http/site.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 75f36955981..fa57e253f24 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -171,7 +171,6 @@ def get_content_length_from_headers(headers: Headers) -> int | None: self.get_method(), self.get_redacted_uri(), ) - self.loseConnection() raise SynapseError( HTTPStatus.BAD_REQUEST, "Too many Content-Length headers." ) @@ -186,8 +185,9 @@ def get_content_length_from_headers(headers: Headers) -> int | None: try: content_length = get_content_length_from_headers(self.requestHeaders) - except Exception: - return + except Exception as e: + self.loseConnection() + raise e if content_length is not None: if content_length < self.content.tell(): logger.info( From d1b04297b3f356d71b0a4309b08179ecbea7b113 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 08:51:47 -0700 Subject: [PATCH 20/36] Add test for too small Content-Length header --- tests/http/test_site.py | 22 +++++----------------- tests/server.py | 16 +++++++++++----- tests/test_server.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/tests/http/test_site.py b/tests/http/test_site.py index d4ab2c7bf26..654ec3190be 100644 --- a/tests/http/test_site.py +++ b/tests/http/test_site.py @@ -24,7 +24,6 @@ from synapse.app._base import max_request_body_size from synapse.app.homeserver import SynapseHomeServer -from synapse.rest.client import capabilities, versions from synapse.server import HomeServer from synapse.util.clock import Clock @@ -32,11 +31,6 @@ class SynapseRequestTestCase(HomeserverTestCase): - servlets = [ - versions.register_servlets, - capabilities.register_servlets, - ] - def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: return self.setup_test_homeserver(homeserver_to_use=SynapseHomeServer) @@ -202,16 +196,13 @@ def test_too_many_content_length_headers(self) -> None: transport = StringTransport() protocol.makeConnection(transport) - # Send a request with Content-Length header that exceeds the limit. - # Default max is 50MB (from media max_upload_size), so send something larger. - oversized_length = 1 + max_request_body_size(self.hs.config) protocol.dataReceived( b"POST / HTTP/1.1\r\n" b"Connection: close\r\n" - b"Content-Length: " + str(oversized_length).encode() + b"\r\n" - b"Content-Length: " + str(oversized_length).encode() + b"\r\n" + b"Content-Length: " + str(5).encode() + b"\r\n" + b"Content-Length: " + str(5).encode() + b"\r\n" b"\r\n" - b"" + b"x" * oversized_length + b"\r\n" + b"" + b"xxxxx" + b"\r\n" b"\r\n" ) @@ -223,7 +214,7 @@ def test_too_many_content_length_headers(self) -> None: response = transport.value().decode() self.assertRegex(response, r"^HTTP/1\.1 400 ") - def test_invalide_content_length_headers(self) -> None: + def test_invalid_content_length_headers(self) -> None: """HTTP requests with invalid Content-Length header should be rejected with 400""" self.hs.start_listening() @@ -238,15 +229,12 @@ def test_invalide_content_length_headers(self) -> None: transport = StringTransport() protocol.makeConnection(transport) - # Send a request with Content-Length header that exceeds the limit. - # Default max is 50MB (from media max_upload_size), so send something larger. - oversized_length = 1 + max_request_body_size(self.hs.config) protocol.dataReceived( b"POST / HTTP/1.1\r\n" b"Connection: close\r\n" b"Content-Length: eight\r\n" b"\r\n" - b"" + b"x" * oversized_length + b"\r\n" + b"" + b"xxxxx" + b"\r\n" b"\r\n" ) diff --git a/tests/server.py b/tests/server.py index c33e8c5b873..baffd73d0a2 100644 --- a/tests/server.py +++ b/tests/server.py @@ -440,11 +440,17 @@ def make_request( # Twisted expects to be at the end of the content when parsing the request. req.content.seek(0, SEEK_END) - # Old version of Twisted (<20.3.0) have issues with parsing x-www-form-urlencoded - # bodies if the Content-Length header is missing - req.requestHeaders.addRawHeader( - b"Content-Length", str(len(content)).encode("ascii") - ) + # If `Content-Length` was passed in as a custom header, don't automatically add it + # here. + if custom_headers is not None and not any( + (k if isinstance(k, bytes) else k.encode("ascii")) == b"Content-Length" + for k, _ in custom_headers + ): + # Old version of Twisted (<20.3.0) have issues with parsing x-www-form-urlencoded + # bodies if the Content-Length header is missing + req.requestHeaders.addRawHeader( + b"Content-Length", str(len(content)).encode("ascii") + ) if access_token: req.requestHeaders.addRawHeader( diff --git a/tests/test_server.py b/tests/test_server.py index ec31b6cc5f6..4e90c177273 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -212,6 +212,34 @@ def _callback( self.assertEqual(channel.code, 200) self.assertNotIn("body", channel.result) + def test_content_too_large(self) -> None: + """ + HTTP requests with content size exceeding Content-Length should be rejected with 400. + """ + + def _callback(request: SynapseRequest, **kwargs: object) -> tuple[int, JsonDict]: + return 200, {} + + res = JsonResource(self.homeserver) + res.register_paths( + "POST", [re.compile("^/_matrix/foo$")], _callback, "test_servlet" + ) + + channel = make_request( + self.reactor, + FakeSite(res, self.reactor), + b"POST", + b"/_matrix/foo", + {}, + # Set the `Content-Length` value to be smaller than the actual content size + custom_headers=[("Content-Length", "1")], + # The request should disconnect early so don't await the result + await_result=False, + ) + + self.reactor.advance(0.1) + self.assertEqual(channel.code, 400) + class OptionsResourceTests(unittest.TestCase): def setUp(self) -> None: From 7648b3b8823349354282aad5646a7a7392ddcfcb Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 08:55:55 -0700 Subject: [PATCH 21/36] Fix linter errors --- tests/test_server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_server.py b/tests/test_server.py index 4e90c177273..9509a15aca9 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -217,7 +217,9 @@ def test_content_too_large(self) -> None: HTTP requests with content size exceeding Content-Length should be rejected with 400. """ - def _callback(request: SynapseRequest, **kwargs: object) -> tuple[int, JsonDict]: + def _callback( + request: SynapseRequest, **kwargs: object + ) -> tuple[int, JsonDict]: return 200, {} res = JsonResource(self.homeserver) From 72c3ab3a6d30e8e2fccee24b5bd75dc80e8f3f34 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 08:59:34 -0700 Subject: [PATCH 22/36] Add comment about why we reject multiple content-length headers --- synapse/http/site.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/http/site.py b/synapse/http/site.py index fa57e253f24..5d6b3bcdf2c 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -164,6 +164,10 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: def get_content_length_from_headers(headers: Headers) -> int | None: content_length_headers = headers.getRawHeaders(b"Content-Length") if content_length_headers is not None: + # If there are multiple `Content-Length` headers return an error. + # We don't want to even try to pick the right one if there are multiple + # as we could run into problems similar to request smuggling vulnerabilities + # which rely on the mismatch of how different systems interpret information. if len(content_length_headers) != 1: logger.warning( "Dropping request from %s because it contains multiple Content-Length headers: %s %s", From c2b8d2ca1bd0a0d31226aadc180b376997c362fc Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 09:08:20 -0700 Subject: [PATCH 23/36] Fix mypy errors --- synapse/http/site.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 5d6b3bcdf2c..818ee249c17 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -186,13 +186,14 @@ def get_content_length_from_headers(headers: Headers) -> int | None: raise SynapseError( HTTPStatus.BAD_REQUEST, "Content-Length value invalid." ) + return None try: content_length = get_content_length_from_headers(self.requestHeaders) except Exception as e: self.loseConnection() raise e - if content_length is not None: + if content_length is not None and self.content is not None: if content_length < self.content.tell(): logger.info( "Rejecting request from %s because Content-Length %d is smaller than the request content size %d: %s %s", From 5ab4736f0eb7bb394a97908a6e5bf4d14cb12de5 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 09:33:58 -0700 Subject: [PATCH 24/36] Fix test content-length header addition logic --- tests/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/server.py b/tests/server.py index baffd73d0a2..ce31a4162ad 100644 --- a/tests/server.py +++ b/tests/server.py @@ -442,7 +442,7 @@ def make_request( # If `Content-Length` was passed in as a custom header, don't automatically add it # here. - if custom_headers is not None and not any( + if custom_headers is None or not any( (k if isinstance(k, bytes) else k.encode("ascii")) == b"Content-Length" for k, _ in custom_headers ): From 7b0b9b2e3b7a85ab4e0a61abb9fd1547a3550516 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 12:29:32 -0700 Subject: [PATCH 25/36] Refactor error response writing to be reusable --- synapse/http/site.py | 59 +++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 818ee249c17..ce48cdc3018 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -38,7 +38,7 @@ from twisted.web.resource import IResource, Resource from twisted.web.server import Request -from synapse.api.errors import Codes, SynapseError +from synapse.api.errors import Codes from synapse.config.server import ListenerConfig from synapse.http import get_request_user_agent, redact_uri from synapse.http.proxy import ProxySite @@ -49,7 +49,7 @@ PreserveLoggingContext, ) from synapse.metrics import SERVER_NAME_LABEL -from synapse.types import ISynapseReactor, Requester +from synapse.types import ISynapseReactor, JsonDict, Requester if TYPE_CHECKING: import opentracing @@ -175,24 +175,37 @@ def get_content_length_from_headers(headers: Headers) -> int | None: self.get_method(), self.get_redacted_uri(), ) - raise SynapseError( - HTTPStatus.BAD_REQUEST, "Too many Content-Length headers." - ) + raise Exception("Too many Content-Length headers.") try: content_length = int(content_length_headers[0]) return content_length except Exception: - raise SynapseError( - HTTPStatus.BAD_REQUEST, "Content-Length value invalid." - ) + raise Exception("Content-Length value invalid.") return None + def respond_with_error(error_code: HTTPStatus, error_json: JsonDict): + self.code = error_code.value + self.code_message = bytes(error_code.phrase, "ascii") + error_response_bytes = (json.dumps(error_json)).encode() + + self.responseHeaders.setRawHeaders(b"Content-Type", [b"application/json"]) + self.responseHeaders.setRawHeaders( + b"Content-Length", [f"{len(error_response_bytes)}"] + ) + self.write(error_response_bytes) + try: content_length = get_content_length_from_headers(self.requestHeaders) except Exception as e: + error_response_json = { + "errcode": Codes.UNKNOWN, + "error": f"{str(e)}", + } + respond_with_error(HTTPStatus.BAD_REQUEST, error_response_json) self.loseConnection() - raise e + return + if content_length is not None and self.content is not None: if content_length < self.content.tell(): logger.info( @@ -204,22 +217,11 @@ def get_content_length_from_headers(headers: Headers) -> int | None: self.get_redacted_uri(), ) - self.code = HTTPStatus.BAD_REQUEST.value - self.code_message = bytes(HTTPStatus.BAD_REQUEST.phrase, "ascii") - error_response_json = { "errcode": Codes.UNKNOWN, "error": "Request content is too small", } - error_response_bytes = (json.dumps(error_response_json)).encode() - - self.responseHeaders.setRawHeaders( - b"Content-Type", [b"application/json"] - ) - self.responseHeaders.setRawHeaders( - b"Content-Length", [f"{len(error_response_bytes)}"] - ) - self.write(error_response_bytes) + respond_with_error(HTTPStatus.BAD_REQUEST, error_response_json) self.loseConnection() return @@ -233,24 +235,13 @@ def get_content_length_from_headers(headers: Headers) -> int | None: self.get_redacted_uri(), ) - self.code = HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value - self.code_message = bytes( - HTTPStatus.REQUEST_ENTITY_TOO_LARGE.phrase, "ascii" - ) - error_response_json = { "errcode": Codes.TOO_LARGE, "error": "Request content is too large", } - error_response_bytes = (json.dumps(error_response_json)).encode() - - self.responseHeaders.setRawHeaders( - b"Content-Type", [b"application/json"] - ) - self.responseHeaders.setRawHeaders( - b"Content-Length", [f"{len(error_response_bytes)}"] + respond_with_error( + HTTPStatus.REQUEST_ENTITY_TOO_LARGE, error_response_json ) - self.write(error_response_bytes) self.loseConnection() return From b2c219e69324fd9a408e5d42a11a75b9ba156408 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 12:43:50 -0700 Subject: [PATCH 26/36] Add return type --- synapse/http/site.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index ce48cdc3018..c92ded07a40 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -184,7 +184,7 @@ def get_content_length_from_headers(headers: Headers) -> int | None: raise Exception("Content-Length value invalid.") return None - def respond_with_error(error_code: HTTPStatus, error_json: JsonDict): + def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: self.code = error_code.value self.code_message = bytes(error_code.phrase, "ascii") error_response_bytes = (json.dumps(error_json)).encode() From 2a2cd4b2cd9e6d348d4ffbd7dac2da131cb27304 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 15:25:03 -0700 Subject: [PATCH 27/36] Move log --- synapse/http/site.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index c92ded07a40..3e2e6325db4 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -169,12 +169,6 @@ def get_content_length_from_headers(headers: Headers) -> int | None: # as we could run into problems similar to request smuggling vulnerabilities # which rely on the mismatch of how different systems interpret information. if len(content_length_headers) != 1: - logger.warning( - "Dropping request from %s because it contains multiple Content-Length headers: %s %s", - self.client, - self.get_method(), - self.get_redacted_uri(), - ) raise Exception("Too many Content-Length headers.") try: @@ -198,6 +192,12 @@ def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: try: content_length = get_content_length_from_headers(self.requestHeaders) except Exception as e: + logger.warning( + f"Rejecting request from %s because: {str(e)}", + self.client, + self.get_method(), + self.get_redacted_uri(), + ) error_response_json = { "errcode": Codes.UNKNOWN, "error": f"{str(e)}", From 485283cefa92c76fcd0a2f82c76210e1af733139 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 15:32:35 -0700 Subject: [PATCH 28/36] Update log --- synapse/http/site.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 3e2e6325db4..4a69dfafaf7 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -193,8 +193,9 @@ def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: content_length = get_content_length_from_headers(self.requestHeaders) except Exception as e: logger.warning( - f"Rejecting request from %s because: {str(e)}", + "Rejecting request from %s because: %s %s %s", self.client, + str(e), self.get_method(), self.get_redacted_uri(), ) From b0954372bd8f23ed898108b03beb5f8bc1182a91 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 16:40:09 -0700 Subject: [PATCH 29/36] Add changelog entry --- changelog.d/19212.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/19212.misc b/changelog.d/19212.misc index 4a5fd4d210d..83158ce2d9c 100644 --- a/changelog.d/19212.misc +++ b/changelog.d/19212.misc @@ -1 +1 @@ -Add HTTP 413 response when incoming request is too large. +Respond with useful error codes with `Content-Length` header/s are invalid. From b5b464364ef07dd8ade1fbdb12734d5973a504bc Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Thu, 4 Dec 2025 17:01:51 -0700 Subject: [PATCH 30/36] Refactor extracted function to be class member --- synapse/http/site.py | 46 ++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 4a69dfafaf7..b3c831dedcf 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -34,7 +34,6 @@ from twisted.internet.protocol import Protocol from twisted.python.failure import Failure from twisted.web.http import HTTPChannel -from twisted.web.http_headers import Headers from twisted.web.resource import IResource, Resource from twisted.web.server import Request @@ -147,6 +146,32 @@ def __repr__(self) -> str: self.synapse_site.site_tag, ) + def _get_content_length_from_headers(self) -> int | None: + """Attempts to obtain the `Content-Length` value from the request's headers. + + Returns: + Content length as `int` if present. Otherwise `None`. + + Raises: + `Exception` if multiple `Content-Length` headers are present or the value + is not an `int`. + """ + content_length_headers = self.requestHeaders.getRawHeaders(b"Content-Length") + if content_length_headers is not None: + # If there are multiple `Content-Length` headers return an error. + # We don't want to even try to pick the right one if there are multiple + # as we could run into problems similar to request smuggling vulnerabilities + # which rely on the mismatch of how different systems interpret information. + if len(content_length_headers) != 1: + raise Exception("Too many Content-Length headers.") + + try: + content_length = int(content_length_headers[0]) + return content_length + except Exception: + raise Exception("Content-Length value invalid.") + return None + # Twisted machinery: this method is called by the Channel once the full request has # been received, to dispatch the request to a resource. def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: @@ -161,23 +186,6 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: self.method, self.uri = command, path self.clientproto = version - def get_content_length_from_headers(headers: Headers) -> int | None: - content_length_headers = headers.getRawHeaders(b"Content-Length") - if content_length_headers is not None: - # If there are multiple `Content-Length` headers return an error. - # We don't want to even try to pick the right one if there are multiple - # as we could run into problems similar to request smuggling vulnerabilities - # which rely on the mismatch of how different systems interpret information. - if len(content_length_headers) != 1: - raise Exception("Too many Content-Length headers.") - - try: - content_length = int(content_length_headers[0]) - return content_length - except Exception: - raise Exception("Content-Length value invalid.") - return None - def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: self.code = error_code.value self.code_message = bytes(error_code.phrase, "ascii") @@ -190,7 +198,7 @@ def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: self.write(error_response_bytes) try: - content_length = get_content_length_from_headers(self.requestHeaders) + content_length = self._get_content_length_from_headers() except Exception as e: logger.warning( "Rejecting request from %s because: %s %s %s", From e7fe25f11cad891c5a2f85202faa1059ea9f423d Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 5 Dec 2025 20:53:31 +0000 Subject: [PATCH 31/36] Update synapse/http/site.py Co-authored-by: Eric Eastwood --- synapse/http/site.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index b3c831dedcf..006d1078ee1 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -201,7 +201,7 @@ def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: content_length = self._get_content_length_from_headers() except Exception as e: logger.warning( - "Rejecting request from %s because: %s %s %s", + "Rejecting request from %s because: %s - %s %s", self.client, str(e), self.get_method(), From bea38d47fc7b90992c56d849895aaf684d8f07d0 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 5 Dec 2025 20:53:54 +0000 Subject: [PATCH 32/36] Update tests/test_server.py Co-authored-by: Eric Eastwood --- tests/test_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_server.py b/tests/test_server.py index 9509a15aca9..34baf6d6f85 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -212,7 +212,7 @@ def _callback( self.assertEqual(channel.code, 200) self.assertNotIn("body", channel.result) - def test_content_too_large(self) -> None: + def test_content_larger_than_content_length(self) -> None: """ HTTP requests with content size exceeding Content-Length should be rejected with 400. """ From e07fd8ba08d4cb7e4c7a56425fd27e57a725bd06 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 5 Dec 2025 13:57:34 -0700 Subject: [PATCH 33/36] Better error messages --- synapse/http/site.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 006d1078ee1..4598c8984d3 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -209,26 +209,27 @@ def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: ) error_response_json = { "errcode": Codes.UNKNOWN, - "error": f"{str(e)}", + "error": f"Rejecting request: {str(e)}", } respond_with_error(HTTPStatus.BAD_REQUEST, error_response_json) self.loseConnection() return if content_length is not None and self.content is not None: - if content_length < self.content.tell(): + actual_content_length = self.content.tell() + if content_length < actual_content_length: logger.info( "Rejecting request from %s because Content-Length %d is smaller than the request content size %d: %s %s", self.client, content_length, - self.content.tell(), + actual_content_length, self.get_method(), self.get_redacted_uri(), ) error_response_json = { "errcode": Codes.UNKNOWN, - "error": "Request content is too small", + "error": f"Rejecting request as the Content-Length header value {content_length} is smaller than the actual request content size {actual_content_length}", } respond_with_error(HTTPStatus.BAD_REQUEST, error_response_json) self.loseConnection() @@ -246,7 +247,7 @@ def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: error_response_json = { "errcode": Codes.TOO_LARGE, - "error": "Request content is too large", + "error": f"Request content is too large (>{self._max_request_body_size})", } respond_with_error( HTTPStatus.REQUEST_ENTITY_TOO_LARGE, error_response_json From 93f58e6eed8a5e79e060c2b16f52fe5a79081b6a Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 5 Dec 2025 15:28:45 -0700 Subject: [PATCH 34/36] Add validation for content_length too large --- synapse/http/site.py | 182 ++++++++++++++++++++++++------------------- tests/test_server.py | 30 +++++++ 2 files changed, 131 insertions(+), 81 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index 4598c8984d3..f10380a9121 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -61,6 +61,16 @@ _next_request_seq = 0 +class ContentLengthError(Exception): + """Raised when content-length validation fails.""" + + def __init__(self, status: HTTPStatus, errcode: str, message: str): + self.status = status + self.errcode = errcode + self.message = message + super().__init__(message) + + class SynapseRequest(Request): """Class which encapsulates an HTTP request to synapse. @@ -146,6 +156,19 @@ def __repr__(self) -> str: self.synapse_site.site_tag, ) + def _respond_with_error(self, error_code: HTTPStatus, error_json: JsonDict) -> None: + """Send an error response and close the connection.""" + self.code = error_code.value + self.code_message = bytes(error_code.phrase, "ascii") + error_response_bytes = json.dumps(error_json).encode() + + self.responseHeaders.setRawHeaders(b"Content-Type", [b"application/json"]) + self.responseHeaders.setRawHeaders( + b"Content-Length", [f"{len(error_response_bytes)}"] + ) + self.write(error_response_bytes) + self.loseConnection() + def _get_content_length_from_headers(self) -> int | None: """Attempts to obtain the `Content-Length` value from the request's headers. @@ -153,24 +176,82 @@ def _get_content_length_from_headers(self) -> int | None: Content length as `int` if present. Otherwise `None`. Raises: - `Exception` if multiple `Content-Length` headers are present or the value - is not an `int`. + ContentLengthError: if multiple `Content-Length` headers are present or the + value is not an `int`. """ content_length_headers = self.requestHeaders.getRawHeaders(b"Content-Length") - if content_length_headers is not None: - # If there are multiple `Content-Length` headers return an error. - # We don't want to even try to pick the right one if there are multiple - # as we could run into problems similar to request smuggling vulnerabilities - # which rely on the mismatch of how different systems interpret information. - if len(content_length_headers) != 1: - raise Exception("Too many Content-Length headers.") - - try: - content_length = int(content_length_headers[0]) - return content_length - except Exception: - raise Exception("Content-Length value invalid.") - return None + if content_length_headers is None: + return None + + # If there are multiple `Content-Length` headers return an error. + # We don't want to even try to pick the right one if there are multiple + # as we could run into problems similar to request smuggling vulnerabilities + # which rely on the mismatch of how different systems interpret information. + if len(content_length_headers) != 1: + raise ContentLengthError( + HTTPStatus.BAD_REQUEST, + Codes.UNKNOWN, + "Multiple Content-Length headers received", + ) + + try: + return int(content_length_headers[0]) + except (ValueError, TypeError): + raise ContentLengthError( + HTTPStatus.BAD_REQUEST, + Codes.UNKNOWN, + "Content-Length header value is not a valid integer", + ) + + def _validate_content_length(self) -> None: + """Validate Content-Length header and actual content size. + + Raises: + ContentLengthError: If validation fails. + """ + # we should have a `content` by now. + assert self.content, "_validate_content_length() called before gotLength()" + content_length = self._get_content_length_from_headers() + + if content_length is None: + return + + actual_content_length = self.content.tell() + + if content_length > self._max_request_body_size: + logger.info( + "Rejecting request from %s because Content-Length %d exceeds maximum size %d: %s %s", + self.client, + content_length, + self._max_request_body_size, + self.get_method(), + self.get_redacted_uri(), + ) + raise ContentLengthError( + HTTPStatus.REQUEST_ENTITY_TOO_LARGE, + Codes.TOO_LARGE, + f"Request content is too large (>{self._max_request_body_size})", + ) + + if content_length != actual_content_length: + comparison = ( + "smaller" if content_length < actual_content_length else "larger" + ) + logger.info( + "Rejecting request from %s because Content-Length %d is %s than the request content size %d: %s %s", + self.client, + content_length, + comparison, + actual_content_length, + self.get_method(), + self.get_redacted_uri(), + ) + raise ContentLengthError( + HTTPStatus.BAD_REQUEST, + Codes.UNKNOWN, + f"Rejecting request as the Content-Length header value {content_length} " + f"is {comparison} than the actual request content size {actual_content_length}", + ) # Twisted machinery: this method is called by the Channel once the full request has # been received, to dispatch the request to a resource. @@ -186,75 +267,14 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: self.method, self.uri = command, path self.clientproto = version - def respond_with_error(error_code: HTTPStatus, error_json: JsonDict) -> None: - self.code = error_code.value - self.code_message = bytes(error_code.phrase, "ascii") - error_response_bytes = (json.dumps(error_json)).encode() - - self.responseHeaders.setRawHeaders(b"Content-Type", [b"application/json"]) - self.responseHeaders.setRawHeaders( - b"Content-Length", [f"{len(error_response_bytes)}"] - ) - self.write(error_response_bytes) - try: - content_length = self._get_content_length_from_headers() - except Exception as e: - logger.warning( - "Rejecting request from %s because: %s - %s %s", - self.client, - str(e), - self.get_method(), - self.get_redacted_uri(), + self._validate_content_length() + except ContentLengthError as e: + self._respond_with_error( + e.status, {"errcode": e.errcode, "error": e.message} ) - error_response_json = { - "errcode": Codes.UNKNOWN, - "error": f"Rejecting request: {str(e)}", - } - respond_with_error(HTTPStatus.BAD_REQUEST, error_response_json) - self.loseConnection() return - if content_length is not None and self.content is not None: - actual_content_length = self.content.tell() - if content_length < actual_content_length: - logger.info( - "Rejecting request from %s because Content-Length %d is smaller than the request content size %d: %s %s", - self.client, - content_length, - actual_content_length, - self.get_method(), - self.get_redacted_uri(), - ) - - error_response_json = { - "errcode": Codes.UNKNOWN, - "error": f"Rejecting request as the Content-Length header value {content_length} is smaller than the actual request content size {actual_content_length}", - } - respond_with_error(HTTPStatus.BAD_REQUEST, error_response_json) - self.loseConnection() - return - - if content_length > self._max_request_body_size: - logger.info( - "Rejecting request from %s because Content-Length %d exceeds maximum size %d: %s %s", - self.client, - content_length, - self._max_request_body_size, - self.get_method(), - self.get_redacted_uri(), - ) - - error_response_json = { - "errcode": Codes.TOO_LARGE, - "error": f"Request content is too large (>{self._max_request_body_size})", - } - respond_with_error( - HTTPStatus.REQUEST_ENTITY_TOO_LARGE, error_response_json - ) - self.loseConnection() - return - # We're patching Twisted to bail/abort early when we see someone trying to upload # `multipart/form-data` so we can avoid Twisted parsing the entire request body into # in-memory (specific problem of this specific `Content-Type`). This protects us diff --git a/tests/test_server.py b/tests/test_server.py index 34baf6d6f85..3e3915b19ae 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -242,6 +242,36 @@ def _callback( self.reactor.advance(0.1) self.assertEqual(channel.code, 400) + def test_content_length_larger_than_content(self) -> None: + """ + HTTP requests with content size smaller than Content-Length should be rejected with 400. + """ + + def _callback( + request: SynapseRequest, **kwargs: object + ) -> tuple[int, JsonDict]: + return 200, {} + + res = JsonResource(self.homeserver) + res.register_paths( + "POST", [re.compile("^/_matrix/foo$")], _callback, "test_servlet" + ) + + channel = make_request( + self.reactor, + FakeSite(res, self.reactor), + b"POST", + b"/_matrix/foo", + {}, + # Set the `Content-Length` value to be larger than the actual content size + custom_headers=[("Content-Length", "10")], + # The request should disconnect early so don't await the result + await_result=False, + ) + + self.reactor.advance(0.1) + self.assertEqual(channel.code, 400) + class OptionsResourceTests(unittest.TestCase): def setUp(self) -> None: From 525bdbd9db9ac2ec7ecb5136051503d92ac5c87f Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Mon, 8 Dec 2025 20:44:35 +0000 Subject: [PATCH 35/36] Update tests/test_server.py Co-authored-by: Eric Eastwood --- tests/test_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_server.py b/tests/test_server.py index 3e3915b19ae..2a36dd4b304 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -242,7 +242,7 @@ def _callback( self.reactor.advance(0.1) self.assertEqual(channel.code, 400) - def test_content_length_larger_than_content(self) -> None: + def test_content_smaller_than_content_length(self) -> None: """ HTTP requests with content size smaller than Content-Length should be rejected with 400. """ From adb43593f9cb3d867d6278e22c15ab8f2417e66f Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Mon, 8 Dec 2025 14:04:03 -0700 Subject: [PATCH 36/36] Use SynapseError type --- synapse/http/site.py | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/synapse/http/site.py b/synapse/http/site.py index f10380a9121..6ced5b98b3c 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -37,7 +37,7 @@ from twisted.web.resource import IResource, Resource from twisted.web.server import Request -from synapse.api.errors import Codes +from synapse.api.errors import Codes, SynapseError from synapse.config.server import ListenerConfig from synapse.http import get_request_user_agent, redact_uri from synapse.http.proxy import ProxySite @@ -48,7 +48,7 @@ PreserveLoggingContext, ) from synapse.metrics import SERVER_NAME_LABEL -from synapse.types import ISynapseReactor, JsonDict, Requester +from synapse.types import ISynapseReactor, Requester if TYPE_CHECKING: import opentracing @@ -61,15 +61,9 @@ _next_request_seq = 0 -class ContentLengthError(Exception): +class ContentLengthError(SynapseError): """Raised when content-length validation fails.""" - def __init__(self, status: HTTPStatus, errcode: str, message: str): - self.status = status - self.errcode = errcode - self.message = message - super().__init__(message) - class SynapseRequest(Request): """Class which encapsulates an HTTP request to synapse. @@ -156,11 +150,10 @@ def __repr__(self) -> str: self.synapse_site.site_tag, ) - def _respond_with_error(self, error_code: HTTPStatus, error_json: JsonDict) -> None: + def _respond_with_error(self, synapse_error: SynapseError) -> None: """Send an error response and close the connection.""" - self.code = error_code.value - self.code_message = bytes(error_code.phrase, "ascii") - error_response_bytes = json.dumps(error_json).encode() + self.setResponseCode(synapse_error.code) + error_response_bytes = json.dumps(synapse_error.error_dict(None)).encode() self.responseHeaders.setRawHeaders(b"Content-Type", [b"application/json"]) self.responseHeaders.setRawHeaders( @@ -190,8 +183,8 @@ def _get_content_length_from_headers(self) -> int | None: if len(content_length_headers) != 1: raise ContentLengthError( HTTPStatus.BAD_REQUEST, - Codes.UNKNOWN, "Multiple Content-Length headers received", + Codes.UNKNOWN, ) try: @@ -199,8 +192,8 @@ def _get_content_length_from_headers(self) -> int | None: except (ValueError, TypeError): raise ContentLengthError( HTTPStatus.BAD_REQUEST, - Codes.UNKNOWN, "Content-Length header value is not a valid integer", + Codes.UNKNOWN, ) def _validate_content_length(self) -> None: @@ -229,8 +222,8 @@ def _validate_content_length(self) -> None: ) raise ContentLengthError( HTTPStatus.REQUEST_ENTITY_TOO_LARGE, - Codes.TOO_LARGE, f"Request content is too large (>{self._max_request_body_size})", + Codes.TOO_LARGE, ) if content_length != actual_content_length: @@ -248,9 +241,9 @@ def _validate_content_length(self) -> None: ) raise ContentLengthError( HTTPStatus.BAD_REQUEST, - Codes.UNKNOWN, f"Rejecting request as the Content-Length header value {content_length} " f"is {comparison} than the actual request content size {actual_content_length}", + Codes.UNKNOWN, ) # Twisted machinery: this method is called by the Channel once the full request has @@ -270,9 +263,7 @@ def requestReceived(self, command: bytes, path: bytes, version: bytes) -> None: try: self._validate_content_length() except ContentLengthError as e: - self._respond_with_error( - e.status, {"errcode": e.errcode, "error": e.message} - ) + self._respond_with_error(e) return # We're patching Twisted to bail/abort early when we see someone trying to upload