8b0083cad9
Related to https://github.com/element-hq/synapse/issues/17035, when Synapse receives a request that is larger than the maximum size allowed, it aborts the connection without ever sending back a HTTP response. I dug into our usage of twisted and how best to try and report such an error and this is what I came up with. It would be ideal to be able to report the status from within `handleContentChunk` but that is called too early on in the twisted http handling code, before things have been setup enough to be able to properly write a response. I tested this change out locally (both with C-S and S-S apis) and they do receive a 413 response now in addition to the connection being closed. Hopefully this will aid in being able to quickly detect when https://github.com/element-hq/synapse/issues/17035 is occurring as the current situation makes it very hard to narrow things down to that specific issue without making a lot of assumptions. This PR also responds with more meaningful error codes now in the case of: - multiple `Content-Length` headers - invalid `Content-Length` header value - request content size being larger than the `Content-Length` value ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [X] Pull request is based on the develop branch * [X] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [X] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Eric Eastwood <erice@element.io>
248 lines
8.9 KiB
Python
248 lines
8.9 KiB
Python
#
|
|
# This file is licensed under the Affero General Public License (AGPL) version 3.
|
|
#
|
|
# Copyright 2021 The Matrix.org Foundation C.I.C.
|
|
# Copyright (C) 2023 New Vector, Ltd
|
|
#
|
|
# This program is free software: you can redistribute it and/or modify
|
|
# it under the terms of the GNU Affero General Public License as
|
|
# published by the Free Software Foundation, either version 3 of the
|
|
# License, or (at your option) any later version.
|
|
#
|
|
# See the GNU Affero General Public License for more details:
|
|
# <https://www.gnu.org/licenses/agpl-3.0.html>.
|
|
#
|
|
# Originally licensed under the Apache License, Version 2.0:
|
|
# <http://www.apache.org/licenses/LICENSE-2.0>.
|
|
#
|
|
# [This file includes modifications made by New Vector Limited]
|
|
#
|
|
#
|
|
|
|
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
|
|
|
|
from tests.unittest import HomeserverTestCase
|
|
|
|
|
|
class SynapseRequestTestCase(HomeserverTestCase):
|
|
def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
|
|
return self.setup_test_homeserver(homeserver_to_use=SynapseHomeServer)
|
|
|
|
def test_large_request(self) -> None:
|
|
"""overlarge HTTP requests should be rejected"""
|
|
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)
|
|
|
|
# as a control case, first send a regular request.
|
|
|
|
# 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)
|
|
|
|
protocol.dataReceived(
|
|
b"POST / HTTP/1.1\r\n"
|
|
b"Connection: close\r\n"
|
|
b"Transfer-Encoding: chunked\r\n"
|
|
b"\r\n"
|
|
b"0\r\n"
|
|
b"\r\n"
|
|
)
|
|
|
|
while not transport.disconnecting:
|
|
self.reactor.advance(1)
|
|
|
|
# we should get a 404
|
|
self.assertRegex(transport.value().decode(), r"^HTTP/1\.1 404 ")
|
|
|
|
# now send an oversized request
|
|
protocol = factory.buildProtocol(client_address)
|
|
transport = StringTransport()
|
|
protocol.makeConnection(transport)
|
|
|
|
protocol.dataReceived(
|
|
b"POST / HTTP/1.1\r\n"
|
|
b"Connection: close\r\n"
|
|
b"Transfer-Encoding: chunked\r\n"
|
|
b"\r\n"
|
|
)
|
|
|
|
# we deliberately send all the data in one big chunk, to ensure that
|
|
# twisted isn't buffering the data in the chunked transfer decoder.
|
|
# we start with the chunk size, in hex. (We won't actually send this much)
|
|
protocol.dataReceived(b"10000000\r\n")
|
|
sent = 0
|
|
while not transport.disconnected:
|
|
self.assertLess(sent, 0x10000000, "connection did not drop")
|
|
protocol.dataReceived(b"\0" * 1024)
|
|
sent += 1024
|
|
|
|
# default max upload size is 50M, so it should drop on the next buffer after
|
|
# that.
|
|
self.assertEqual(sent, 50 * 1024 * 1024 + 1024)
|
|
|
|
def test_content_type_multipart(self) -> None:
|
|
"""HTTP POST requests with `content-type: multipart/form-data` should be rejected"""
|
|
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)
|
|
|
|
# as a control case, first send a regular request.
|
|
|
|
# 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)
|
|
|
|
protocol.dataReceived(
|
|
b"POST / HTTP/1.1\r\n"
|
|
b"Connection: close\r\n"
|
|
b"Transfer-Encoding: chunked\r\n"
|
|
b"\r\n"
|
|
b"0\r\n"
|
|
b"\r\n"
|
|
)
|
|
|
|
while not transport.disconnecting:
|
|
self.reactor.advance(1)
|
|
|
|
# we should get a 404
|
|
self.assertRegex(transport.value().decode(), r"^HTTP/1\.1 404 ")
|
|
|
|
# now send request with content-type header
|
|
protocol = factory.buildProtocol(client_address)
|
|
transport = StringTransport()
|
|
protocol.makeConnection(transport)
|
|
|
|
protocol.dataReceived(
|
|
b"POST / HTTP/1.1\r\n"
|
|
b"Connection: close\r\n"
|
|
b"Transfer-Encoding: chunked\r\n"
|
|
b"Content-Type: multipart/form-data\r\n"
|
|
b"\r\n"
|
|
b"0\r\n"
|
|
b"\r\n"
|
|
)
|
|
|
|
while not transport.disconnecting:
|
|
self.reactor.advance(1)
|
|
|
|
# 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 = 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"\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 413 Content Too Large
|
|
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)
|
|
|
|
protocol.dataReceived(
|
|
b"POST / HTTP/1.1\r\n"
|
|
b"Connection: close\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"xxxxx" + 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_invalid_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)
|
|
|
|
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"xxxxx" + 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 ")
|