Notification Validation Issue on [GET /_matrix/client/v3/notifications]

[TrevisGordan]

Description

There appears to be a discrepancy in data type validation for the from parameter in the [GET /_matrix/client/v3/notifications] endpoint, leading to errors when interacting with PostgreSQL databases. Specifically, the issue arises from the from parameter being passed as a string (from='XXX'), which contradicts the database's expectation of a bigint, resulting in an InvalidTextRepresentation error from psycopg2.

Detailed Description:

The error encountered is as follows:

psycopg2.errors.InvalidTextRepresentation: invalid input syntax for type bigint: "xxxxx"

This indicates that the database expects a bigint value, not a string. The problematic behavior is linked to the handling of the from_token parameter, as detailed in the code reference. Here, the from_token is passed to the get_push_actions_for_user function as a string under the before argument, which is then used to query the event_push_actions.stream_ordering database value, an integer.

It's notable that the line of code in question is 8 years old, suggesting that the from_token parameter's expected data type might have been a string in earlier implementations. However, throughout the codebase, the usage of from_token varies between integers and strings.

This however conflicts with the schema specification requiring a string value, detailed further in the Matrix specification.

Proposed Solution:

To address this inconsistency and eliminate the database error, I suggest modifying the handling of the from_token parameter within the GET function as this also is the only reference to the get_push_actions_for_user function.

The most straightforward solution would be to replace the parse_string method with parse_integer for this specific case and to ensure it validates the parameter as an integer.

Additionally, it may be necessary to revisit the schema specification to ensure it aligns with the actual data types used in the code, especially considering the numeric nature of the stream_ordering value.

I am prepared to draft a pull request to implement these changes, pending approval or further discussion (as I am uncertain about the from_token history).

Steps to reproduce

• call notifications endpoint with from string parameter
• Reproduce with:
  curl -X GET 'http://matrix.localhost/_matrix/client/v3/notifications?from=xxxxx&limit=20&only=highlight&access_token=≤TOKEN>'

Homeserver

local

Synapse Version

1.94.0

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL

Workers

Multiple workers

Platform

Kubernetes

Configuration

No response

Relevant log output

"""
2024-01-31 08:26:26,051 - synapse.http.server - 140 - ERROR - GET-122018 - Failed handle request via 'NotificationsServlet': <XForwardedForRequest at 0x7fffbdee1ac0 method='GET' uri='/_matrix/client/v3/notifications?from=xxxxx&limit=20&only=highlight&access_token=<redacted>' clientproto='HTTP/1.1' site='8083'>
"""
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/synapse/http/server.py", line 326, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "/usr/local/lib/python3.9/dist-packages/synapse/http/server.py", line 538, in _async_render
    callback_return = await raw_callback_return
  File "/usr/local/lib/python3.9/dist-packages/synapse/rest/client/notifications.py", line 58, in on_GET
    push_actions = await self.store.get_push_actions_for_user(
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/databases/main/event_push_actions.py", line 1783, in get_push_actions_for_user
    push_actions = await self.db_pool.runInteraction("get_push_actions_for_user", f)
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/database.py", line 934, in runInteraction
    return await delay_cancellation(_runInteraction())
  File "/usr/local/lib/python3.9/dist-packages/twisted/internet/defer.py", line 1993, in _inlineCallbacks
    result = context.run(
  File "/usr/local/lib/python3.9/dist-packages/twisted/python/failure.py", line 518, in throwExceptionIntoGenerator
    return g.throw(self.type, self.value, self.tb)
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/database.py", line 900, in _runInteraction
    result = await self.runWithConnection(
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/database.py", line 1029, in runWithConnection
    return await make_deferred_yieldable(
  File "/usr/local/lib/python3.9/dist-packages/twisted/python/threadpool.py", line 244, in inContext
    result = inContext.theWork()  # type: ignore[attr-defined]
  File "/usr/local/lib/python3.9/dist-packages/twisted/python/threadpool.py", line 260, in <lambda>
    inContext.theWork = lambda: context.call(  # type: ignore[attr-defined]
  File "/usr/local/lib/python3.9/dist-packages/twisted/python/context.py", line 117, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/usr/local/lib/python3.9/dist-packages/twisted/python/context.py", line 82, in callWithContext
    return func(*args, **kw)
  File "/usr/local/lib/python3.9/dist-packages/twisted/enterprise/adbapi.py", line 282, in _runWithConnection
    result = func(conn, *args, **kw)
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/database.py", line 1022, in inner_func
    return func(db_conn, *args, **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/database.py", line 762, in new_transaction
    r = func(cursor, *args, **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/databases/main/event_push_actions.py", line 1778, in f
    txn.execute(sql, args)
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/database.py", line 421, in execute
    self._do_execute(self.txn.execute, sql, parameters)
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/database.py", line 473, in _do_execute
    return func(sql, *args, **kwargs)
psycopg2.errors.InvalidTextRepresentation: invalid input syntax for type bigint: "xxxxx"
LINE 7: ...00002:server.localhost' AND epa.stream_ordering < 'xxxxx' AN...

Anything else that would be useful to know?

No response