Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

detect incomplete body before reading next request #3256

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pajod
Copy link
Contributor

@pajod pajod commented Jul 31, 2024

**DO NOT MERGE: ** The attached patch only checks that we have read the entire expected body after processing is done. It merely assist in testing and demonstrating that there is a problem, it does not prevent replying to truncated input yet.

I wonder if there is a way to approach #3234 with just one length check. We have one NoMoreData now, but it appears we want two variants - a) done reading, and b) unable to read more.

@benoitc
Copy link
Owner

benoitc commented Aug 6, 2024

I don't understand the point there. We don't crash in the midldle of the appliication processing. How is the application supposed to handle such error there?

@pajod
Copy link
Contributor Author

pajod commented Aug 7, 2024

We don't crash in the middle of the application processing

We do raise InvalidHeaderName when we see something funny in chunked trailer fields. To me that feels like sufficient precedent for demanding that the application (and, where failing that, Gunicorn) can reasonably deal with the result of invalid or truncated input up to and including processing even the very last octet of arbitrary-length input.

EDIT: I have not yet made much progress on determining what "reasonably deal with" means in this context, hence the repro-only state of this PR.

@benoitc
Copy link
Owner

benoitc commented Aug 7, 2024

We don't crash in the middle of the application processing

We do raise InvalidHeaderName when we see something funny in chunked trailer fields. To me that feels like sufficient precedent for demanding that the application (and, where failing that, Gunicorn) can reasonably deal with the result of invalid or truncated input up to and including processing even the very last octet of arbitrary-length input.

fair enough :) please go ahead then :) We shoudl probably document this somewhere also

# Discard any unread body of the previous message
if self.mesg:
data = self.mesg.body.read(8192)
while data:
data = self.mesg.body.read(8192)
if not self.mesg.body.finished:
raise IncompleteBody()
Copy link
Owner

Choose a reason for hiding this comment

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

maybe adding a reason there?

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