Skip to content

publicRooms validation error & parse_integer_from_args changes #16918

Closed
@TrevisGordan

Description

@TrevisGordan

Description

During testing, I encountered some 500 errors due to missing or bad validation.
For example, related to:
#13147
#14223

curl -X GET 'http://matrix.localhost/_matrix/client/v3/publicRooms?limit=-2&access_token=<TOKEN>'

-> limit=-2 causing an error on the DB level (if run with PostgreSQL), missing validation.

I've noticed there are already some motions for better parameter validation on the REST endpoints.
Ill go ahead and create a PR to fix this - My first change would be expanding the parse_integer_from_args function to optionally check and validate negative values. And move the related SynapseError INVALID_PARAM logic into it.

In addition, this change would allow the removal of around 20 duplicate code blocks.

  • Change function
  • Fix publicRooms validation
  • Remove duplicate code lines

Steps to reproduce

Homeserver

local

Synapse Version

1.94.0

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL

Workers

Multiple workers

Platform

kubernetes Dockerdesktop

Configuration

No response

Relevant log output

"""
2024-01-31 08:26:26,665 - synapse.http.server - 140 - ERROR - GET-122048 - Failed handle request via 'PublicRoomListRestServlet': <XForwardedForRequest at 0x7fffddf62640 method='GET' uri='/_matrix/client/v3/publicRooms?limit=-2&access_token=<redacted>' clientproto='HTTP/1.1' site='8083'>
"""
Traceback (most recent call last):
  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/util/caches/response_cache.py", line 258, in cb
    return await callback(*args, **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/synapse/handlers/room_list.py", line 164, in _get_public_room_list
    results = await self.store.get_largest_public_rooms(
  File "/usr/local/lib/python3.9/dist-packages/synapse/storage/databases/main/room.py", line 518, in get_largest_public_rooms
    ret_val = await self.db_pool.runInteraction(
  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/room.py", line 509, in _get_largest_public_rooms_txn
    txn.execute(sql, query_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.InvalidRowCountInLimitClause: LIMIT must not be negative

Anything else that would be useful to know?

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions