Replace pako with native DecompressionStream#113
Open
kellenmurphy wants to merge 2 commits into
Open
Conversation
The HTTP-Redirect binding's raw-DEFLATE inflation is the only thing pako was
used for, and native DecompressionStream('deflate-raw') covers it well below
the extension's strict_min_version of 140.0. Removing it drops one of the two
third-party libraries AMO flagged for provenance in issue simplesamlphp#109.
DecompressionStream is async, so b64inflate() and parseSAML() become async.
parseSAML() moves out of the Request constructor, since RequestItem inspects
saml/samlart to decide which tabs exist, and both callers of addRequestItem()
now await it -- including the import path, where restoreFromImport()'s result
was previously assigned to a variable that would have become a
permanently-truthy Promise.
Adds the Playwright suite along with a spec that drives a genuine deflated
SAMLRequest end-to-end, plus jest coverage of the inflate itself including
latin1 byte semantics and Shibboleth's line-wrapped base64.
…als from new contexts
tvdijen
approved these changes
Jul 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the pako half of the plan discussed in #109. pako was only ever used in one place —
b64inflate()in SAMLTrace.js, inflating the HTTP-Redirect binding — and the browser's nativeDecompressionStream('deflate-raw')does the same job, so the library can go away entirely, along withlib/pako_inflate.min.js, its copy step in the release workflow, and its section in attribution.md. One less third-party file for an AMO reviewer to verify.The one wrinkle is that DecompressionStream is asynchronous where pako was synchronous, so
parseSAML()moves out of the Request constructor and is awaited inaddRequestItem()before the request is published to the list, and the import path in SAMLTraceIO.js awaits it likewise. The parameter-matching loop keeps its first-match-wins semantics, just written as a sequentialfor...ofinstead ofArray.some(). The manifest already requires Firefox 140, comfortably above wheredeflate-rawsupport landed.On tests: a new jest suite pins the
b64inflate()behaviour, including the subtle detail thatpako.inflateRawplusString.fromCharCodeproduced a latin1 byte-string rather than decoded UTF-8 — downstream parsing depends on that, so the replacement is asserted to match it byte for byte. There is also a Playwright end-to-end suite (npm run test:e2e, documented in test/E2E_TESTS.md) that loads the extension in a real Chromium and drives a genuine raw-deflated SAMLRequest through the query string. That spec is implementation-agnostic and passes against pako too, so it serves as a true before/after check for this change. The suite needs a display and reaches example.com over the network, so I deliberately did not wire it into CI — happy to add a workflow for it if you want one, but that seemed like your call to make. Two of its specs cover the #69/#106 export fixes from #108 rather than this change; they ride along because they share the harness and those fixes previously had no browser-level coverage.The second commit fixes a CI failure I hit on my fork: under jest on Node 26, a context created with
vm.runInNewContextno longer sees web globals likeatobandDecompressionStream, so the new test loads SAMLTrace.js through aFunctionwrapper in the test's own scope instead. SAMLTraceIO.test.js keeps the vm loader unchanged, since it touches no web globals.The branch is rebased onto main after #112. Jest is green on Node 26 in CI, and the Playwright suite passes 5/5 locally against the combined pako-removal and cdn-assets state.