Repository navigation
fix: a worker sees a multipart body cut short as PHP sees it - #66
Merged
Merged
Conversation
A body that ended before its closing boundary was a 400 in worker mode, and the app never ran. PHP's parser never refuses a body: a file the body ends in is UPLOAD_ERR_PARTIAL, a field it ends in keeps what arrived (short of a tail that could still have become the next boundary), and the parts before are kept. The body now gets the start of one more part appended once it has ended, so the part it ended in ends as a part does, and the parser shows where the body stopped. The body limit moves into that wrapper so the tail is not counted. A body that stops making sense part-way leaves the parts before that point instead of a 400; a body that cannot be read is still refused. php-src's rfc1867_missing_boundary_2 now passes in worker mode.
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.
In worker mode a multipart body that ended before its closing boundary was a 400 and the app never ran. PHP never refuses such a body:
UPLOAD_ERR_PARTIAL(3): no temp file, no size, no typeHow: once the body has ended, Askr appends the start of one more part, named for this request alone. Whatever part the body ended in then ends as a part does, and that extra part is the sign the body stopped inside the last one. The size limit moved into the same wrapper, so the appended tail is not counted against it. A body that stops making sense part-way now leaves the parts before that point instead of a 400. A body that cannot be read (client gone) is still refused.
Tests
a_worker_sees_a_body_cut_short_as_php_sees_it: 15 cut-short bodies, each compared with per-request PHP as the oracle. It fails on main with a 400.upload.rscover chunk sizes 1/3/7/4096, PHP's tail trimming, cuts between parts and in headers, a read error, andpost_max_sizeto the byte. 11 mutations were tried and every one is caught.scripts/phpt-uploads.py:rfc1867_missing_boundary_2passes in worker mode and is gone fromphpt-uploads.known(worker 9 pass, 7 known).