Skip to content

don't omit CRLF when those are the only bytes the in the chunk - #469

Open
ogalland-tob wants to merge 1 commit into
psf:masterfrom
trail-of-forks:ptp/fix-11
Open

don't omit CRLF when those are the only bytes the in the chunk#469
ogalland-tob wants to merge 1 commit into
psf:masterfrom
trail-of-forks:ptp/fix-11

Conversation

@ogalland-tob

Copy link
Copy Markdown
Contributor

CallbackFileWrapper._safe_read() omits a legitimate two-byte CRLF chunk from the cached response body. If a response contains a chunk consisting exclusively of CRLF, it is omitted from the cached response.

This PR fixes the issue by keeping track of remaining bytes in CallbackFileWrapper and only omitting the trailing CRLF. It also introduces a relevant unit test.

@woodruffw woodruffw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ogalland-tob!

Making sure I understand: this happens when we're reading chunks of an HTTP response, and previously we'd accidentally discard a CRLF if it happened to be the only contents of a chunk?

@ogalland-tob

Copy link
Copy Markdown
Contributor Author

Yes, this would cause chunks made up entirely of CRLF to be dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants