StateFilter.must_await_full_state falls over if called on a StateFilters for membership events with a bogus state key

[DMRobertson]

IndexError: list index out of range
  File "synapse/http/server.py", line 366, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "synapse/http/server.py", line 572, in _async_render
    callback_return = await raw_callback_return
  File "synapse/rest/client/room.py", line 167, in on_GET
    data = await msg_handler.get_room_data(
  File "synapse/handlers/message.py", line 128, in get_room_data
    data = await self._storage_controllers.state.get_current_state_event(
  File "synapse/storage/controllers/state.py", line 482, in get_current_state_event
    state_map = await self.get_current_state(
  File "synapse/storage/controllers/state.py", line 464, in get_current_state
    state_map_ids = await self.get_current_state_ids(room_id, state_filter)
  File "synapse/storage/controllers/state.py", line 392, in get_current_state_ids
    if not state_filter or state_filter.must_await_full_state(self._is_mine_id):
  File "synapse/storage/state.py", line 563, in must_await_full_state
    if not is_mine_id(state_key):
  File "synapse/server.py", line 341, in is_mine_id
    return string.split(":", 1)[1] == self.hostname

Sentry:
https://sentry.tools.element.io/organizations/element/issues/124/events/d3c32827e3264d689cd3985618361192/?project=2&query=firstRelease%3A1.61.0rc1&sort=freq#tags

Matrix.org received a request of the form GET /_matrix/client/r0/rooms/ROOM_IDstate/m.room.member/foo@example.com. After checking the requester is in the room, we lookup the given state key in our server's view of the room.:

synapse/synapse/handlers/message.py

Lines 127 to 130 in a164a46

	if membership == Membership.JOIN:
	data = await self._storage_controllers.state.get_current_state_event(
	room_id, event_type, state_key
	)

This constructs a StateFilter looking for events matching that (type, state_key) pair.

synapse/synapse/storage/controllers/state.py

Lines 479 to 488 in 7c6b220

	async def get_current_state_event(
	self, room_id: str, event_type: str, state_key: str
	) -> Optional[EventBase]:
	"""Get the current state event for the given type/state_key."""
	key = (event_type, state_key)
	state_map = await self.get_current_state(
	room_id, StateFilter.from_types((key,))
	)
	return state_map.get(key)

We eventually fall into must_await_full_state. Because the event is a membership event, we check to see if the sender is local.

synapse/synapse/storage/state.py

Lines 561 to 566 in 7c6b220

	# otherwise, consider whose membership we are looking for. If it's entirely
	# local users, then we don't need to wait.
	for state_key in member_state_keys:
	if not is_mine_id(state_key):
	# remote user
	return True

But is_mind_id throws because the given state_key doesn't contain a colon.

synapse/synapse/server.py

Lines 339 to 341 in 1e45305

	def is_mine_id(self, string: str) -> bool:
	return string.split(":", 1)[1] == self.hostname

Options that spring to mind:

1. Add a try/except wrapper around is_mine_id. Any failure means the fancy faster join stuff is not available.
2. At the rest level, validate that the state_key is a matrix ID when the type is m.room.membership. The spec would allow us to return 404 Not Found in this case.

Related: #12489. I don't think this is directly related to faster joins; rather it's collateral damage? Might be of interest to @richvdh anyway.

[squahtx]

Considering only is_mine_id in isolation: I think we should make it return False for malformed user/room IDs or raise a ValueError.