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

Permalinks should be able to decode and encode via servers. #300

Merged
merged 9 commits into from Mar 19, 2023

Conversation

Gnuxie
Copy link
Contributor

@Gnuxie Gnuxie commented Mar 7, 2023

Previously it was skipping &via and just joining via when there were more servers. And it also just would not attempt to parse via servers when there was no event id to go with a room.

Closes #299

Checklist

  • Tests written for all new code
  • Linter has been satisfied
  • Sign-off given on the changes (see CONTRIBUTING.md)

Previously it was skipping `&via` and just joining `via` when there were more servers.
And it also just would not attempt to parse via servers when there was no event id to go with a room.

turt2live#299
Signed-off-by: gnuxie <Gnuxie@protonmail.com>
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this pull request Mar 9, 2023
turt2live/matrix-bot-sdk#300
Element takes forever reviewing anything so i suspect
it'll be months before Travis even sniffs it.
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this pull request Mar 9, 2023
turt2live/matrix-bot-sdk#300
Element takes forever reviewing anything so i suspect
it'll be months before Travis even sniffs it.
@@ -4,27 +4,27 @@ describe('Permalinks', () => {
describe('forRoom', () => {
it('should generate a URL for a room ID', () => {
const roomId = "!test:example.org";
const expected = `https://matrix.to/#/${roomId}`;
const expected = `https://matrix.to/#/${encodeURIComponent(roomId)}`;
Copy link
Owner

Choose a reason for hiding this comment

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

not encoding is deliberate for compatibility with some clients: the SDK should be able to parse encoded and unencoded URLs, but should not be producing encoded URLs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, that's no problem, I've changed it

Historically, clients have not produced URIs which are fully encoded. Clients should try to interpret these cases to the best of their ability. For example, an unencoded room alias should still work within the client if possible.

The components of the matrix.to URI ( and ) are to be percent-encoded as per RFC 3986

Though the spec does imply the bad links should no longer be generated when it probably should not.

@Gnuxie Gnuxie requested a review from turt2live March 14, 2023 12:15
@turt2live turt2live enabled auto-merge (squash) March 19, 2023 20:25
Copy link
Owner

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

I guess I need to actually hit the approve button, hey

@turt2live turt2live merged commit 8f7ef01 into turt2live:main Mar 19, 2023
7 checks passed
@Gnuxie
Copy link
Contributor Author

Gnuxie commented Mar 20, 2023

Thanks!

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.

Permalinks.parseUrl cannot parse matrix.to urls for rooms with via parameters.
2 participants