Respect MAX_FILE_SIZE POST field in MultipartParser#261
Conversation
WyriHaximus
changed the title
WyriHaximus
force-pushed
the
258
branch
4 times, most recently
from
e20fc99 to
4f6ec08
Compare
WyriHaximus
changed the title
src/Io/MultipartParser.php
Outdated
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If i remember correctly, it's not that easy. According to http://php.net/manual/en/features.file-upload.post-method.php#example-383, the field must precede the file input field.
The reason is, that you can have multiple MAX_FILE_SIZE hidden fields. Each field controls the max size of the following file fields.
<input name="file1" type="file">
<input name="file2" type="file">
<!-- "file1" and "file2" will have no MAX_FILE_SIZE (no limit) -->
<input name="MAX_FILE_SIZE" value="1024" type="hidden">
<input name="file3" type="file">
<input name="file4" type="file">
<!-- "file3" and "file4" will have MAX_FILE_SIZE = 1024 bytes -->
<input name="MAX_FILE_SIZE" value="0" type="hidden">
<input name="file5" type="file">
<input name="file6" type="file">
<!-- "file5" and "file6" will have MAX_FILE_SIZE = 0 (no limit) -->There was a problem hiding this comment.
Updated the middleware to support this behavior :+
WyriHaximus
force-pushed
the
258
branch
2 times, most recently
from
b0db02a to
106c358
Compare
src/Io/MultipartParser.php
Outdated
| if ($body == '0') { | ||
| $this->maxFileSize = null; | ||
| } | ||
| break; |
clue
left a comment
clue
left a comment
There was a problem hiding this comment.
Nice progress, would love to get this in! 
Added two minor comments, otherwise LGTM! 👍 Can you add some minimal documentation for this?
src/Io/MultipartParser.php
Outdated
| if ($this->maxFileSize !== null && $bodyLength > $this->maxFileSize) { | ||
| return new UploadedFile( | ||
| Psr7\stream_for(''), | ||
| 0, |
There was a problem hiding this comment.
Couldn't find anything in PHP's documentation about this (http://php.net/manual/en/reserved.variables.files.php), but it's my understanding that we should still set the file size as given by the client here so consumers can show a more descriptive error message ("tried to upload X bytes, only Y allowed")?
|
|
||
| if ($body == '0') { | ||
| $this->maxFileSize = null; | ||
| } |
There was a problem hiding this comment.
Should probably cast to int above and use strict comparison here? This seems to be in line with how PHP parses this: https://github.com/php/php-src/blob/1c295d4a9ac78fcc2f77d6695987598bb7abcb83/main/rfc1867.c#L914
3 participants
Implements / closes #258