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

Change display names/avatar URLs to None if they contain null bytes before storing in DB #11230

Merged
merged 7 commits into from
Nov 12, 2021
Merged

Change display names/avatar URLs to None if they contain null bytes before storing in DB #11230

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fdb5ce2
change display names/avatar URLS to None if they contain null bytes
H-Shay Nov 1, 2021
fab4d0f
add changelog
H-Shay Nov 1, 2021
a8ebf2d
add POC test, requested changes
H-Shay Nov 3, 2021
faaebc0
add a saner test and remove old one
H-Shay Nov 4, 2021
d02a52c
update test to verify that display name has been changed to None
H-Shay Nov 8, 2021
d8b733c
make test less fragile
H-Shay Nov 9, 2021
8b32da9
Merge branch 'develop' into disallow_null_byte
H-Shay Nov 9, 2021
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
2 changes: 2 additions & 0 deletions changelog.d/11230.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a bug wherein display names or avatar URLs containing null bytes cause an internal server error
when stored in the DB.
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ def _store_room_members_txn(self, txn, events, backfilled):
"""Store a room member in the database."""

def str_or_none(val: Any) -> Optional[str]:
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should update the name of this now that it does more than ensuring it is a string? 🤷 It's a pretty simple function so not a big deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the name, let me know if you think it's a little clearer.

return val if isinstance(val, str) else None
return val if isinstance(val, str) and "\u0000" not in val else None

self.db_pool.simple_insert_many_txn(
txn,
Expand Down