Respond correctly to unknown methods on known endpoints (#14605)

Respond with a 405 error if a request is received on a known endpoint,
but to an unknown method, per MSC3743.
This commit is contained in:
Patrick Cloke 2023-02-09 13:04:24 -05:00 committed by GitHub
parent 8a6e043488
commit d22c1c862c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 89 additions and 51 deletions

1
changelog.d/14605.bugfix Normal file
View File

@ -0,0 +1 @@
Return spec-compliant JSON errors when unknown endpoints are requested.

View File

@ -235,6 +235,14 @@ The following fields are returned in the JSON response body:
Request:
```
POST /_synapse/admin/v1/media/delete?before_ts=<before_ts>
{}
```
*Deprecated in Synapse v1.78.0:* This API is available at the deprecated endpoint:
```
POST /_synapse/admin/v1/media/<server_name>/delete?before_ts=<before_ts>
@ -243,7 +251,7 @@ POST /_synapse/admin/v1/media/<server_name>/delete?before_ts=<before_ts>
URL Parameters
* `server_name`: string - The name of your local server (e.g `matrix.org`).
* `server_name`: string - The name of your local server (e.g `matrix.org`). *Deprecated in Synapse v1.78.0.*
* `before_ts`: string representing a positive integer - Unix timestamp in milliseconds.
Files that were last used before this timestamp will be deleted. It is the timestamp of
last access, not the timestamp when the file was created.

View File

@ -88,6 +88,15 @@ process, for example:
dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
```
# Upgrading to v1.78.0
## Deprecate the `/_synapse/admin/v1/media/<server_name>/delete` admin API
Synapse 1.78.0 replaces the `/_synapse/admin/v1/media/<server_name>/delete`
admin API with an identical endpoint at `/_synapse/admin/v1/media/delete`. Please
update your tooling to use the new endpoint. The deprecated version will be removed
in a future release.
# Upgrading to v1.76.0
## Faster joins are enabled by default
@ -137,6 +146,7 @@ and then do `pip install matrix-synapse[user-search]` for a PyPI install.
Docker images and Debian packages need nothing specific as they already
include or specify ICU as an explicit dependency.
# Upgrading to v1.73.0
## Legacy Prometheus metric names have now been removed

View File

@ -30,7 +30,6 @@ from typing import (
Iterable,
Iterator,
List,
NoReturn,
Optional,
Pattern,
Tuple,
@ -340,7 +339,8 @@ class _AsyncResource(resource.Resource, metaclass=abc.ABCMeta):
return callback_return
return _unrecognised_request_handler(request)
# A request with an unknown method (for a known endpoint) was received.
raise UnrecognizedRequestError(code=405)
@abc.abstractmethod
def _send_response(
@ -396,7 +396,6 @@ class DirectServeJsonResource(_AsyncResource):
@attr.s(slots=True, frozen=True, auto_attribs=True)
class _PathEntry:
pattern: Pattern
callback: ServletCallback
servlet_classname: str
@ -425,13 +424,14 @@ class JsonResource(DirectServeJsonResource):
):
super().__init__(canonical_json, extract_context)
self.clock = hs.get_clock()
self.path_regexs: Dict[bytes, List[_PathEntry]] = {}
# Map of path regex -> method -> callback.
self._routes: Dict[Pattern[str], Dict[bytes, _PathEntry]] = {}
self.hs = hs
def register_paths(
self,
method: str,
path_patterns: Iterable[Pattern],
path_patterns: Iterable[Pattern[str]],
callback: ServletCallback,
servlet_classname: str,
) -> None:
@ -455,8 +455,8 @@ class JsonResource(DirectServeJsonResource):
for path_pattern in path_patterns:
logger.debug("Registering for %s %s", method, path_pattern.pattern)
self.path_regexs.setdefault(method_bytes, []).append(
_PathEntry(path_pattern, callback, servlet_classname)
self._routes.setdefault(path_pattern, {})[method_bytes] = _PathEntry(
callback, servlet_classname
)
def _get_handler_for_request(
@ -478,14 +478,17 @@ class JsonResource(DirectServeJsonResource):
# 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, []):
m = path_entry.pattern.match(request_path)
for path_pattern, methods in self._routes.items():
m = path_pattern.match(request_path)
if m:
# We found a match!
# We found a matching path!
path_entry = methods.get(request_method)
if not path_entry:
raise UnrecognizedRequestError(code=405)
return path_entry.callback, path_entry.servlet_classname, m.groupdict()
# Huh. No one wanted to handle that? Fiiiiiine. Send 400.
return _unrecognised_request_handler, "unrecognised_request_handler", {}
# Huh. No one wanted to handle that? Fiiiiiine.
raise UnrecognizedRequestError(code=404)
async def _async_render(self, request: SynapseRequest) -> Tuple[int, Any]:
callback, servlet_classname, group_dict = self._get_handler_for_request(request)
@ -567,19 +570,6 @@ class StaticResource(File):
return super().render_GET(request)
def _unrecognised_request_handler(request: Request) -> NoReturn:
"""Request handler for unrecognised requests
This is a request handler suitable for return from
_get_handler_for_request. It actually just raises an
UnrecognizedRequestError.
Args:
request: Unused, but passed in to match the signature of ServletCallback.
"""
raise UnrecognizedRequestError(code=404)
class UnrecognizedRequestResource(resource.Resource):
"""
Similar to twisted.web.resource.NoResource, but returns a JSON 404 with an

View File

@ -15,7 +15,7 @@
import logging
from http import HTTPStatus
from typing import TYPE_CHECKING, Tuple
from typing import TYPE_CHECKING, Optional, Tuple
from synapse.api.constants import Direction
from synapse.api.errors import Codes, NotFoundError, SynapseError
@ -285,7 +285,12 @@ class DeleteMediaByDateSize(RestServlet):
timestamp and size.
"""
PATTERNS = admin_patterns("/media/(?P<server_name>[^/]*)/delete$")
PATTERNS = [
*admin_patterns("/media/delete$"),
# This URL kept around for legacy reasons, it is undesirable since it
# overlaps with the DeleteMediaByID servlet.
*admin_patterns("/media/(?P<server_name>[^/]*)/delete$"),
]
def __init__(self, hs: "HomeServer"):
self.store = hs.get_datastores().main
@ -294,7 +299,7 @@ class DeleteMediaByDateSize(RestServlet):
self.media_repository = hs.get_media_repository()
async def on_POST(
self, request: SynapseRequest, server_name: str
self, request: SynapseRequest, server_name: Optional[str] = None
) -> Tuple[int, JsonDict]:
await assert_requester_is_admin(self.auth, request)
@ -322,7 +327,8 @@ class DeleteMediaByDateSize(RestServlet):
errcode=Codes.INVALID_PARAM,
)
if self.server_name != server_name:
# This check is useless, we keep it for the legacy endpoint only.
if server_name is not None and self.server_name != server_name:
raise SynapseError(HTTPStatus.BAD_REQUEST, "Can only delete local media")
logging.info(
@ -489,6 +495,8 @@ def register_servlets_for_media_repo(hs: "HomeServer", http_server: HttpServer)
ProtectMediaByID(hs).register(http_server)
UnprotectMediaByID(hs).register(http_server)
ListMediaInRoom(hs).register(http_server)
DeleteMediaByID(hs).register(http_server)
# XXX DeleteMediaByDateSize must be registered before DeleteMediaByID as
# their URL routes overlap.
DeleteMediaByDateSize(hs).register(http_server)
DeleteMediaByID(hs).register(http_server)
UserMediaRestServlet(hs).register(http_server)

View File

@ -259,6 +259,32 @@ class RoomKeysNewVersionServlet(RestServlet):
self.auth = hs.get_auth()
self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler()
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"""
Retrieve the version information about the most current backup version (if any)
It takes out an exclusive lock on this user's room_key backups, to ensure
clients only upload to the current backup.
Returns 404 if the given version does not exist.
GET /room_keys/version HTTP/1.1
{
"version": "12345",
"algorithm": "m.megolm_backup.v1",
"auth_data": "dGhpcyBzaG91bGQgYWN0dWFsbHkgYmUgZW5jcnlwdGVkIGpzb24K"
}
"""
requester = await self.auth.get_user_by_req(request, allow_guest=False)
user_id = requester.user.to_string()
try:
info = await self.e2e_room_keys_handler.get_version_info(user_id)
except SynapseError as e:
if e.code == 404:
raise SynapseError(404, "No backup found", Codes.NOT_FOUND)
return 200, info
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"""
Create a new backup version for this user's room_keys with the given
@ -301,7 +327,7 @@ class RoomKeysNewVersionServlet(RestServlet):
class RoomKeysVersionServlet(RestServlet):
PATTERNS = client_patterns("/room_keys/version(/(?P<version>[^/]+))?$")
PATTERNS = client_patterns("/room_keys/version/(?P<version>[^/]+)$")
def __init__(self, hs: "HomeServer"):
super().__init__()
@ -309,12 +335,11 @@ class RoomKeysVersionServlet(RestServlet):
self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler()
async def on_GET(
self, request: SynapseRequest, version: Optional[str]
self, request: SynapseRequest, version: str
) -> Tuple[int, JsonDict]:
"""
Retrieve the version information about a given version of the user's
room_keys backup. If the version part is missing, returns info about the
most current backup version (if any)
room_keys backup.
It takes out an exclusive lock on this user's room_key backups, to ensure
clients only upload to the current backup.
@ -339,20 +364,16 @@ class RoomKeysVersionServlet(RestServlet):
return 200, info
async def on_DELETE(
self, request: SynapseRequest, version: Optional[str]
self, request: SynapseRequest, version: str
) -> Tuple[int, JsonDict]:
"""
Delete the information about a given version of the user's
room_keys backup. If the version part is missing, deletes the most
current backup version (if any). Doesn't delete the actual room data.
room_keys backup. Doesn't delete the actual room data.
DELETE /room_keys/version/12345 HTTP/1.1
HTTP/1.1 200 OK
{}
"""
if version is None:
raise SynapseError(400, "No version specified to delete", Codes.NOT_FOUND)
requester = await self.auth.get_user_by_req(request, allow_guest=False)
user_id = requester.user.to_string()
@ -360,7 +381,7 @@ class RoomKeysVersionServlet(RestServlet):
return 200, {}
async def on_PUT(
self, request: SynapseRequest, version: Optional[str]
self, request: SynapseRequest, version: str
) -> Tuple[int, JsonDict]:
"""
Update the information about a given version of the user's room_keys backup.
@ -386,11 +407,6 @@ class RoomKeysVersionServlet(RestServlet):
user_id = requester.user.to_string()
info = parse_json_object_from_request(request)
if version is None:
raise SynapseError(
400, "No version specified to update", Codes.MISSING_PARAM
)
await self.e2e_room_keys_handler.update_version(user_id, version, info)
return 200, {}

View File

@ -34,7 +34,9 @@ class TagListServlet(RestServlet):
GET /user/{user_id}/rooms/{room_id}/tags HTTP/1.1
"""
PATTERNS = client_patterns("/user/(?P<user_id>[^/]*)/rooms/(?P<room_id>[^/]*)/tags")
PATTERNS = client_patterns(
"/user/(?P<user_id>[^/]*)/rooms/(?P<room_id>[^/]*)/tags$"
)
def __init__(self, hs: "HomeServer"):
super().__init__()

View File

@ -213,7 +213,8 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase):
self.admin_user_tok = self.login("admin", "pass")
self.filepaths = MediaFilePaths(hs.config.media.media_store_path)
self.url = "/_synapse/admin/v1/media/%s/delete" % self.server_name
self.url = "/_synapse/admin/v1/media/delete"
self.legacy_url = "/_synapse/admin/v1/media/%s/delete" % self.server_name
# Move clock up to somewhat realistic time
self.reactor.advance(1000000000)
@ -332,11 +333,13 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase):
channel.json_body["error"],
)
def test_delete_media_never_accessed(self) -> None:
@parameterized.expand([(True,), (False,)])
def test_delete_media_never_accessed(self, use_legacy_url: bool) -> None:
"""
Tests that media deleted if it is older than `before_ts` and never accessed
`last_access_ts` is `NULL` and `created_ts` < `before_ts`
"""
url = self.legacy_url if use_legacy_url else self.url
# upload and do not access
server_and_media_id = self._create_media()
@ -351,7 +354,7 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase):
now_ms = self.clock.time_msec()
channel = self.make_request(
"POST",
self.url + "?before_ts=" + str(now_ms),
url + "?before_ts=" + str(now_ms),
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)