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

Update the error code when a canonical alias that does not exist is used #7109

Merged
merged 2 commits into from
Mar 23, 2020
Merged

Update the error code when a canonical alias that does not exist is used #7109

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
affbb58
Treat an unknown alias as a BAD_ALIAS for the canonical alias event.
clokep Mar 19, 2020
783701c
Reduce duplicated code.
clokep Mar 23, 2020
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/7109.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Return the proper error (M_BAD_ALIAS) when a non-existant canonical alias is provided.
26 changes: 24 additions & 2 deletions synapse/handlers/message.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,18 @@ def persist_and_notify_client_event(
directory_handler = self.hs.get_handlers().directory_handler
if room_alias_str and room_alias_str != original_alias:
room_alias = RoomAlias.from_string(room_alias_str)
clokep marked this conversation as resolved.
Show resolved Hide resolved
mapping = yield directory_handler.get_association(room_alias)
try:
mapping = yield directory_handler.get_association(room_alias)
except SynapseError as e:
# Turn M_NOT_FOUND errors into M_BAD_ALIAS errors.
Copy link
Member

Choose a reason for hiding this comment

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

have you checked that this codepath is reached when it's a remote alias? I wouldn't be surprised if we fail to turn the HTTP 404 into a SynapseError.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have not checked that. Will look into it!

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe this is properly handled:

synapse/synapse/handlers/directory.py

Lines 240 to 264 in 229eb81

try:
result = yield self.federation.make_query(
destination=room_alias.domain,
query_type="directory",
args={"room_alias": room_alias.to_string()},
retry_on_dns_fail=False,
ignore_backoff=True,
)
except CodeMessageException as e:
logging.warning("Error retrieving alias")
if e.code == 404:
result = None
else:
raise
if result and "room_id" in result and "servers" in result:
room_id = result["room_id"]
servers = result["servers"]
if not room_id:
raise SynapseError(
404,
"Room alias %s not found" % (room_alias.to_string(),),
Codes.NOT_FOUND,
)

That code seems to handle the 404, never setting a room_id, which then raises the SynapseError.

if e.errcode == Codes.NOT_FOUND:
raise SynapseError(
400,
"Room alias %s does not point to the room"
% (room_alias_str,),
Codes.BAD_ALIAS,
)
raise

if mapping["room_id"] != event.room_id:
raise SynapseError(
Expand All @@ -932,7 +943,18 @@ def persist_and_notify_client_event(
if new_alt_aliases:
for alias_str in new_alt_aliases:
room_alias = RoomAlias.from_string(alias_str)
mapping = yield directory_handler.get_association(room_alias)
try:
mapping = yield directory_handler.get_association(room_alias)
except SynapseError as e:
# Turn M_NOT_FOUND errors into M_BAD_ALIAS errors.
if e.errcode == Codes.NOT_FOUND:
raise SynapseError(
400,
"Room alias %s does not point to the room"
% (room_alias_str,),
Codes.BAD_ALIAS,
)
raise

if mapping["room_id"] != event.room_id:
raise SynapseError(
Expand Down