Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Return an empty body for OPTIONS requests. #7886

Merged
merged 2 commits into from
Jul 24, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7886.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Return an empty body for OPTIONS requests.
25 changes: 6 additions & 19 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I think we should remove this. If clients can cache OPTIONS responses, then they should.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Less bytes then too! 🎉


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":
Expand Down
8 changes: 4 additions & 4 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
clokep marked this conversation as resolved.
Show resolved Hide resolved
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(
Expand All @@ -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(
Expand Down