Skip to content

Commit dc1279b

Browse files
author
Chekote
committed
Implement multipart/form-data for PUT, DELETE and PATCH requests.
PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few years ago, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226. Conflicts: src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
1 parent 1017d83 commit dc1279b

File tree

4 files changed

+137
-9
lines changed

4 files changed

+137
-9
lines changed

src/Symfony/Component/HttpFoundation/File/UploadedFile.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@
2626
*/
2727
class UploadedFile extends File
2828
{
29+
/**
30+
* List of files that were uploaded outside of PHP's standard POST multipart/form-data method.
31+
*
32+
* @var array
33+
*/
34+
public static $files = array();
35+
2936
/**
3037
* Whether the test mode is activated.
3138
*
@@ -218,7 +225,7 @@ public function isValid()
218225
{
219226
$isOk = $this->error === UPLOAD_ERR_OK;
220227

221-
return $this->test ? $isOk : $isOk && is_uploaded_file($this->getPathname());
228+
return $this->test ? $isOk : $isOk && (is_uploaded_file($this->getPathname()) || in_array($this->getPathname(), self::$files));
222229
}
223230

224231
/**
@@ -242,7 +249,7 @@ public function move($directory, $name = null)
242249

243250
$target = $this->getTargetFile($directory, $name);
244251

245-
if (!@move_uploaded_file($this->getPathname(), $target)) {
252+
if (!@rename($this->getPathname(), $target)) {
246253
$error = error_get_last();
247254
throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message'])));
248255
}

src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 108 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpFoundation;
1313

14+
use Symfony\Component\HttpFoundation\File\UploadedFile;
1415
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1516

1617
/**
@@ -252,26 +253,126 @@ public function initialize(array $query = array(), array $request = array(), arr
252253
}
253254

254255
/**
255-
* Creates a new request with values from PHP's super globals.
256+
* Creates a new request with values from PHP's super globals, or by analyzing the request content if the
257+
* request method is PUT, DELETE or PATCH.
256258
*
257-
* @return Request A new request
259+
* @param Boolean $test Whether the test mode is active
260+
* @return Request A new request
258261
*
259262
* @api
260263
*/
261264
public static function createFromGlobals()
262265
{
263266
$request = new static($_GET, $_POST, array(), $_COOKIE, $_FILES, $_SERVER);
264267

265-
if (0 === strpos($request->headers->get('CONTENT_TYPE'), 'application/x-www-form-urlencoded')
266-
&& in_array(strtoupper($request->server->get('REQUEST_METHOD', 'GET')), array('PUT', 'DELETE', 'PATCH'))
267-
) {
268-
parse_str($request->getContent(), $data);
269-
$request->request = new ParameterBag($data);
268+
if (in_array(strtoupper($request->server->get('REQUEST_METHOD', 'GET')), array('PUT', 'DELETE', 'PATCH'))) {
269+
$contentType = $request->headers->get('CONTENT_TYPE');
270+
271+
if (0 === strpos($contentType, 'application/x-www-form-urlencoded')) {
272+
parse_str($request->getContent(), $data);
273+
$request->request = new ParameterBag($data);
274+
} elseif (0 === strpos($contentType, 'multipart/form-data')) {
275+
$request->decodeMultiPartFormData();
276+
}
270277
}
271278

272279
return $request;
273280
}
274281

282+
/**
283+
* Decodes a multipart/form-data request, and injects the data into the request object. This method is used
284+
* whenever a multipart/form-data request is submitted with a PUT, DELETE or PATCH method.
285+
*
286+
* This implementation is based on an example by netcoder at http://stackoverflow.com/a/9469615.
287+
*
288+
* @param Request $request The Request object whos content should be decoded and places into it's files
289+
* and request properties.
290+
*/
291+
protected function decodeMultiPartFormData()
292+
{
293+
/**
294+
* The files array is structured such that it mimics PHP's $_FILES superglobal, allowing it to be passed on
295+
* to the Request constructor.
296+
* @var array
297+
*/
298+
$files = array();
299+
300+
/**
301+
* Key/value pairs of decoded form-data
302+
* @var array
303+
*/
304+
$data = array();
305+
306+
// Fetch content and determine boundary
307+
$rawData = $this->getContent();
308+
if ($rawData) {
309+
$boundary = substr($rawData, 0, strpos($rawData, "\r\n"));
310+
311+
// Fetch and process each part
312+
$parts = array_slice(explode($boundary, $rawData), 1);
313+
foreach ($parts as $part) {
314+
// If this is the last part, break
315+
if ($part == "--\r\n") {
316+
break;
317+
}
318+
319+
// Separate content from headers
320+
$part = ltrim($part, "\r\n");
321+
list($rawHeaders, $content) = explode("\r\n\r\n", $part, 2);
322+
$content = substr($content, 0, strlen($content) - 2);
323+
324+
// Parse the headers list
325+
$rawHeaders = explode("\r\n", $rawHeaders);
326+
$headers = array();
327+
foreach ($rawHeaders as $header) {
328+
list($name, $value) = explode(':', $header, 2);
329+
$headers[strtolower($name)] = ltrim($value, ' ');
330+
}
331+
332+
// Parse the Content-Disposition to get the field name, etc.
333+
if (isset($headers['content-disposition'])) {
334+
$filename = null;
335+
preg_match(
336+
'/^form-data; *name="([^"]+)"(?:; *filename="([^"]+)")?/',
337+
$headers['content-disposition'],
338+
$matches
339+
);
340+
341+
$fieldName = $matches[1];
342+
$fileName = (isset($matches[2]) ? $matches[2] : null);
343+
344+
// If we have no filename, save the data. Otherwise, save the file.
345+
if ($fileName === null) {
346+
$data[$fieldName] = $content;
347+
} else {
348+
$localFileName = tempnam(sys_get_temp_dir(), 'sfy');
349+
file_put_contents($localFileName, $content);
350+
351+
$files[$fieldName] = array(
352+
'name' => $fileName,
353+
'type' => $headers['content-type'],
354+
'tmp_name' => $localFileName,
355+
'error' => 0,
356+
'size' => filesize($localFileName)
357+
);
358+
359+
// Record the file path so that it can be verified as an uploaded file later
360+
UploadedFile::$files[] = $localFileName;
361+
362+
// If the uploaded file is not moved, we need to delete it. To do that, we
363+
// register a shutdown function to cleanup the temporary file
364+
register_shutdown_function(function () use ($localFileName) {
365+
@unlink($localFileName);
366+
});
367+
}
368+
}
369+
}
370+
}
371+
372+
$this->request = new ParameterBag($data);
373+
$this->files = new FileBag($files);
374+
}
375+
275376
/**
276377
* Creates a Request based on a given URI and configuration.
277378
*
Binary file not shown.

src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,18 @@ public function testCreateFromGlobals($method)
970970

971971
unset($_POST['_method'], $_POST['foo6'], $_SERVER['REQUEST_METHOD']);
972972
$this->disableHttpMethodParameterOverride();
973+
974+
$_SERVER['REQUEST_METHOD'] = $method;
975+
$_SERVER['CONTENT_TYPE'] = 'multipart/form-data';
976+
$request = MultiPartFormDataRequestContentProxy::createFromGlobals();
977+
$this->assertEquals('value', $request->request->get('field'), '::fromGlobals() decodes data from multipart-formdata');
978+
$this->assertTrue($request->files->has('file'), '::fromGlobals() decodes files from multipart-formdata');
979+
$file = $request->files->get('file');
980+
$this->assertEquals('chekote_tiny.png', $file->getClientOriginalName(), '::fromGlobals() decodes file name from multipart-formdata');
981+
$this->assertEquals('image/png', $file->getClientMimeType(), '::fromGlobals() decodes file type from multipart-formdata');
982+
$this->assertEquals(7411, $file->getClientSize(), '::fromGlobals() decodes file size from multipart-formdata');
983+
984+
unset($_SERVER['REQUEST_METHOD'], $_SERVER['CONTENT_TYPE']);
973985
}
974986

975987
public function testOverrideGlobals()
@@ -1595,3 +1607,11 @@ public function getContent($asResource = false)
15951607
return http_build_query(array('_method' => 'PUT', 'content' => 'mycontent'));
15961608
}
15971609
}
1610+
1611+
class MultiPartFormDataRequestContentProxy extends Request
1612+
{
1613+
public function getContent($asResource = false)
1614+
{
1615+
return file_get_contents(__DIR__.'/Fixtures/multipart-formdata');
1616+
}
1617+
}

0 commit comments

Comments
 (0)