MSC1884: Proposal to replace slashes in event IDs

[richvdh]

Rendered

[Half-Shot]

It seems like a good idea to broaden the proposal to fix all affected identifiers, rather than doing just the one that has caused the most trouble. What's the reason for not doing so?

[richvdh]

like https://github.com/matrix-org/matrix-doc/pull/1884/files#diff-423836d994b22ebd6bfac8329976ab5eR70 ? Just that it's a big job.

[richvdh]

Specifically: if we were to do so, I think it would be a multi-month project, and until it's done it doesn't really deliver any value. Even when it is done, you still have to remember to URL-encode your parameters.

[uhoreg]

IMHO, we should either allow slashes in event IDs, or we should commit to disallowing slashes everywhere. We don't have to do it right away, but I think we should at least commit to it, if we're going to disallow slashes in event IDs. However, given that people are already using slashes in their user IDs, that, ahem, pony may have already bolted.

[Half-Shot]

I mean, there are lots of user IDs that are invalid under the current grammar, so I'm not sure it matters too much.

[richvdh]

we have a terrible history of saying that we are going to do things and not following through. I do not believe that even with the best will in the world that the other identifiers will get fixed. There will always be more pressing things to deal with, and meanwhile client developers really need to url-encode their path parameters.

Basically I just don't see what this solution buys us. Matthew says "every client developer is going to trip over and curse us", but critically this doesn't avoid the need to url-encode your path parameters, because of all those other cases. What it means instead is that you are likely to get all the way to releasing your client and having active users discovering edge-cases before you find the problem, rather than finding it during development. I know that if I were a client developer I'd do a lot less cursing if I found a bug myself rather than having to figure out why it's not working for a user.

[ara4n]

My rationale is this:

• Today (v1) you don't in practice have to escape event IDs in URIs, so client developers don't bother and have perfectly working clients which don't do so.
• Tomorrow (v4) the spec forces you to, and so those clients break. This is frustrating, as an encoding could easily have been picked that didn't need to be escaped, and the client developers curse the backend developers for optimising for their view of the world.

Entirely separately we have the fact that clients should be escaping room IDs and user IDs today - and it's true to say that clients which don't are timebombs waiting for breakage when /dev/ponies comes along. However, I think we should entirely decouple this from the event_id format question, and make it abundantly clear that right now clients MUST escape room & user IDs in URIs (and then relax this in future if/when we switch those IDs to be URI safe).

In other words, today's client developers are empirically not url-encoding each and every thing that goes into their URLs - instead they are only escaping the things they know need encoding (e.g. they escape room aliases, but they don't escape event_ids). So: please don't force them to escape something that shouldn't need to be escaped.

[richvdh]

why would you url-encode some parameters and not others? I mean, sure, you could write a client that way, but it sounds like a funny thing to do.

(e.g. they escape room aliases, but they don't escape event_ids).

citation needed?

[ara4n]

I mean, sure, you could write a client that way, but it sounds like a funny thing to do.

Perhaps, but empirically that's what people are doing.

(e.g. they escape room aliases, but they don't escape event_ids).

From quickly flipping through https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/base-apis.js, it looks like every HTTP request in the js-sdk manually decides whether it needs to URI escape its parameters or not. As it happens, the implementor was cautious and escaped the event IDs anyway - but i am sure others will not have done so, and being told "well you should have escaped it anyway" will be cold comfort to people asking "why did you break my app when you could have avoided it?"

[richvdh]

Perhaps, but empirically that's what people are doing.

Again: I'm doubtful that there are many (any?) instances where user-ids and room aliases are correctly encoded but event-ids are not.

From quickly flipping through https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/base-apis.js, it looks like every HTTP request in the js-sdk manually decides whether it needs to URI escape its parameters or not.

it looks very much to me as if every request which uses path parameters uri-encodes them.

"why did you break my app when you could have avoided it?"

Again, it was already broken, and I do not believe that special-casing event ids really fixes that.

[KitsuneRal]

Note that it also means 48% of to-come Matrix URIs that strive to be kinda human-readable and more or less easily human-constructible. Consider people trying to copy-paste ids into a URL, e.g.

[ara4n]

@KitsuneRal your eg went missing...

[KitsuneRal]

It didn't :) Read "E.g., consider people trying..."

[turt2live]

Apparently I didn't say it a month ago, but this looks great to me.

[ara4n]

@mscbot fcp merge

calling fcp to see if anyone wants this other than me. my reasons for wanting it is that it is an unnecessary pain that almost every client developer is going to trip over and curse us for. ideally we would spec it for room ids and user ids at the same time and avoid mandating escaping in urls in general, but at least this doesn’t needlessly break clients in a way that client developers will consider avoidable.

[mscbot]

Team member @ara4n has proposed to merge this. The next step is review by the rest of the tagged people:

• @KitsuneRal
• @anoadragon453
• @ara4n
• @dbkr
• @erikjohnston
• @richvdh
• @turt2live
• @uhoreg

Concerns:

• this doesn't fix anything resolved by MSC1884: Proposal to replace slashes in event IDs #1884 (comment)
• update the proposal for a conclusion on the above discussion resolved by MSC1884: Proposal to replace slashes in event IDs #1884 (comment)

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[richvdh]

this point makes me incredibly sad, but I think I can live with it.

[richvdh]

@mscbot concern this doesn't fix anything

#1884 (comment), basically. If we can actually fix the other identifiers rather than just saying we'd like to do so, I'll drop my opposition here.

[erikjohnston]

I really really would prefer it if we were consistent with the encoding we used for things. If we're going to go down this route I would really rather we did this across the board (or at least agreed that that is where we should aim for). Just because a given encoded value isn't currently used in a URL doesn't mean it won't be; certainly I wouldn't be surprised if some of the newer E2E APIs ended up with keys and things in the URL.

I think the worst of all worlds we could end up in is where we have a mix of encodings and still require people to URL encode their parameters. Whether that is due to existing fields getting used in new APIs, or other server implementations coming along and allowing slashes in identifiers, or whatever.

[erikjohnston]

ideally we would spec it for room ids and user ids at the same time and avoid mandating escaping in urls in general...

Wouldn't this effectively mean that we could never use unicode for user IDs? I know that we currently don't allow them but I thought we were wanting to support them at some stage in the future?

... but at least this doesn’t needlessly break clients in a way that client developers will consider avoidable.

There is a perverse effect here that having slashes in identifiers quickly forces clients to correctly URL encode parameters, which while annoying for existing clients may actually be easier for newer client implementers. URL encoding parameters is such a standard thing to do that I don't think people will get annoyed at us if they realise almost immediately that they need to remember to do it, but it will be annoying if it works 99% but then breaks the odd time they do actually need to do so.

Effectively, I imagine this is short term pain for long term gain, in the sense that new clients will be written correctly from the start.

[ara4n]

I just spoke this through IRL with @erikjohnston and @neilisfragile and came to realise that a lot of the debate here (e.g. whether should pander to client developers who don't encode URIs) is specious.

The whole question here boils down to selecting between two trade-offs:

Either:

1. We stick with using normal base64 everywhere, and oblige all client developers, casual API users (e.g. folks using curl or postman or swagger style UIs), ops folk who are grepping logs, etc, to have to remember to urlencode/decode these IDs - manually going through switching / for %2F when needed or vice versa. Every time you copy-paste an unescaped ID from a JSON blob or a debugger into curl, or a browser URL bar, etc, you're going to have to manually go through escaping the slashes. Meanwhile the logs fill up with ugly %2Fs everywhere, and your columns and loglines are suddenly unnecessarily variable length, making them hard to scan. The situation gets worse as we switch to using keys for more IDs as per [WIP] MSC1228: Removing MXIDs from events #1228. The CS API developers curse us for not using url-safe base64 encoding in our URLs, and wonder why the protocol is so perversely developer unfriendly. The fact that human-generated IDs (e.g. mxids, room aliases) may still need to be manually escaped is irrelevant, as it's obvious that human-generated stuff should be escaped - but for IDs which are computer generated, why not skip the hassle?

or:

2. We oblige all E2EE and Federation developers to have to keep tabs on whether a given base64 string is url-safe or not url-safe encoded when selecting an encoder/decoder. E2EE related base64 is probably stuck as non-url-safe for the forseeable as we don't have an easy way to bump the ID format for the devices API, so we'd likely be stuck with a mix of formats.

Now, I think that many many more people are going to be bitten by the inconveniences of the first tradeoff than will be by the second. Particularly, the only people who may be bitten by the second are expert server and encryption developers, who are relatively uncommon, and also sophisticated enough to be able to check whether a given string is base64 or url-safe base64.

Having expressed it like this, I strongly think that we should optimise for supporting the CS API developers over the SS/E2EE developers, and adopt the following rule of thumb: all base64 presented in the CS API (ignoring E2EE) should be URL safe encoded. This starts with event IDs, and then in the #1228 future will include room ids, user ids, etc too.

Meanwhile, the E2EE API is stuck with pure base64, but typical client developers shouldn't need to go anywhere near it (especially with https://github.com/matrix-org/pantalaimon), and the SS API can use whatever mix makes most sense, and yes - it makes it a bit harder for server & encryption developers to remember which encoding to use.... but this is a small price to pay for making the CS API easier to play with.

Hopefully this accurately represents the discussion, and that @erikjohnston agreed in the end on the conclusion.

If this sounds good to other people, I suggest we indicate it by voting on entering FCP, and then fix up the MSC to reflect the rationale (and the intention in general to standardise on URL-safe base64 in the CS API), and then move on.

cc @dbkr @uhoreg @erikjohnston @richvdh as the remaining checkboxes.

[richvdh]

I think you're still missing the point of my argument, so your dichotomy is false.

My concern is not really with having two different base64 encodings. That is ugly and annoying, but I wouldn't see it as a hard blocker if I thought it bought us anything.

What does concern me is that, given you have to url-encode room aliases, room ids, user ids, state keys, device ids, transaction ids, etc, excluding event ids from that list doesn't buy you much except (1) timebombs for developers who fail to grok url-encoding and (2) the aforementioned ugliness.

Further, I think some of your arguments against regular base64 are flawed:

ops folk who are grepping logs
Meanwhile the logs fill up with ugly %2Fs everywhere

The logs are already full of %3As. I don't buy this.

your columns and loglines are suddenly unnecessarily variable length

Which columns? And loglines are already variable length?

The CS API developers curse us for not using url-safe base64 encoding in our URLs, and wonder why the protocol is so perversely developer unfriendly. The fact that human-generated IDs (e.g. mxids, room aliases) may still need to be manually escaped is irrelevant, as it's obvious that human-generated stuff should be escaped

Is it? You keep saying things like this, but I don't see any evidence of clients which correctly url-encode mxids and room aliases but not event ids. Nor do I agree that having to url-encode your path parameters constitutes a perversely developer unfriendly protocol, however if it does then, as I keep saying: changing the event ID format doesn't fix this.

I guess ultimately this isn't a mountain that I'm prepared to die on, and I accept that there are some advantages to changing the format of event IDs. I just think that you are overstating them, and I really don't think that one of those advantages is saving client developers from having to URL-encode things.

[ara4n]

What does concern me is that, given you have to url-encode room aliases, room ids, user ids, state keys, device ids, transaction ids, etc, excluding event ids from that list doesn't buy you much except (1) timebombs for developers who fail to grok url-encoding and (2) the aforementioned ugliness.

I totally agree that client developers should be url-encoding the strings they put in their URLs, and i'm not suggesting they should cut the corner by skipping it for event-ids. This was my first sentence at #1884 (comment).

Meanwhile the logs fill up with ugly %2Fs everywhere

The logs are already full of %3As. I don't buy this.

According to RFC3986 you don't have to escape colons in the path part of URLs.
Obviously some url escaping functions do this anyway, but worst case that ends up with constant-width ugliness in the strings, which is annoying, but given it's optional anyway, it doesn't matter if you copy-paste $1234678:foo.com into curl and hand it a raw colon rather than a %3A.

However, if you copy-paste $hPQNcabIABgGnx3/ACv/jmMmiQHoeFfu into curl or postman or a matrix.to URL in your browser's URL bar or a riot/web URL bar you have to manually go through turning the slashes into %2Fs. This is going to get very stale, very quickly, and every time somebody does it they will swear at whoever thought it was a good idea to not use URL-safe base64 for identifiers found in URLs.

your columns and loglines are suddenly unnecessarily variable length

Which columns

Anywhere you are dealing with a stack of event IDs. or msc1228-user IDs or room IDs, where you are building them into a SQL query or a commandline arg or otherwise manipulating them in a text editor. Rather than being a nice neat stack of 32-character IDs, they're arbitrarily meandering around all over the place.

You keep saying things like this, but I don't see any evidence of clients which correctly url-encode mxids and room aliases but not event ids.

But that was my point!! It's quite possible that all the clients might and could and should correctly url-encode their event IDs. The reason to have URL-safe IDs in URLs is because it makes life way easier for the humans whilst working on the clients, rather than anything to do with letting client developers cut corners in their escaping logic.

Nor do I agree that having to url-encode your path parameters constitutes a perversely developer unfriendly protocol, however if it does then, as I keep saying: changing the event ID format doesn't fix this.

I never said that having to url-encode your path parameters constitutes a perversely developer unfriendly protocol. But a protocol that wilfully picks the non-URL-safe format of an encoding to use in URLs is surely unquestionably perverse, from a CS API developer perspective. And changing the event ID format to be URL safe does fix that.

and I really don't think that one of those advantages is saving client developers from having to URL-encode things.

I did not say it was.

Please can we FCP this?

[ara4n]

If this sounds good to other people, I suggest we indicate it by voting on entering FCP, and then fix up the MSC to reflect the rationale (and the intention in general to standardise on URL-safe base64 in the CS API), and then move on.

@erikjohnston points out that this isn't what the checkboxes are for, so ignore that.

[richvdh]

What does concern me is that, given you have to url-encode room aliases, room ids, user ids, state keys, device ids, transaction ids, etc, excluding event ids from that list doesn't buy you much except (1) timebombs for developers who fail to grok url-encoding and (2) the aforementioned ugliness.

I totally agree that client developers should be url-encoding the strings they put in their URLs, and i'm not suggesting they should cut the corner by skipping it for event-ids. This was my first sentence at #1884 (comment).

Your first sentence was "I just spoke this through IRL with @erikjohnston and @neilisfragile and came to realise that a lot of the debate here (e.g. whether should pander to client developers who don't encode URIs) is specious."

You've certainly previously argued that one reason for favouring url-safe base64 is to avoid breaking clients which do not correctly url-encode their parameters (eg here). But ok, if we're now agreed that this is not a valid argument, \o/.

Meanwhile the logs fill up with ugly %2Fs everywhere

The logs are already full of %3As. I don't buy this.

According to RFC3986 you don't have to escape colons in the path part of URLs.
Obviously some url escaping functions do this anyway,

s/some/most/, I'd suggest. Certainly enough that it means you need to handle %-decoding when you are grokking logs.

but worst case that ends up with constant-width ugliness in the strings, which is annoying,

Given, as you point out, it's optional, I don't see how it's constant-width; and in any case, why is its width relevant in logs?

but given it's optional anyway, it doesn't matter if you copy-paste $1234678:foo.com into curl and hand it a raw colon rather than a %3A.

I'm failing to see the relevance to the logs containing %-encoded characters here.

However, if you copy-paste $hPQNcabIABgGnx3/ACv/jmMmiQHoeFfu into curl or postman or a matrix.to URL in your browser's URL bar or a riot/web URL bar you have to manually go through turning the slashes into %2Fs. This is going to get very stale, very quickly, and every time somebody does it they will swear at whoever thought it was a good idea to not use URL-safe base64 for identifiers found in URLs.

How often do you actually build a URL including an event id by hand? I often cut-and-paste event ids into postgres queries and the like, but I think I could count the times I've manually put one in a URL on the fingers of one hand.

your columns and loglines are suddenly unnecessarily variable length

Which columns

Anywhere you are dealing with a stack of event IDs. or msc1228-user IDs or room IDs, where you are building them into a SQL query or a commandline arg or otherwise manipulating them in a text editor. Rather than being a nice neat stack of 32-character IDs, they're arbitrarily meandering around all over the place.

why would you have a stack of %-encoded event ids? I mean, I suppose you might fish them out of an access log, but I'd have thought by the time you've done that you'd probably want to stick them through perl -pe 's/%(..)/chr hex $1/eg' anyway, which ok is slightly tedious, but that regex is hardwired to my muscle memory and feels considerably less tedious than whatever other log grepping I'm doing.

You keep saying things like this, but I don't see any evidence of clients which correctly url-encode mxids and room aliases but not event ids.

But that was my point!! It's quite possible that all the clients might and could and should correctly url-encode their event IDs. The reason to have URL-safe IDs in URLs is because it makes life way easier for the humans whilst working on the clients, rather than anything to do with letting client developers cut corners in their escaping logic.

If that was your point, then I think you've changed your stance since previously, and #1884 (comment) really didn't make it clear, but okm fine.

Nor do I agree that having to url-encode your path parameters constitutes a perversely developer unfriendly protocol, however if it does then, as I keep saying: changing the event ID format doesn't fix this.

I never said that having to url-encode your path parameters constitutes a perversely developer unfriendly protocol. But a protocol that wilfully picks the non-URL-safe format of an encoding to use in URLs is surely unquestionably perverse, from a CS API developer perspective. And changing the event ID format to be URL safe does fix that.

and I really don't think that one of those advantages is saving client developers from having to URL-encode things.

I did not say it was.

Well, you certainly did previously.

Please can we FCP this?

I think it's inappropriate to repeatedly demand FCP while there is still a very active conversation ongoing.

[ara4n]

But ok, if we're now agreed that this is not a valid argument, \o/.

great.

but worst case that ends up with constant-width ugliness in the strings, which is annoying,

Given, as you point out, it's optional, I don't see how it's constant-width; and in any case, why is its width relevant in logs?

I think it boils down to a matter of aesthetics and legibility. As a human scanning a log file, the less variation in the fields in the logfile the better, from my perspective.

but given it's optional anyway, it doesn't matter if you copy-paste $1234678:foo.com into curl and hand it a raw colon rather than a %3A.

I'm failing to see the relevance to the logs containing %-encoded characters here.

I was trying to point out that the fact that colons are optionally URL encoded doesn't hinder developers in general, so the implication of "colons already get encoded, so why does it matter if slashes do" is invalid.

However, if you copy-paste $hPQNcabIABgGnx3/ACv/jmMmiQHoeFfu into curl or postman or a matrix.to URL in your browser's URL bar or a riot/web URL bar you have to manually go through turning the slashes into %2Fs. This is going to get very stale, very quickly, and every time somebody does it they will swear at whoever thought it was a good idea to not use URL-safe base64 for identifiers found in URLs.

How often do you actually build a URL including an event id by hand? I often cut-and-paste event ids into postgres queries and the like,

So whenever you cut-and-paste an event ID from an HTTP log into a postgres query you're going to have to manually unescape it. For no reason.

but I think I could count the times I've manually put one in a URL on the fingers of one hand.

I spend way too much time currently going through DBs, finding IDs, and putting them into the admin API - basically every time anyone reports abuse on matrix.org, or any time i'm doing bridge maintenance or otherwise using the CS API based on data from the DB (or conversely using IDs from HTTP logs in the DB). I'm guessing about 50 times so far this year? Someone like half-shot probably even more so.

your columns and loglines are suddenly unnecessarily variable length

Which columns

Anywhere you are dealing with a stack of event IDs. or msc1228-user IDs or room IDs, where you are building them into a SQL query or a commandline arg or otherwise manipulating them in a text editor. Rather than being a nice neat stack of 32-character IDs, they're arbitrarily meandering around all over the place.

why would you have a stack of %-encoded event ids?

Because you've just grepped them out of an access log, or your client's HTTP logs, or your URLs in Chrome Dev Tools, etc.

I mean, I suppose you might fish them out of an access log, but I'd have thought by the time you've done that you'd probably want to stick them through perl -pe 's/%(..)/chr hex $1/eg' anyway

THIS.

which ok is slightly tedious, but that regex is hardwired to my muscle memory and feels considerably less tedious than whatever other log grepping I'm doing.

Optimising for tech architects who have perl oneliners hardwired to their muscle memory is precisely what I mean by perverse protocol design. We have the option to avoid that pain, let's use it.

Please can we FCP this?

I think it's inappropriate to repeatedly demand FCP while there is still a very active conversation ongoing.

As per #1884 (comment) i've given up asking people to vote to express their position; will wait to see if there is any hope of getting aligned first.

[richvdh]

I think it boils down to a matter of aesthetics and legibility. As a human scanning a log file, the less variation in the fields in the logfile the better, from my perspective.

I'm afraid I'm totally unconvinced that a few %-encoded characters add meaningfully to the variation in logfile lines.

I was trying to point out that the fact that colons are optionally URL encoded doesn't hinder developers in general, so the implication of "colons already get encoded, so why does it matter if slashes do" is invalid.

But it is valid when we are talking about grokking logfile lines.

So whenever you cut-and-paste an event ID from an HTTP log into a postgres query you're going to have to manually unescape it. For no reason.

This is a slightly different argument to your previous one, which was about C&P in the other direction. Anyway, you have to unescape v1 event IDs currently, but ok, I can see that it would be nice to fix this.

I spend way too much time currently going through DBs, finding IDs, and putting them into the admin API -

Which admin APIs take event IDs, ooi?

Optimising for tech architects who have perl oneliners hardwired to their muscle memory is precisely what I mean by perverse protocol design. We have the option to avoid that pain, let's use it.

Well, I think that if you attempt to grok logs without making an attempt to unescape %-encoded identifiers, you're going to have a bad time.

To be clear, my position remains:

I accept that there are some advantages to changing the format of event IDs. I just think that you are overstating them.

Now that we've dismissed the "clients shouldn't be forced to url-encode" argument, I think the stated advantages are:

• it's harder to grok logs if they contain %-encoded characters.
• you can't cut-and-paste an event ID into a URL, which is true, but as I question above: how often do you actually need to do that?

Ultimately it's a judgement call as to whether those advantages outweigh the disadvantages of making the change. I'm not convinced they do, but as I've said previously: it's not a mountain I'm prepared to die on, so, with my reservations recorded:

@mscbot resolve this doesn't fix anything

[turt2live]

@mscbot concern update the proposal for a conclusion on the above discussion

[ara4n]

Okay, I've updated the MSC to try to reflect the conclusions from the above discussions with @richvdh and @erikjohnston.

Rather than going through the rigmarole of cancelling FCP proposal and starting over again, please can we just update the checkboxes to match current thoughts on the updated proposal?

[mscbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[anoadragon453]

@mscbot reviewed

[mscbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

[turt2live]

our process is to merge the PR (not the MSC) when it clears FCP, so doing that

[turt2live]

Proof of concept is over at matrix-org/synapse#5210 - marking as "spec-pr-missing" because by the time we get around to writing the spec the change should be tested enough to be considered stable. If it ends up being not stable, we can always turn this MSC down the path of revisions.

[turt2live]

Spec PR: #2019

[turt2live]

merged 🎉