Skip to content

Remove check on Room IDs having a domain component#467

Open
jason-famedly wants to merge 2 commits into
matrix-org:mainfrom
jason-famedly:jl/remove-domain-check-on-roomid
Open

Remove check on Room IDs having a domain component#467
jason-famedly wants to merge 2 commits into
matrix-org:mainfrom
jason-famedly:jl/remove-domain-check-on-roomid

Conversation

@jason-famedly

Copy link
Copy Markdown

... as Room IDs should be opaque strings

Pull Request Checklist

Signed-off-by: Jason Little <j.little@famedly.com>

@jason-famedly

Copy link
Copy Markdown
Author

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
@jason-famedly

Copy link
Copy Markdown
Author

Actually, you know what? This is used in a couple different places for other sigil checks. Let me re-evaluate. I was to hasty.

There, that is more inclusive of uses other kind == room. It probably would have been fine, but this is probably better

Comment thread event.go
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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants