Remove check on Room IDs having a domain component#467
Conversation
… be opaque strings
|
Actually, you know what? This is used in a couple different places for other sigil checks. Let me re-evaluate. I was to hasty. |
To be fair, "user" is only featured once while "room" is far more prevelant
There, that is more inclusive of uses other |
| if _, err = domainFromID(id); err != nil { | ||
| return | ||
| if kind != "room" { | ||
| // RoomIDs should always be an opaque string, so only do this check for other kinds of ID |
There was a problem hiding this comment.
This isn't true though for rooms <v12.
It would be better to assert that the room ID either:
- has a domain OR
- is exactly 43 characters of base 64 (we do a similar check in Synapse)
There was a problem hiding this comment.
Perhaps it would be better to just use parseAndValidateRoomID() and discard the returned RoomID?
There was a problem hiding this comment.
Come to think of it, the original premise of was discussed in the Complement room was about just removing the check for "TrustedJson" entirely. Perhaps should just go down that road instead of any of this?
2 participants
... as Room IDs should be opaque strings
Pull Request Checklist
Signed-off-by:
Jason Little <j.little@famedly.com>