Skip to content

fix: Correctly percent-encode addresses in securejoin invite codes - #8747

Merged
Hocuri merged 2 commits into
mainfrom
hoc/fix-securejoin-qr-encoding
Sep 24, 2026
Merged

Hocuri merged 2 commits into
mainfrom
hoc/fix-securejoin-qr-encoding

Conversation

@Hocuri

@Hocuri Hocuri commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes how addresses in securejoin invite codes (i.e. QR codes and invite links) are percent-encoded.

Before this PR,

  • @ in the addresses was percent-encoded. This makes the invite code harder to read, and is not necessary; at least, every software we tested correctly handled links that contain @ (Signal, WhatsApp, Telegram, Thunderbird, Delta Chat on Android, iOS, and Desktop)
  • But, the first address in the r= parameter of a securejoin link was not percent-encoded at all. This was a sneaky bug caused by using the reduce function; I always find it hard to follow code that uses reduce (and similar functions like fold), and apparently others have the same problem since neither @j-g00da nor @link2xt noticed the problem when implementing & reviewing the PR that introduced the bug.

With this PR:

  • @ is allowed in addresses in securejoin invite codes
  • all addresses are percent-encoded
  • reduced is not used anymore

@Hocuri
Hocuri requested a review from link2xt September 24, 2026 15:16
Comment thread src/securejoin/securejoin_tests.rs Outdated
@link2xt

link2xt commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

I always find it hard to follow code that uses reduce

Reduce is sometimes fine, but not for doing something with the arguments at the same time. E.g. if you need to concatenate all the strings (possibly with the separator), or union all the sets or something like this, then fold/reduce is doing the expected thing.

Co-authored-by: l <link2xt@testrun.org>
@Hocuri
Hocuri enabled auto-merge (squash) September 24, 2026 20:44
@Hocuri
Hocuri merged commit 22578ea into main Sep 24, 2026
30 checks passed
@Hocuri
Hocuri deleted the hoc/fix-securejoin-qr-encoding branch September 24, 2026 20:58
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