Allow guest access to /sync #455
Conversation
| EventTypes.RoomHistoryVisibility | ||
| ) | ||
| if state and "history_visibility" in state.content: | ||
| defer.returnValue(state.content["history_visibility"] == "world_readable") |
There was a problem hiding this comment.
http://matrix.org/docs/spec/r0.0.0/client_server.html#redactions makes no mention of history_visibility being a preserved key in the event of a redacted m.room.history_visibility event. This will throw if the event is redacted.
There was a problem hiding this comment.
I don't think this will throw because we check if the history_visibility is in the content before accessing it.
This the same check as used for /events FWIW
There was a problem hiding this comment.
My bad; completely didn't see that you are indeed checking. Sorry!
Conflicts: synapse/api/filtering.py
| @@ -118,8 +120,14 @@ def on_GET(self, request): | |||
| except: | |||
| filter = FilterCollection({}) | |||
|
|
|||
| if is_guest and filter.list_rooms() is None: | |||
There was a problem hiding this comment.
It looks like filter.list_rooms() will return None iff there is no rooms key but there is no 0-length array check. We should probably guard against { "rooms": [] } as well.
There was a problem hiding this comment.
Does it matter? I'm not sure if passing an empty list of rooms is invalid or not here.
| if "rooms" in self.filter_json: | ||
| return list(set(self.filter_json["rooms"])) | ||
| else: | ||
| return None |
There was a problem hiding this comment.
We don't do None checks in a ton of places in this PR which call list_rooms(). Either do the null-guards or make non-existence return an empty list, else this is too error-prone IMO.
There was a problem hiding this comment.
We check that the list is non-None before calling into these handlers. I'd rather have a hard error here than a default.
|
Other than these points, LGTM though I don't have the full context to be confident on these changes. |
|
Okay, LGTM |
Allow guest access to /sync
Tested by matrix-org/sytest#122
TODO: