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

Prevent message search in upgraded rooms we're not in #6385

Merged
merged 11 commits into from
Dec 11, 2019

Conversation

anoadragon453
Copy link
Member

Fixes #6356

I assume this is preferred instead of attempting to join the predecessor room before searching it, as that could be slow and we may not be able to join.

@anoadragon453 anoadragon453 requested a review from a team November 19, 2019 16:52
@anoadragon453 anoadragon453 self-assigned this Nov 19, 2019
synapse/handlers/search.py Outdated Show resolved Hide resolved
richvdh
richvdh previously requested changes Dec 4, 2019
synapse/handlers/search.py Outdated Show resolved Hide resolved
synapse/handlers/search.py Outdated Show resolved Hide resolved
synapse/handlers/search.py Outdated Show resolved Hide resolved
synapse/handlers/search.py Outdated Show resolved Hide resolved
richvdh
richvdh previously requested changes Dec 4, 2019
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

I don't think this fixes the bug...

synapse/handlers/search.py Outdated Show resolved Hide resolved
synapse/handlers/search.py Show resolved Hide resolved
synapse/handlers/search.py Show resolved Hide resolved
richvdh
richvdh previously requested changes Dec 4, 2019
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

sorry anoa, but this feels like a very complicated way of solving the problem. How about this:

        # the initial room must have been known for us to get this far.
        predecessor_room = yield self.store.get_room_predecessor(room_id)

        while True:
            if not predecessor_room:
                # We have reached the end of the chain of predecessors
                break

            predecessor_room_id = predecessor["room_id"]

            # don't add it to the list until we have checked that we are in the room.
            try:
                next_predecessor_room = yield self.store.get_room_predecessor(
                    predecessor_room_id
                )
            except NotFoundError:
                # The predecessor is not a known room, so we are done here
                break
            
            historical_room_ids.append(predecessor_room_id)

             # and repeat
            predecessor_room = next_predecessor_room

        return historical_room_ids

@anoadragon453
Copy link
Member Author

predecessor_room_id = predecessor["room_id"]

We would still need to check that predecessor actually contained a room_id first, right?

@anoadragon453 anoadragon453 requested a review from richvdh December 5, 2019 13:26
@anoadragon453
Copy link
Member Author

I've implemented a very similar version, thanks :)

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgmt!

synapse/handlers/federation.py Outdated Show resolved Hide resolved
synapse/handlers/search.py Outdated Show resolved Hide resolved
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

synapse/handlers/federation.py Outdated Show resolved Hide resolved
@anoadragon453 anoadragon453 merged commit ea0f0ad into develop Dec 11, 2019
@anoadragon453 anoadragon453 deleted the anoa/fix_search_predecessor_room branch December 11, 2019 13:07
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'ea0f0ad41':
  Prevent message search in upgraded rooms we're not in (#6385)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants