fix(middleware): stop BodyLimit handing out more than the limit - #3072
Open
Rohilalala wants to merge 1 commit into
Open
fix(middleware): stop BodyLimit handing out more than the limit#3072Rohilalala wants to merge 1 commit into
Rohilalala wants to merge 1 commit into
Conversation
limitedReader.Read passed the caller's buffer to the source untouched and only looked at the running total afterwards, and the refusal did not stick. io.Reader asks callers to process the n>0 bytes of a read before treating its error as fatal, so a caller following that advice — encoding/json's Decoder among them — kept getting real data on every call after the limit had already been passed. With a 5 byte limit and a 50 byte body, 50 bytes came through. The read is now capped at one byte past the limit, which is all it takes to know the body is too large; that byte is not handed to the caller; and once the limit is passed the reader stays refused without touching the source again. Fixes labstack#3071
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.
Fixes #3071.
limitedReader.Readpassed the caller's buffer to the source untouched and only looked at the running total afterwards, and the refusal did not stick.io.Readerasks callers to process then>0bytes of a read before treating its error as fatal, so a caller following that advice kept getting real data on every call after the limit had already been passed.With a 5 byte limit against a 50 byte body, reading the way the docs describe returned all 50.
Three changes, each of which the tests pin separately:
LimitBytes.Existing
BodyLimittests are unchanged and pass.