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

Internal error due to NUL byte in Conduit federation #10886

Closed
jplatte opened this issue Sep 22, 2021 · 3 comments · Fixed by #11230
Closed

Internal error due to NUL byte in Conduit federation #10886

jplatte opened this issue Sep 22, 2021 · 3 comments · Fixed by #11230
Assignees
Labels
S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@jplatte
Copy link
Contributor

jplatte commented Sep 22, 2021

This internal error just happened to sbd. when joining #conduit:fachschaften.org (the main Conduit room) with Synapse:

8ZhxlGdY - Error attempting to resolve state at missing prev_events
Sep 22 10:50:55 debian matrix-synapse[3275]: Traceback (most recent call last):
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/handlers/federation_event.py", line 710, in _resolve_state_at_missing_prevs
Sep 22 10:50:55 debian matrix-synapse[3275]:     remote_state = await self._get_state_after_missing_prev_event(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/handlers/federation_event.py", line 817, in _get_state_after_missing_prev_event
Sep 22 10:50:55 debian matrix-synapse[3275]:     await self._get_events_and_persist(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/handlers/federation_event.py", line 1171, in _get_events_and_persist
Sep 22 10:50:55 debian matrix-synapse[3275]:     await self._auth_and_persist_fetched_events(destination, room_id, roots)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/handlers/federation_event.py", line 1240, in _auth_and_persist_fetched_events
Sep 22 10:50:55 debian matrix-synapse[3275]:     await self.persist_events_and_notify(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/handlers/federation_event.py", line 1767, in persist_events_and_notify
Sep 22 10:50:55 debian matrix-synapse[3275]:     events, max_stream_token = await self._storage.persistence.persist_events(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/persist_events.py", line 322, in persist_events
Sep 22 10:50:55 debian matrix-synapse[3275]:     ret_vals = await yieldable_gather_results(enqueue, partitioned.items())
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/persist_events.py", line 239, in handle_queue_loop
Sep 22 10:50:55 debian matrix-synapse[3275]:     ret = await self._per_item_callback(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/persist_events.py", line 577, in _persist_event_batch
Sep 22 10:50:55 debian matrix-synapse[3275]:     await self.persist_events_store._persist_events_and_state_updates(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/databases/main/events.py", line 174, in _persist_events_and_state_updates
Sep 22 10:50:55 debian matrix-synapse[3275]:     await self.db_pool.runInteraction(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 686, in runInteraction
Sep 22 10:50:55 debian matrix-synapse[3275]:     result = await self.runWithConnection(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 791, in runWithConnection
Sep 22 10:50:55 debian matrix-synapse[3275]:     return await make_deferred_yieldable(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/python/threadpool.py", line 238, in inContext
Sep 22 10:50:55 debian matrix-synapse[3275]:     result = inContext.theWork()  # type: ignore[attr-defined]
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/python/threadpool.py", line 254, in <lambda>
Sep 22 10:50:55 debian matrix-synapse[3275]:     inContext.theWork = lambda: context.call(  # type: ignore[attr-defined]
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/python/context.py", line 118, in callWithContext
Sep 22 10:50:55 debian matrix-synapse[3275]:     return self.currentContext().callWithContext(ctx, func, *args, **kw)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/python/context.py", line 83, in callWithContext
Sep 22 10:50:55 debian matrix-synapse[3275]:     return func(*args, **kw)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/enterprise/adbapi.py", line 293, in _runWithConnection
Sep 22 10:50:55 debian matrix-synapse[3275]:     compat.reraise(excValue, excTraceback)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/python/deprecate.py", line 298, in deprecatedFunction
Sep 22 10:50:55 debian matrix-synapse[3275]:     return function(*args, **kwargs)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/python/compat.py", line 404, in reraise
Sep 22 10:50:55 debian matrix-synapse[3275]:     raise exception.with_traceback(traceback)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/twisted/enterprise/adbapi.py", line 284, in _runWithConnection
Sep 22 10:50:55 debian matrix-synapse[3275]:     result = func(conn, *args, **kw)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 786, in inner_func
Sep 22 10:50:55 debian matrix-synapse[3275]:     return func(db_conn, *args, **kwargs)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 554, in new_transaction
Sep 22 10:50:55 debian matrix-synapse[3275]:     r = func(cursor, *args, **kwargs)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/logging/utils.py", line 69, in wrapped
Sep 22 10:50:55 debian matrix-synapse[3275]:     return f(*args, **kwargs)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/databases/main/events.py", line 396, in _persist_events_txn
Sep 22 10:50:55 debian matrix-synapse[3275]:     self._update_metadata_tables_txn(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/databases/main/events.py", line 1528, in _update_metadata_tables_txn
Sep 22 10:50:55 debian matrix-synapse[3275]:     self._store_room_members_txn(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/databases/main/events.py", line 1666, in _store_room_members_txn
Sep 22 10:50:55 debian matrix-synapse[3275]:     self.db_pool.simple_insert_many_txn(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 931, in simple_insert_many_txn
Sep 22 10:50:55 debian matrix-synapse[3275]:     txn.execute_values(sql, vals, fetch=False)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 293, in execute_values
Sep 22 10:50:55 debian matrix-synapse[3275]:     return self._do_execute(
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 331, in _do_execute
Sep 22 10:50:55 debian matrix-synapse[3275]:     return func(sql, *args)
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/synapse/storage/database.py", line 294, in <lambda>
Sep 22 10:50:55 debian matrix-synapse[3275]:     lambda *x: execute_values(self.txn, *x, fetch=fetch), sql, *args
Sep 22 10:50:55 debian matrix-synapse[3275]:   File "/home/synapse/synapse/env/lib/python3.9/site-packages/psycopg2/extras.py", line 1267, in execute_values
Sep 22 10:50:55 debian matrix-synapse[3275]:     parts.append(cur.mogrify(template, args))
Sep 22 10:50:55 debian matrix-synapse[3275]: ValueError: A string literal cannot contain NUL (0x00) characters.

Originally posted by @jplatte in #9341 (comment)

@erikjohnston erikjohnston added S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Sep 23, 2021
@reivilibre reivilibre self-assigned this Sep 30, 2021
@reivilibre
Copy link
Contributor

reivilibre commented Sep 30, 2021

This seems to be caused by this code (where we insert into the room_memberships table):

def _store_room_members_txn(self, txn, events, backfilled):
"""Store a room member in the database."""
def str_or_none(val: Any) -> Optional[str]:
return val if isinstance(val, str) else None
self.db_pool.simple_insert_many_txn(
txn,
table="room_memberships",
values=[
{
"event_id": event.event_id,
"user_id": event.state_key,
"sender": event.user_id,
"room_id": event.room_id,
"membership": event.membership,
"display_name": str_or_none(event.content.get("displayname")),
"avatar_url": str_or_none(event.content.get("avatar_url")),
}
for event in events
],
)

It's likely a null byte in a display name or avatar URL. (I wonder if it's possible for null bytes to be in the membership value at this point? Would have thought not as presumably that event would be soft-failed and not trigger this code, but I haven't checked that.)

It's not entirely clear what the best thing to do is here; it probably depends on where this table is used. It might be acceptable to convert display names/avatar URLs to None if they contain null bytes, or perhaps the types of the fields could be changed to be BLOB (or is it BINARY?) rather than TEXT so that null bytes are not a problem.

@reivilibre reivilibre removed their assignment Sep 30, 2021
@richvdh
Copy link
Member

richvdh commented Sep 30, 2021

It might be acceptable to convert display names/avatar URLs to None if they contain null bytes,

I think we already do this for displaynames/avatar URLs which aren't strings somewhere. +1 to extending that to NUL bytes

@richvdh
Copy link
Member

richvdh commented Sep 30, 2021

aside: a concern was expressed about storing JSON which contains NULs in the database in general. What we forgot at the time is that, in JSON, NUL bytes within strings are represented as "\0" (or possibly "\u0000" or similar). It's only a problem when we take the strings out of the JSON and try to work with the string itself - as we do here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
5 participants