Allow ini-like filesize for RequestBodyBufferMiddleware and MultipartParser#278
Allow ini-like filesize for RequestBodyBufferMiddleware and MultipartParser#278clue merged 1 commit intoreactphp:masterfrom
Conversation
clue
left a comment
There was a problem hiding this comment.
Functionally LGTM, needs some documentation and tests though 👍
| $data .= "Content-type: text/plain\r\n"; | ||
| $data .= "\r\n"; | ||
| $data .= "world\r\n"; | ||
| $data .= str_repeat('world', 1024) . "\r\n"; |
There was a problem hiding this comment.
The test makes perfect sense to me, but I feel that this is a bit misplaced here. Maybe add a separate test case for this? 👍
| } | ||
|
|
||
| $this->sizeLimit = $sizeLimit; | ||
| $this->sizeLimit = IniUtil::iniSizeToBytes($sizeLimit); |
There was a problem hiding this comment.
LGTM, but doesn't match docblock/documentation.
| } | ||
|
|
||
| $this->uploadMaxFilesize = (int)$uploadMaxFilesize; | ||
| $this->uploadMaxFilesize = IniUtil::iniSizeToBytes($uploadMaxFilesize); |
There was a problem hiding this comment.
LGTM, but doesn't match docblock/documentation.
|
@clue updated and added docs and tests |
|
And fixed the test error that just came up 😊 |
| 'GET', | ||
| 'https://example.com/', | ||
| array(), | ||
| new HttpBodyStream($stream, 2) |
There was a problem hiding this comment.
I agree, sementically no, doesn't make sense. Practically doesn't make a difference though.
|
|
||
| /** | ||
| * @param int|null $uploadMaxFilesize | ||
| * @param int|string|null $uploadMaxFilesize |
There was a problem hiding this comment.
LGTM, same change is required for RequestBodyParserMiddleware 👍
| 'GET', | ||
| 'https://example.com/', | ||
| array(), | ||
| new HttpBodyStream($stream, 2) |
There was a problem hiding this comment.
I agree, sementically no, doesn't make sense. Practically doesn't make a difference though.
| * configuration. (Note that the value from | ||
| * the CLI configuration will be used.) | ||
| * @param int|string|null $sizeLimit Either an int with the max request body size | ||
| * or an ini like sze string |
There was a problem hiding this comment.
sze -> size. Also, maybe mention an example like 8M.
| * or null to use post_max_size from PHP's | ||
| * configuration. (Note that the value from | ||
| * the CLI configuration will be used.) | ||
| * @param int|string|null $sizeLimit Either an int with the max request body size |
Split off from #276
Implements / closes #275