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

Fix destination_is errors seen in sentry. #13041

Merged
merged 8 commits into from
Jun 14, 2022
Merged
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
Prev Previous commit
Don't verify domain is nonempty in from_string
Use is_valid instead
  • Loading branch information
David Robertson committed Jun 14, 2022
commit c7f90f3ab1c87a3c7563a61d291e884201064d27
20 changes: 16 additions & 4 deletions synapse/rest/client/profile.py
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@
# limitations under the License.

""" This module contains REST servlets to do with profile: /profile/<paths> """

from http import HTTPStatus
from typing import TYPE_CHECKING, Tuple

from synapse.api.errors import Codes, SynapseError
@@ -45,8 +45,12 @@ async def on_GET(
requester = await self.auth.get_user_by_req(request)
requester_user = requester.user

user = UserID.from_string(user_id)
if not UserID.is_valid(user_id):
raise SynapseError(
HTTPStatus.BAD_REQUEST, "Invalid user id", Codes.INVALID_PARAM
)

user = UserID.from_string(user_id)
await self.profile_handler.check_profile_query_allowed(user, requester_user)

displayname = await self.profile_handler.get_displayname(user)
@@ -98,8 +102,12 @@ async def on_GET(
requester = await self.auth.get_user_by_req(request)
requester_user = requester.user

user = UserID.from_string(user_id)
if not UserID.is_valid(user_id):
raise SynapseError(
HTTPStatus.BAD_REQUEST, "Invalid user id", Codes.INVALID_PARAM
)

user = UserID.from_string(user_id)
await self.profile_handler.check_profile_query_allowed(user, requester_user)

avatar_url = await self.profile_handler.get_avatar_url(user)
@@ -150,8 +158,12 @@ async def on_GET(
requester = await self.auth.get_user_by_req(request)
requester_user = requester.user

user = UserID.from_string(user_id)
if not UserID.is_valid(user_id):
raise SynapseError(
HTTPStatus.BAD_REQUEST, "Invalid user id", Codes.INVALID_PARAM
)

user = UserID.from_string(user_id)
await self.profile_handler.check_profile_query_allowed(user, requester_user)

displayname = await self.profile_handler.get_displayname(user)
11 changes: 2 additions & 9 deletions synapse/types.py
Original file line number Diff line number Diff line change
@@ -267,15 +267,6 @@ def from_string(cls: Type[DS], s: str) -> DS:
)

domain = parts[1]
if not domain:
raise SynapseError(
400,
f"{cls.__name__} must have a domain after the colon",
Codes.INVALID_PARAM,
)
# TODO: this does not reject an empty localpart or an overly-long string.
# See https://spec.matrix.org/v1.2/appendices/#identifier-grammar

# This code will need changing if we want to support multiple domain
# names on one HS
return cls(localpart=parts[0], domain=domain)
@@ -287,6 +278,8 @@ def to_string(self) -> str:
@classmethod
def is_valid(cls: Type[DS], s: str) -> bool:
"""Parses the input string and attempts to ensure it is valid."""
# TODO: this does not reject an empty localpart or an overly-long string.
# See https://spec.matrix.org/v1.2/appendices/#identifier-grammar
try:
obj = cls.from_string(s)
# Apply additional validation to the domain. This is only done
3 changes: 2 additions & 1 deletion tests/rest/client/test_profile.py
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@
# limitations under the License.

"""Tests REST events for /profile paths."""
import urllib.parse
from http import HTTPStatus
from typing import Any, Dict, Optional

@@ -52,7 +53,7 @@ def test_get_displayname(self) -> None:

def test_get_displayname_rejects_bad_username(self) -> None:
channel = self.make_request(
"GET", "/profile/notanmxid%40example.com/displayname"
"GET", f"/profile/{urllib.parse.quote('@alice:')}/displayname"
)
self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result)

5 changes: 2 additions & 3 deletions tests/test_types.py
Original file line number Diff line number Diff line change
@@ -38,9 +38,8 @@ def test_parse_rejects_missing_separator(self):
with self.assertRaises(SynapseError):
UserID.from_string("@alice.example.com")

def test_parse_rejects_missing_domain(self):
with self.assertRaises(SynapseError):
UserID.from_string("@alice:")
def test_validation_rejects_missing_domain(self):
self.assertFalse(UserID.is_valid("@alice:"))

def test_build(self):
user = UserID("5678efgh", "my.domain")