Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid repeated upgrade of the same room #452

Open
KitsuneRal opened this issue Mar 24, 2019 · 11 comments
Open

Forbid repeated upgrade of the same room #452

KitsuneRal opened this issue Mar 24, 2019 · 11 comments
Labels
A-Client-Server Issues affecting the CS API enhancement A suggestion for a relatively simple improvement to the protocol wart A point where the protocol is inconsistent or inelegant

Comments

@KitsuneRal
Copy link
Member

As of this writing, the spec allows upgrading a room that already has a tombstone event. I don't think there was a provision to use the upgrade mechanism to fork rooms; so let's just disallow upgrading a room if it already has m.room.tombstone in its state.

@turt2live turt2live added enhancement A suggestion for a relatively simple improvement to the protocol wart A point where the protocol is inconsistent or inelegant A-Client-Server Issues affecting the CS API labels Mar 24, 2019
@turt2live
Copy link
Member

based on some internal troubleshooting over the last couple weeks, I'm not entirely convinced that we should do this. Although it's not great that people can rewrite the upgrade chain, the benefit is that when someone screws up the upgrade and it needs to be fixed it is incredibly useful to overwrite their disaster.

@anoadragon453
Copy link
Member

How exactly is the update screwed up though? Is it a bug or just the user messing with things?

Additionally, we could support rewriting the chain by redacting the tombstone event.

@turt2live
Copy link
Member

I've seen both cases equally. There's not much difference in redacting a tombstone and overwriting it.

@anoadragon453
Copy link
Member

I'd prefer redacting, assuming clients would handle it properly.

@anoadragon453
Copy link
Member

Have we discussed whether sending any events after a tombstone is allowed?

@turt2live
Copy link
Member

It's normally prevented by the power level change, but not strictly enforced. Reason being the rooms we want to upgrade are great at resetting state, I think

@anoadragon453
Copy link
Member

Is the server supposed to drop everyone to PL 0 in the room?

@turt2live
Copy link
Member

Nope, just raise the requirement to send events.

@anoadragon453
Copy link
Member

It raises it to 50, but that's going to prevent new events from being sent by mods/admins.

@turt2live
Copy link
Member

Correct. It's just meant to stop clients which don't support upgrades from using the old room

@ShadowJonathan
Copy link
Contributor

FWIW there is a sytest test asserting this: "Cannot send tombstone event that points to the same room" in 30rooms/60version_upgrade

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Client-Server Issues affecting the CS API enhancement A suggestion for a relatively simple improvement to the protocol wart A point where the protocol is inconsistent or inelegant
Projects
None yet
Development

No branches or pull requests

4 participants