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

Implement handling of HEAD requests #7999

Merged
merged 8 commits into from
Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
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/7999.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long standing bug where HEAD requests were not properly rendered.
clokep marked this conversation as resolved.
Show resolved Hide resolved
16 changes: 11 additions & 5 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,12 @@ async def _async_render(self, request: Request):
no appropriate method exists. Can be overriden in sub classes for
different routing.
"""
# Treat HEAD requests as GET requests.
Copy link
Member

Choose a reason for hiding this comment

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

well, ideally we'd only do this if the class didn't have an _async_render_HEAD, but given we don't have any such classes, it doesn't really matter

Copy link
Member Author

@clokep clokep Aug 3, 2020

Choose a reason for hiding this comment

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

I debated on checking for _async_render_HEAD, but it seemed silly to add processing for something we know is always going to be false! If we need this in the future we can add this check.

Copy link
Member

Choose a reason for hiding this comment

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

seems fair!

request_method = request.method.decode("ascii")
if request_method == "HEAD":
request_method = "GET"

method_handler = getattr(
self, "_async_render_%s" % (request.method.decode("ascii"),), None
)
method_handler = getattr(self, "_async_render_%s" % (request_method,), None)
if method_handler:
raw_callback_return = method_handler(request)

Expand Down Expand Up @@ -362,11 +364,15 @@ def _get_handler_for_request(
A tuple of the callback to use, the name of the servlet, and the
key word arguments to pass to the callback
"""
# Treat HEAD requests as GET requests.
request_path = request.path.decode("ascii")
request_method = request.method
if request_method == b"HEAD":
request_method = b"GET"

# Loop through all the registered callbacks to check if the method
# and path regex match
for path_entry in self.path_regexs.get(request.method, []):
for path_entry in self.path_regexs.get(request_method, []):
m = path_entry.pattern.match(request_path)
if m:
# We found a match!
Expand Down Expand Up @@ -579,7 +585,7 @@ def set_cors_headers(request: Request):
"""
request.setHeader(b"Access-Control-Allow-Origin", b"*")
request.setHeader(
b"Access-Control-Allow-Methods", b"GET, POST, PUT, DELETE, OPTIONS"
b"Access-Control-Allow-Methods", b"GET, HEAD, POST, PUT, DELETE, OPTIONS"
)
request.setHeader(
b"Access-Control-Allow-Headers",
Expand Down
45 changes: 42 additions & 3 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,29 @@ def _callback(request, **kwargs):
self.assertEqual(channel.json_body["error"], "Unrecognized request")
self.assertEqual(channel.json_body["errcode"], "M_UNRECOGNIZED")

def test_head_request(self):
"""
JsonResource.handler_for_request gives correctly decoded URL args to
the callback, while Twisted will give the raw bytes of URL query
arguments.
"""

def _callback(request, **kwargs):
return 200, {"result": True}

res = JsonResource(self.homeserver)
res.register_paths(
"GET", [re.compile("^/_matrix/foo$")], _callback, "test_servlet",
)

# The path was registered as GET, but this is a HEAD request.
request, channel = make_request(self.reactor, b"HEAD", b"/_matrix/foo")
render(request, res, self.reactor)

self.assertEqual(channel.result["code"], b"200")
self.assertNotIn("body", channel.result)
self.assertEqual(channel.headers.getRawHeaders(b"Content-Length"), [b"15"])


class OptionsResourceTests(unittest.TestCase):
def setUp(self):
Expand Down Expand Up @@ -255,7 +278,7 @@ def setUp(self):
self.reactor = ThreadedMemoryReactorClock()

def test_good_response(self):
def callback(request):
async def callback(request):
Copy link
Member Author

Choose a reason for hiding this comment

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

Somewhat unrelated, but the TestResource._async_render_GET awaits this function so it should be async.

request.write(b"response")
request.finish()

Expand All @@ -275,7 +298,7 @@ def test_redirect_exception(self):
with the right location.
"""

def callback(request, **kwargs):
async def callback(request, **kwargs):
raise RedirectException(b"/look/an/eagle", 301)

res = WrapHtmlRequestHandlerTests.TestResource()
Expand All @@ -295,7 +318,7 @@ def test_redirect_exception_with_cookie(self):
returned too
"""

def callback(request, **kwargs):
async def callback(request, **kwargs):
e = RedirectException(b"/no/over/there", 304)
e.cookies.append(b"session=yespls")
raise e
Expand All @@ -312,3 +335,19 @@ def callback(request, **kwargs):
self.assertEqual(location_headers, [b"/no/over/there"])
cookies_headers = [v for k, v in headers if k == b"Set-Cookie"]
self.assertEqual(cookies_headers, [b"session=yespls"])

def test_head_request(self):
"""A head request should work by being turned into a GET request."""

async def callback(request):
request.write(b"response")
request.finish()

res = WrapHtmlRequestHandlerTests.TestResource()
res.callback = callback

request, channel = make_request(self.reactor, b"HEAD", b"/path")
render(request, res, self.reactor)

self.assertEqual(channel.result["code"], b"200")
self.assertNotIn("body", channel.result)