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

Remove any NULL characters from remote displaynames before updating user directory #12743

Closed
wants to merge 4 commits into from
Closed

Remove any NULL characters from remote displaynames before updating user directory #12743

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1101dc3
Remove any NULL characters from displaynames to avoid breaking databa…
Fizzadar May 16, 2022
29aff26
Make user directory null string handling same as member events
Fizzadar May 17, 2022
621d8f1
Add changelog file
Fizzadar May 17, 2022
cf63121
Remove old replace fix
Fizzadar May 17, 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
4 changes: 4 additions & 0 deletions synapse/handlers/user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ async def _handle_possible_remote_profile_change(

prev_name = prev_event.content.get("displayname")
new_name = event.content.get("displayname")

# Replace any NULL characters in the name as these cannot be stored in the database
new_name = new_name.replace("\x00", "\uFFFD")
Copy link
Contributor

Choose a reason for hiding this comment

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

For completeness, sqlite sort-of-supports null-codepointsin strings, with scary caveats: https://sqlite.org/nulinstr.html
Postgres definitely doesn't and won't any time soon. See e.g. this HN post.

Copy link
Contributor

Choose a reason for hiding this comment

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

We used a space as the replacement character in #10820. I think this might have been chosen to make sure that the text-search stuff (as in https://www.postgresql.org/docs/current/datatype-textsearch.html and https://www.postgresql.org/docs/current/textsearch-controls.html) work more nicely. Let me see if I can experiment to see how postgres handles a replacement character in that context...

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll want to move this after the isinstance(new_name, str) validation below.

... actually do we want to do it after the has-anything-changed condition?:
prev_name != new_name or prev_avatar != new_avatar


# If the new name is an unexpected form, do not update the directory.
if not isinstance(new_name, str):
new_name = prev_name
Expand Down