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

Fix a long-standing spec compliance bug where Synapse would accept a trailing slash on the end of /get_missing_events federation requests. #13789

Merged
merged 3 commits into from
Sep 14, 2022
Merged

Fix a long-standing spec compliance bug where Synapse would accept a trailing slash on the end of /get_missing_events federation requests. #13789

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56a528a
Don't accept a trailing slash on the end of /get_missing_events
reivilibre Sep 13, 2022
1099c07
Newsfile
reivilibre Sep 13, 2022
bb4e460
Merge branch 'develop' into rei/gme_remove_trailing_slash
reivilibre Sep 13, 2022
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/13789.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing spec compliance bug where Synapse would accept a trailing slash on the end of `/get_missing_events` federation requests.
3 changes: 1 addition & 2 deletions synapse/federation/transport/server/federation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,7 @@ async def on_POST(


class FederationGetMissingEventsServlet(BaseFederationServerServlet):
# TODO(paul): Why does this path alone end with "/?" optional?
PATH = "/get_missing_events/(?P<room_id>[^/]*)/?"
PATH = "/get_missing_events/(?P<room_id>[^/]*)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should that * be a +? Presumably room ids must be nonempty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at a few other examples, for identifiers we just do * and rely on validating them after the fact, so using + would be 'inconsistent'. Not sure it makes much difference either way; maybe we should also validate that they start with ! but I don't know.

Copy link
Member

Choose a reason for hiding this comment

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

I think if it was a + then the servlet matching code wouldn't even match to this request and you would get a generic M_UNRECOGNIZED_REQUEST error. Hopefully by being more graceful in matching we can give a better error about a missing parameters or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like we should leave this alone!


async def on_POST(
self,
Expand Down