fix(decompress): join a repeated content-encoding header - #5664
Open
luantaraschi wants to merge 1 commit into
Open
fix(decompress): join a repeated content-encoding header#5664luantaraschi wants to merge 1 commit into
luantaraschi wants to merge 1 commit into
Conversation
parseHeaders turns a repeated field into an array, and this method's own JSDoc already types the headers record as string | string[] | undefined. An array is truthy, so the skip check let it through and toLowerCase threw a TypeError. RFC 9110 section 5.3 allows joining repeated field lines with commas, which produces the single-line form the decompression chain already splits and handles, including the maxContentEncodings guard.
7 tasks
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
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 relates to...
No open issue. Found while reading
lib/interceptor/decompress.js.Rationale
DecompressHandler.onResponseStartreads the header as a string:parseHeadersturns a repeated field into an array (lib/core/util.js, theval = [val]branch), and the JSDoc on this very method already typesheadersasRecord<string, string | string[] | undefined>. An array is truthy, so#shouldSkipDecompressionlets it through and.toLowerCase()throws:Reproduced against a raw socket server that writes the field twice:
RFC 9110 section 5.3 lets a recipient combine repeated field lines into one by joining the values with commas, and that comma-joined form is exactly what
#createDecompressionChainalready splits and handles. So the fix is to join before anything else looks at the value.The existing chain limit still applies afterwards: six repeated
Content-Encodinglines join into six parts and hit the samemaxContentEncodingsguard as the single-line form, which the suite already covers.Changes
onResponseStartjoins an array-valuedcontent-encodingwith commas before the skip check and the chain build.Test added to
test/interceptors/decompress.js: a response with twoContent-Encoding: gziplines and a doubly gzipped body decompresses to the original text.Features
N/A
Bug Fixes
The decompress interceptor no longer throws a
TypeErrorwhen a response repeatsContent-Encodingacross field lines, and decompresses it the same way it decompresses the comma-joined single-line form.Breaking Changes and Deprecations
None. Responses with a single
Content-Encodingline take exactly the same path as before.Status