Stabilize support for MSC4326: Device masquerading for appservices #19033
Conversation
| and DEVICE_ID_ARG_NAME in request.args | ||
| ): | ||
| effective_device_id = request.args[DEVICE_ID_ARG_NAME][0].decode("utf8") | ||
| # We only just set this so it can't be None! |
There was a problem hiding this comment.
This is pre-existing but I don't understand this comment. As far as I can tell, this is user-provided input
There was a problem hiding this comment.
I think .decode("utf8") should never return None (invalid unicode would throw an error) and the list is probably also guaranteed to only contain bytes, so the assertion should never fail
There was a problem hiding this comment.
I see. It makes sense that it can't be None.
Perhaps this was done to appease mypy but if I reveal_type(effective_device_id) before/after the assertion, even the types don't show it as being possible to be None as you explained.
synapse/api/auth/base.py:351: note: Revealed type is "Union[builtins.str, Any]"
synapse/api/auth/base.py:354: note: Revealed type is "Union[builtins.str, Any]"
I guess we can remove that check altogether.
There was a problem hiding this comment.
I'll leave it for now since the diff no longer touches that line at all
|
Thanks for moving this forward @tulir 🦙 |
2 participants
matrix-org/matrix-spec-proposals#4326
Note: the code references MSC3202, which is what MSC4326 was split off from. Only MSC4326 was accepted, MSC3202 wasn't yet.
Pull Request Checklist