From 3616b40c781c1b820cb6b27b7bcf49fcbad42c8e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 17 Jul 2020 11:34:56 -0400 Subject: [PATCH 1/2] Return an empty body for OPTIONS requests. --- changelog.d/7886.misc | 1 + synapse/http/server.py | 25 ++++++------------------- tests/test_server.py | 8 ++++---- 3 files changed, 11 insertions(+), 23 deletions(-) create mode 100644 changelog.d/7886.misc diff --git a/changelog.d/7886.misc b/changelog.d/7886.misc new file mode 100644 index 000000000000..e729ab24511b --- /dev/null +++ b/changelog.d/7886.misc @@ -0,0 +1 @@ +Return an empty body for OPTIONS requests. diff --git a/synapse/http/server.py b/synapse/http/server.py index cff49202f460..3dd8215c5eef 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -442,21 +442,6 @@ def render_GET(self, request: Request): return super().render_GET(request) -def _options_handler(request): - """Request handler for OPTIONS requests - - This is a request handler suitable for return from - _get_handler_for_request. It returns a 200 and an empty body. - - Args: - request (twisted.web.http.Request): - - Returns: - Tuple[int, dict]: http code, response body. - """ - return 200, {} - - def _unrecognised_request_handler(request): """Request handler for unrecognised requests @@ -490,11 +475,13 @@ class OptionsResource(resource.Resource): """Responds to OPTION requests for itself and all children.""" def render_OPTIONS(self, request): - code, response_json_object = _options_handler(request) + request.setResponseCode(204) + request.setHeader(b"Content-Length", b"0") + request.setHeader(b"Cache-Control", b"no-cache, no-store, must-revalidate") - return respond_with_json( - request, code, response_json_object, send_cors=True, canonical_json=False, - ) + set_cors_headers(request) + + return b"" def getChildWithDefault(self, path, request): if request.method == b"OPTIONS": diff --git a/tests/test_server.py b/tests/test_server.py index 030f58cbdc14..82db34b004cf 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -201,8 +201,8 @@ def _make_request(self, method, path): def test_unknown_options_request(self): """An OPTIONS requests to an unknown URL still returns 200 OK.""" channel = self._make_request(b"OPTIONS", b"/foo/") - self.assertEqual(channel.result["code"], b"200") - self.assertEqual(channel.result["body"], b"{}") + self.assertEqual(channel.result["code"], b"204") + self.assertNotIn("body", channel.result) # Ensure the correct CORS headers have been added self.assertTrue( @@ -221,8 +221,8 @@ def test_unknown_options_request(self): def test_known_options_request(self): """An OPTIONS requests to an known URL still returns 200 OK.""" channel = self._make_request(b"OPTIONS", b"/res/") - self.assertEqual(channel.result["code"], b"200") - self.assertEqual(channel.result["body"], b"{}") + self.assertEqual(channel.result["code"], b"204") + self.assertNotIn("body", channel.result) # Ensure the correct CORS headers have been added self.assertTrue( From 4d23b7fc9d276b2a2e2d3d5fffe68a7eeb4c23bb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 17 Jul 2020 13:28:36 -0400 Subject: [PATCH 2/2] Review comments. --- synapse/http/server.py | 1 - tests/test_server.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/synapse/http/server.py b/synapse/http/server.py index 3dd8215c5eef..49e5726a3fd3 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -477,7 +477,6 @@ class OptionsResource(resource.Resource): def render_OPTIONS(self, request): request.setResponseCode(204) request.setHeader(b"Content-Length", b"0") - request.setHeader(b"Cache-Control", b"no-cache, no-store, must-revalidate") set_cors_headers(request) diff --git a/tests/test_server.py b/tests/test_server.py index 82db34b004cf..78b33f4cd60e 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -199,7 +199,7 @@ def _make_request(self, method, path): return channel def test_unknown_options_request(self): - """An OPTIONS requests to an unknown URL still returns 200 OK.""" + """An OPTIONS requests to an unknown URL still returns 204 No Content.""" channel = self._make_request(b"OPTIONS", b"/foo/") self.assertEqual(channel.result["code"], b"204") self.assertNotIn("body", channel.result) @@ -219,7 +219,7 @@ def test_unknown_options_request(self): ) def test_known_options_request(self): - """An OPTIONS requests to an known URL still returns 200 OK.""" + """An OPTIONS requests to an known URL still returns 204 No Content.""" channel = self._make_request(b"OPTIONS", b"/res/") self.assertEqual(channel.result["code"], b"204") self.assertNotIn("body", channel.result)