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

Respect ini settings for file uploads #274

Merged
merged 8 commits into from
Dec 8, 2017
38 changes: 38 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,8 @@ $handler = function (ServerRequestInterface $request) {
if ($avatar instanceof UploadedFileInterface) {
if ($avatar->getError() === UPLOAD_ERR_OK) {
$uploaded = $avatar->getSize() . ' bytes';
} elseif ($avatar->getError() === UPLOAD_ERR_INI_SIZE) {
$uploaded = 'file too large';
} else {
$uploaded = 'with error';
}
Expand All @@ -782,6 +784,34 @@ $server = new StreamingServer(new MiddlewareRunner([

See also [example #12](examples) for more details.

By default, this middleware respects the
[`upload_max_filesize`](http://php.net/manual/en/ini.core.php#ini.upload-max-filesize)
(default `2M`) ini setting.
Files that exceed this limit will be rejected with an `UPLOAD_ERR_INI_SIZE` error.
You can control the maximum filesize for each individual file upload by
explicitly passing the maximum filesize in bytes as the first parameter to the
constructor like this:

```php
new RequestBodyParserMiddleware(8 * 1024 * 1024); // 8 MiB limit per file
```

By default, this middleware respects the
[`file_uploads`](http://php.net/manual/en/ini.core.php#ini.file-uploads)
(default `1`) and
[`max_file_uploads`](http://php.net/manual/en/ini.core.php#ini.max-file-uploads)
(default `20`) ini settings.
These settings control if any and how many files can be uploaded in a single request.
If you upload more files in a single request, additional files will be ignored
and the `getUploadedFiles()` method returns a truncated array.
Note that upload fields left blank on submission do not count towards this limit.
You can control the maximum number of file uploads per request by explicitly
passing the second parameter to the constructor like this:

```php
new RequestBodyParserMiddleware(10 * 1024, 100); // 100 files with 10 KiB each
```

> Note that this middleware handler simply parses everything that is already
buffered in the request body.
It is imperative that the request body is buffered by a prior middleware
Expand All @@ -796,13 +826,21 @@ See also [example #12](examples) for more details.
more details.

> PHP's `MAX_FILE_SIZE` hidden field is respected by this middleware.
Files that exceed this limit will be rejected with an `UPLOAD_ERR_FORM_SIZE` error.

> This middleware respects the
[`max_input_vars`](http://php.net/manual/en/info.configuration.php#ini.max-input-vars)
(default `1000`) and
[`max_input_nesting_level`](http://php.net/manual/en/info.configuration.php#ini.max-input-nesting-level)
(default `64`) ini settings.

> Note that this middleware ignores the
[`enable_post_data_reading`](http://php.net/manual/en/ini.core.php#ini.enable-post-data-reading)
(default `1`) ini setting because it makes little sense to respect here and
is left up to higher-level implementations.
If you want to respect this setting, you have to check its value and
effectively avoid using this middleware entirely.

#### Third-Party Middleware

A non-exhaustive list of third-party middleware can be found at the [`Middleware`](https://github.com/reactphp/http/wiki/Middleware) wiki page.
Expand Down
6 changes: 4 additions & 2 deletions examples/12-upload.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
// contents via `(string)$file->getStream()` instead.
// Here, we simply use an inline image to send back to client:
$avatar = '<img src="data:'. $file->getClientMediaType() . ';base64,' . base64_encode($file->getStream()) . '" /> (' . $file->getSize() . ' bytes)';
} elseif ($file->getError() === UPLOAD_ERR_INI_SIZE) {
$avatar = 'upload exceeds file size limit';
} else {
// Real applications should probably check the error number and
// should print some human-friendly text
Expand Down Expand Up @@ -119,8 +121,8 @@

// buffer and parse HTTP request body before running our request handler
$server = new StreamingServer(new MiddlewareRunner(array(
new RequestBodyBufferMiddleware(100000), // 100 KB max, ignore body otherwise
new RequestBodyParserMiddleware(),
new RequestBodyBufferMiddleware(8 * 1024 * 1024), // 8 MiB max, ignore body otherwise
new RequestBodyParserMiddleware(100 * 1024, 1), // 1 file with 100 KiB max, reject upload otherwise
$handler
)));

Expand Down
105 changes: 90 additions & 15 deletions src/Io/MultipartParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,32 @@ final class MultipartParser
*/
private $maxInputNestingLevel = 64;

private $postCount = 0;
/**
* ini setting "upload_max_filesize"
*
* @var int
*/
private $uploadMaxFilesize;

public static function parseRequest(ServerRequestInterface $request)
{
$parser = new self($request);
return $parser->parse();
}
/**
* ini setting "max_file_uploads"
*
* Additionally, setting "file_uploads = off" effectively sets this to zero.
*
* @var int
*/
private $maxFileUploads;

private function __construct(ServerRequestInterface $request)
{
$this->request = $request;
private $postCount = 0;
private $filesCount = 0;
private $emptyCount = 0;

/**
* @param int|null $uploadMaxFilesize
* @param int|null $maxFileUploads
*/
public function __construct($uploadMaxFilesize = null, $maxFileUploads = null)
{
$var = ini_get('max_input_vars');
if ($var !== false) {
$this->maxInputVars = (int)$var;
Expand All @@ -67,18 +81,29 @@ private function __construct(ServerRequestInterface $request)
if ($var !== false) {
$this->maxInputNestingLevel = (int)$var;
}

$this->uploadMaxFilesize = $uploadMaxFilesize === null ? $this->iniUploadMaxFilesize() : (int)$uploadMaxFilesize;
Copy link
Member

@jsor jsor Dec 8, 2017

Choose a reason for hiding this comment

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

Does is make sense to also allow a string similar to the ini setting, like "8M"? The conversion logic is already there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! I'm not opposed, but would rather look into this in a follow-up PR because this also affects the RequestBodyBufferMiddleware 👍

Copy link
Member

Choose a reason for hiding this comment

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

Opened #275 for that 👍

$this->maxFileUploads = $maxFileUploads === null ? (ini_get('file_uploads') === '' ? 0 : (int)ini_get('max_file_uploads')) : (int)$maxFileUploads;
}

private function parse()
public function parse(ServerRequestInterface $request)
{
$contentType = $this->request->getHeaderLine('content-type');
$contentType = $request->getHeaderLine('content-type');
if(!preg_match('/boundary="?(.*)"?$/', $contentType, $matches)) {
return $this->request;
return $request;
}

$this->parseBody('--' . $matches[1], (string)$this->request->getBody());
$this->request = $request;
$this->parseBody('--' . $matches[1], (string)$request->getBody());

$request = $this->request;
$this->request = null;
$this->postCount = 0;
$this->filesCount = 0;
$this->emptyCount = 0;
$this->maxFileSize = null;

return $this->request;
return $request;
}

private function parseBody($boundary, $buffer)
Expand Down Expand Up @@ -137,10 +162,15 @@ private function parsePart($chunk)

private function parseFile($name, $filename, $contentType, $contents)
{
$file = $this->parseUploadedFile($filename, $contentType, $contents);
if ($file === null) {
return;
}

$this->request = $this->request->withUploadedFiles($this->extractPost(
$this->request->getUploadedFiles(),
$name,
$this->parseUploadedFile($filename, $contentType, $contents)
$file
));
}

Expand All @@ -150,6 +180,11 @@ private function parseUploadedFile($filename, $contentType, $contents)

// no file selected (zero size and empty filename)
if ($size === 0 && $filename === '') {
// ignore excessive number of empty file uploads
if (++$this->emptyCount + $this->filesCount > $this->maxInputVars) {
return;
}

return new UploadedFile(
Psr7\stream_for(''),
$size,
Expand All @@ -159,6 +194,22 @@ private function parseUploadedFile($filename, $contentType, $contents)
);
}

// ignore excessive number of file uploads
if (++$this->filesCount > $this->maxFileUploads) {
return;
}

// file exceeds "upload_max_filesize" ini setting
if ($size > $this->uploadMaxFilesize) {
return new UploadedFile(
Psr7\stream_for(''),
$size,
UPLOAD_ERR_INI_SIZE,
$filename,
$contentType
);
}

// file exceeds MAX_FILE_SIZE value
if ($this->maxFileSize !== null && $size > $this->maxFileSize) {
return new UploadedFile(
Expand Down Expand Up @@ -271,4 +322,28 @@ private function extractPost($postFields, $key, $value)

return $postFields;
}

/**
* Gets upload_max_filesize from PHP's configuration expressed in bytes
*
* @return int
* @link http://php.net/manual/en/ini.core.php#ini.upload-max-filesize
* @codeCoverageIgnore
*/
private function iniUploadMaxFilesize()
{
$size = ini_get('upload_max_filesize');
$suffix = strtoupper(substr($size, -1));
if ($suffix === 'K') {
return substr($size, 0, -1) * 1024;
}
if ($suffix === 'M') {
return substr($size, 0, -1) * 1024 * 1024;
}
if ($suffix === 'G') {
return substr($size, 0, -1) * 1024 * 1024 * 1024;
}

return $size;
}
}
13 changes: 12 additions & 1 deletion src/Middleware/RequestBodyParserMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@

final class RequestBodyParserMiddleware
{
private $multipart;

/**
* @param int|null $uploadMaxFilesize
* @param int|null $maxFileUploads
*/
public function __construct($uploadMaxFilesize = null, $maxFileUploads = null)
{
$this->multipart = new MultipartParser($uploadMaxFilesize, $maxFileUploads);
}

public function __invoke(ServerRequestInterface $request, $next)
{
$type = strtolower($request->getHeaderLine('Content-Type'));
Expand All @@ -17,7 +28,7 @@ public function __invoke(ServerRequestInterface $request, $next)
}

if ($type === 'multipart/form-data') {
return $next(MultipartParser::parseRequest($request));
return $next($this->multipart->parse($request));
}

return $next($request);
Expand Down
Loading