Repository navigation
Count multipart preamble bytes against the parser length limit - #1336
Closed
neilberkman wants to merge 1 commit into
Closed
neilberkman wants to merge 1 commit into
neilberkman wants to merge 1 commit into
Conversation
Preamble bytes before the first boundary delimiter were never charged to
:length. :plug_multipart.skip_preamble/2 discards each chunk down to its
last three bytes before returning {:more, rest}, so read_part_headers_more/7
only ever compared the retained bytes against the limit, and
Plug.Parsers.MULTIPART subtracted only the header size from its budget.
A 9 MB preamble ahead of a 2-byte part passed the default 8 MB limit on
both Bandit and Cowboy.
read_part_headers/8 now carries the number of discarded preamble bytes,
returns {:error, :too_large, conn} as soon as they exceed the limit, and
records the total in conn.private[:plug_multipart_skipped] so the parser
subtracts it from its remaining budget.
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.
Preamble bytes before the first
--boundarydelimiter are never charged to:length.:plug_multipart.skip_preamble/2discards each chunk down to its last three bytes before returning{:more, rest}, soread_part_headers_more/7only ever compares those retained bytes against the limit. When the boundary is reached,Plug.Parsers.MULTIPART.parse_multipart_headers/5subtracts onlyheaders_size(headers)from its budget, so preamble bytes skipped in earlier chunks, or ahead of--boundaryin the final chunk, are not counted either.With default parser options (
length: 8_000_000,read_length: 1_000_000) on both Bandit and Cowboy:With this change both raise
Plug.Parsers.RequestTooLargeError(413).Changes:
lib/plug/conn.ex:read_part_headers/8carries the count of discarded preamble bytes, returns{:error, :too_large, conn}as soon as that count exceedslength, and stores the total for the part (including any prefix before--boundaryin the final header chunk) inconn.private[:plug_multipart_skipped].lib/plug/parsers/multipart.ex:parse_multipart_headers/5subtracts the stored count fromlimit.test/plug/parsers_test.exs: a test with an 800 KB preamble and a 7.5 MB part under default options expectsRequestTooLargeErrorwith status 413.Full suite passes on
a735b14(690 passed). Preambles are rare from browsers, so the only behavioural change for ordinary clients is that a request whose preamble plus parts exceeds:lengthis now rejected.