Repository navigation
Perf/decode response encodings - #706
SanderDeclerck wants to merge 3 commits into
Conversation
| * @returns {Promise<Buffer>} | ||
| */ | ||
| function decodeGzip(buffer) { | ||
| return gunzipAsync(buffer, { maxOutputLength: MAX_DECODED_BODY_BYTES }); |
There was a problem hiding this comment.
🟡 Medium - Over-limit gzip metadata bypasses minimum-age filtering
A registry can return a gzip metadata response whose decoded size exceeds the new 256 MiB limit. The decoder rejects it, but the response handler catches that error and forwards the original compressed bytes without invoking the metadata interceptor, so npm or pip receives unfiltered version/file metadata and can select releases that the minimum-age rewrite was meant to remove.
Show fix
Treat an over-limit decode as a failed upstream response and do not forward it to the package manager, or use a bounded streaming rewrite path that can safely enforce the limit while still applying the metadata checks. Add an integration test proving that an over-limit gzip response cannot reach the client as an unmodified metadata response.
More info - Reply on this comment to give feedback or ignore the issue.
There was a problem hiding this comment.
Partially fixed in commit 7f82be1.
This issue remains because packages/safe-chain/src/registryProxy/mitmRequestHandler.js:270-278 catches the MAX_DECODED_BODY_BYTES rejection from decodeBody and sends status/headers plus originalBuffer, allowing the over-limit compressed metadata to reach the client unmodified.
ac62603 to
7f82be1
Compare
| export function supportedAcceptEncoding(acceptEncoding) { | ||
| const offered = String(acceptEncoding ?? "") | ||
| .split(",") | ||
| .map((token) => token.trim()) | ||
| .filter(Boolean); | ||
| const supported = offered.filter((token) => | ||
| Object.hasOwn(decoders, token.split(";")[0].trim().toLowerCase()) | ||
| ); | ||
| return supported.length > 0 ? supported.join(", ") : "identity"; | ||
| } |
There was a problem hiding this comment.
🔵 Low - Accept-Encoding wildcard with identity refusal is rewritten to an unacceptable encoding
A client that sends Accept-Encoding: *;q=1, identity;q=0 can accept gzip, but supportedAcceptEncoding drops the wildcard and falls back to identity. The proxy therefore asks the registry for identity and forwards an identity metadata response, violating the client's explicit refusal and causing clients that enforce their encoding negotiation to abort the metadata request or installation.
Show fix
| export function supportedAcceptEncoding(acceptEncoding) { | |
| const offered = String(acceptEncoding ?? "") | |
| .split(",") | |
| .map((token) => token.trim()) | |
| .filter(Boolean); | |
| const supported = offered.filter((token) => | |
| Object.hasOwn(decoders, token.split(";")[0].trim().toLowerCase()) | |
| ); | |
| return supported.length > 0 ? supported.join(", ") : "identity"; | |
| } | |
| export function supportedAcceptEncoding(acceptEncoding) { | |
| const entries = String(acceptEncoding ?? "").split(",").map((token) => { | |
| const [name, ...params] = token.trim().split(";"); | |
| const q = params.find((param) => param.trim().toLowerCase().startsWith("q=")); | |
| return { token, name: name.trim().toLowerCase(), q: q ? Number(q.split("=")[1]) : 1 }; | |
| }).filter(({ name }) => name); | |
| const supported = entries.filter(({ name, q }) => Object.hasOwn(decoders, name) && q > 0); | |
| if (supported.length) return supported.map(({ token }) => token).join(", "); | |
| const wildcard = entries.find(({ name, q }) => name === "*" && q > 0); | |
| const blocked = new Set(entries.filter(({ q }) => q === 0).map(({ name }) => name)); | |
| const wildcardEncoding = wildcard && Object.keys(decoders).find((name) => !blocked.has(name)); | |
| if (wildcardEncoding) return `${wildcardEncoding}${wildcard.q === 1 ? "" : `;q=${wildcard.q}`}`; | |
| const identity = entries.find(({ name }) => name === "identity"); | |
| if (!identity || identity.q > 0) return "identity"; | |
| throw new Error("No acceptable content-encoding supported"); | |
| } |
More info - Reply on this comment to give feedback or ignore the issue.
7f82be1 to
8aa0e9a
Compare
Only gzip bodies were decoded, synchronously on the event loop. Any other encoding reached the interceptors still encoded, failed to parse, and was forwarded unfiltered. The npm registry serves abbreviated metadata as br to clients that accept it. Decode gzip, br and deflate on the thread pool, and forward bodies with an unsupported encoding through the existing error path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A response in an encoding decodeBody does not support was forwarded unread. When an interceptor may read the response, narrow the forwarded Accept-Encoding to the offered encodings it can decode, and look decoders up as own properties only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
npm's brotli metadata responses are no smaller than its gzip ones, so br (and deflate, which no registry we intercept serves) only added decompression surface. Clients offering them now get gzip negotiated. Also cap decoded bodies at 256MB so a small, highly compressible response can't exhaust the proxy's memory.
8aa0e9a to
c0cd2ec
Compare
No description provided.