fix(ui): encode oversized QR payloads instead of rendering nothing - #3656
fix(ui): encode oversized QR payloads instead of rendering nothing#3656MattDHill wants to merge 4 commits into
Conversation
|
@helix-nine rebase and test |
Action-result QR codes were drawn on a fixed 350px canvas at correction level M. Past 3391 alphanumeric characters level M has no version left, so the encoder threw and the modal rendered nothing; below that a dense code drew modules under 3px, too fine for a hardware wallet's camera. Draw the canvas oversized and let CSS pick the physical size, and drop to level L for payloads M cannot hold. All three call sites now share one component rather than repeating the fixed size.
…er count The level-`L` fallback keyed off `value.length > 3391`, which is the version-40 *alphanumeric* capacity. Anything containing a lowercase letter — a URL, a base64 key, a PEM certificate — encodes in byte mode, where level `M` tops out at 2331 bytes and `L` at 2953. A 2.4 kB mixed-case payload therefore stayed on `M`, the encoder still threw, and the modal still came up empty: the case this is meant to fix. Measured against qrcode 1.5.4, the version ng-qrcode wraps. Over every payload length to 4400 across byte, alphanumeric and PEM content the new rule renders 1289 lengths that previously rendered nothing and regresses none. The largest payload that draws goes 2331 -> 2953 bytes for mixed-case content and 2495 -> 3162 for a PEM body; alphanumeric is unchanged at 4296, where the old constant was already right.
0a5359f to
65665af
Compare
|
Rebased onto
Testing turned up a real problem, so there is a second commit on the branch. The
|
level L |
level M |
|
|---|---|---|
| alphanumeric | 4296 | 3391 |
| byte | 2953 | 2331 |
So a payload between 2332 and 3391 bytes of mixed-case content stayed on M, the encoder threw, and the modal still came up empty:
chars old rule new rule
2331 M -> v40 M -> v40
2332 M -> nothing L -> v36
2500 M -> nothing L -> v37
2953 M -> nothing L -> v40
2954 M -> nothing L -> nothing
A JSON blob of a URL, a key and a 1.6 kB certificate lands squarely in that band, which is the case the PR is named after.
The table in the description reproduces exactly on alphanumeric input (1660 → v24/v28, 1812 → v25/v29, 3484 → v36/throws), which is where the constant came from. The same payload lengths in byte mode are v30/v34, v31/v36, and throws at both levels.
Fix
Key the fallback on UTF-8 byte length against the byte-mode capacity instead:
readonly level = computed(() =>
utf8.encode(this.value()).length > 2331 ? 'L' : 'M',
)Byte mode is the worst case — qrcode's segment optimiser never does worse than encoding everything as bytes — so "≤ 2331 bytes fits at M" holds whatever the content is. Checked against 2331-byte strings built from ASCII, base64, PEM, URL, punctuation and 2/3/4-byte UTF-8 alphabets, plus 400 random mixed payloads. A mostly-alphanumeric payload in the 2332–3391 range now drops to L where M would have held it, which lowers the version and gives the modules more room, so it costs nothing here.
Swept every payload length up to 4400 across byte, alphanumeric and PEM content: 1289 lengths that rendered nothing now render, and none regress. The largest payload that draws goes 2331 → 2953 bytes for mixed-case content and 2495 → 3162 for a PEM body; alphanumeric stays at 4296, where the old constant was already right.
The changelog said "past 3391 characters"; it now says "past about 2.3 kB".
Still open
Above 2953 bytes the encoder still throws and the modal still renders nothing with no message — the ceiling moves, the silent-failure mode doesn't. Left alone since a user-facing string means five locale dictionaries and this is your PR, but it is the one part of "a modal that silently renders nothing is a bug" that isn't closed.
I also tried to settle whether image-rendering: pixelated helps or hurts, by rasterising codes and decoding them back through jsQR under nearest-neighbour vs box downsampling. The results didn't separate the two cleanly and a software decoder on a synthetic raster isn't a camera, so I have nothing to report either way. What did come through consistently is the part the PR is built on: the same code that fails to decode at 350px decodes at 480px.
@dr-bonez the force-push dismissed your approval — dismiss_stale_reviews_on_push is on for master, and the rebase you asked for needed one.
65665af to
03eff13
Compare
|
Re-tested the rewritten branch ( Two things worth a look before this goes in.
|
| payload | len | rule picks | result |
|---|---|---|---|
ActionResBigQr (the mock) |
3618 | L |
v37 ✅ |
| same UR, lowercased | 3618 | L |
blank — 3618 B > L's 2953 |
{url, key, PEM cert} |
2529 | M |
blank — but v36 at L |
That last row is the payload the first description opened with, and it's the one that stings: 2529 characters is under 3391, so the rule leaves it on M, the encoder throws, and the dialog is still empty — while L would have drawn it fine. Passwords, onion URLs, PEM blocks and JSON results are all byte mode.
Gating on byte length against the byte-mode number fixes it:
readonly level = computed(() =>
utf8.encode(this.value()).length > 2331 ? 'L' : 'M',
)Byte mode is the worst case — qrcode's segment optimiser never does worse than encoding everything as bytes — so "≤ 2331 bytes fits at M" holds for any content, multibyte UTF-8 included. Sweeping every length to 4400 across byte, alphanumeric and PEM content, it renders 1289 lengths that currently come up blank and regresses none; the mock keeps encoding at L exactly as it does now.
A second fixture with the same shape but lowercase would pin the difference — though note that at 3618 characters lowercase is past L's byte ceiling too, so a realistic one wants to be ~2.5 kB.
The rebase came off
03eff13 is based on 6a1f75d again, six commits behind master, and GitHub has the PR as DIRTY — projects/start-os/CHANGELOG.md conflicts with the "Image upgrades verify their checksum again" entry that landed in the same ### Fixed block. Same conflict as before; keeping both entries resolves it.
Happy to push either change, or leave the branch to you — say which. Not pushing unasked, since that's how the last commit got crossed.
The canvas was drawn at 1024px and sized by a .g-qr utility passed through ng-qrcode's styleClass. The class never reached the canvas, so the dialog rendered a 1024px code clipped to its bounds instead of a scaled one. Reverts to the 350px canvas, keeping the correction-level fallback that fixes the blank modal. The density it was chasing does not help the case that motivated it either: a Jade scans at 320x240, so a symbol has to resolve within 240 pixels — about a version-13 code — and a certificate-bearing payload is version 28 or higher regardless of how large it is drawn.
The QR modal's failure mode only shows up past correction level M's 3391-character capacity, which nothing in the mock data reached. Adds a 'Show Large QR' action on bitcoind returning a 3618-character payload — long enough to throw at M, short enough to encode at L. runAction ignored actionId and always returned the same fixture; it now dispatches so the new action can return its own.
03eff13 to
2647578
Compare
MattDHill
changed the title
|
Tested Behaviour at the tip, against
1289 payload lengths that used to come up blank now render, none regress. Nothing further from me — @dr-bonez this needs your re-approval, the earlier one was dismissed by the force-pushes. |
3 participants
Action-result QR codes are encoded at correction level
M, which runs out of versions well before the payloads services actually hand it. When that happensqrcodethrowsThe amount of data is too big to be stored in a QR Code, and the dialog opens empty — no code, no message, just a console error.Level
Lcarries more, so oversized payloads fall back to it.Picking the level
The threshold is byte-mode capacity, not character count. Anything containing a lowercase letter — a URL, a base64 key, a PEM certificate — encodes in byte mode, where level
Mtops out at 2331 bytes andLat 2953. Keying off the version-40 alphanumeric capacity (3391) left exactly those payloads onM, still throwing.Measured against
qrcode1.5.4, the versionng-qrcodewraps: across every payload length to 4400 in byte, alphanumeric and PEM content, the rule renders 1289 lengths that previously rendered nothing and regresses none. The largest payload that draws goes 2331 → 2953 bytes for mixed-case content and 2495 → 3162 for a PEM body; alphanumeric is unchanged at 4296.Changes
app-qrcomponent owns the level rule. The three portal call sites — the QR dialog, action-success single and member — each hard-codedsize="350"and the default level; they now share one component. Rendering is unchanged: same 350px canvas as before.big-qrmock action on bitcoind returns a 3618-character payload, long enough to throw atMand short enough to encode atL.runActionignoredactionIdand always returned the same fixture, so it now dispatches.npm run start:ui→ Bitcoin Core → Actions → Show Large QR is a before/after in one click.Not in scope
An earlier revision of this branch also drew the canvas oversized and let CSS size it, to get more pixels per module. That is reverted — the class never reached the canvas through
ng-qrcode'sstyleClass, so it rendered a 1024px code clipped to the dialog.It would not have helped the case that motivated it in any event: a Blockstream Jade scans at 320×240, so the symbol must resolve within 240 pixels — roughly a version-13 code at three pixels per module, version 23 even at the two-pixel floor. A certificate-bearing payload is version 28 or higher no matter how large it is drawn, because the limit is how much sensor the symbol gets once it fills the frame.
🤖 Generated with Claude Code