From f0bd5dda8136e660919fb73184ef0d914024d9ab Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Thu, 16 Oct 2025 15:03:54 +0900 Subject: [PATCH 01/18] fixed multiple file upload errors --- flight/net/Request.php | 8 +++---- tests/RequestTest.php | 52 +++++++++++++++++++++++++++++++----------- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index c56f1f4..1397842 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -477,7 +477,7 @@ class Request */ public function getUploadedFiles(): array { - $files = []; + $uploadedFiles = []; $correctedFilesArray = $this->reArrayFiles($this->files); foreach ($correctedFilesArray as $keyName => $files) { foreach ($files as $file) { @@ -489,14 +489,14 @@ class Request $file['error'] ); if (count($files) > 1) { - $files[$keyName][] = $UploadedFile; + $uploadedFiles[$keyName][] = $UploadedFile; } else { - $files[$keyName] = $UploadedFile; + $uploadedFiles[$keyName] = $UploadedFile; } } } - return $files; + return $uploadedFiles; } /** diff --git a/tests/RequestTest.php b/tests/RequestTest.php index 214b26e..5dc1f1a 100644 --- a/tests/RequestTest.php +++ b/tests/RequestTest.php @@ -330,31 +330,57 @@ class RequestTest extends TestCase public function testGetMultiFileUpload(): void { - $_FILES['files'] = [ + // Arrange: Setup multiple file upload arrays + $_FILES['files_1'] = [ 'name' => ['file1.txt', 'file2.txt'], 'type' => ['text/plain', 'text/plain'], 'size' => [123, 456], 'tmp_name' => ['/tmp/php123', '/tmp/php456'], 'error' => [0, 0] ]; + $_FILES['files_2'] = [ + 'name' => ['file3.txt', 'file4.txt'], + 'type' => ['text/html', 'application/json'], + 'size' => [789, 321], + 'tmp_name' => ['/tmp/php789', '/tmp/php321'], + 'error' => [0, 0] + ]; + // Act $request = new Request(); + $uploadedFiles = $request->getUploadedFiles(); - $files = $request->getUploadedFiles()['files']; + // Assert: Verify first file group + $firstGroup = $uploadedFiles['files_1'] ?? []; + $this->assertCount(2, $firstGroup, 'First file group should contain 2 files'); - $this->assertCount(2, $files); + $this->assertUploadedFile($firstGroup[0], 'file1.txt', 'text/plain', 123, '/tmp/php123', 0); + $this->assertUploadedFile($firstGroup[1], 'file2.txt', 'text/plain', 456, '/tmp/php456', 0); - $this->assertEquals('file1.txt', $files[0]->getClientFilename()); - $this->assertEquals('text/plain', $files[0]->getClientMediaType()); - $this->assertEquals(123, $files[0]->getSize()); - $this->assertEquals('/tmp/php123', $files[0]->getTempName()); - $this->assertEquals(0, $files[0]->getError()); + // Assert: Verify second file group + $secondGroup = $uploadedFiles['files_2'] ?? []; + $this->assertCount(2, $secondGroup, 'Second file group should contain 2 files'); + + $this->assertUploadedFile($secondGroup[0], 'file3.txt', 'text/html', 789, '/tmp/php789', 0); + $this->assertUploadedFile($secondGroup[1], 'file4.txt', 'application/json', 321, '/tmp/php321', 0); + } - $this->assertEquals('file2.txt', $files[1]->getClientFilename()); - $this->assertEquals('text/plain', $files[1]->getClientMediaType()); - $this->assertEquals(456, $files[1]->getSize()); - $this->assertEquals('/tmp/php456', $files[1]->getTempName()); - $this->assertEquals(0, $files[1]->getError()); + /** + * Helper method to assert uploaded file properties + */ + private function assertUploadedFile( + $file, + string $expectedName, + string $expectedType, + int $expectedSize, + string $expectedTmpName, + int $expectedError + ): void { + $this->assertEquals($expectedName, $file->getClientFilename()); + $this->assertEquals($expectedType, $file->getClientMediaType()); + $this->assertEquals($expectedSize, $file->getSize()); + $this->assertEquals($expectedTmpName, $file->getTempName()); + $this->assertEquals($expectedError, $file->getError()); } public function testUrlWithAtSymbol(): void From ca46fd041d1a61bd72640e5a263a080b692713a3 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Thu, 16 Oct 2025 17:38:21 +0900 Subject: [PATCH 02/18] fix file upload handling to preserve array format for single files --- flight/net/Request.php | 11 ++++++--- tests/RequestTest.php | 51 +++++++++++++++++++++++++++++------------- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 1397842..829356f 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -480,6 +480,10 @@ class Request $uploadedFiles = []; $correctedFilesArray = $this->reArrayFiles($this->files); foreach ($correctedFilesArray as $keyName => $files) { + // Check if original data was array format (files_name[] style) + $originalFile = $this->files->getData()[$keyName] ?? null; + $isArrayFormat = $originalFile && is_array($originalFile['name']); + foreach ($files as $file) { $UploadedFile = new UploadedFile( $file['name'], @@ -488,7 +492,9 @@ class Request $file['tmp_name'], $file['error'] ); - if (count($files) > 1) { + + // Always use array format if original data was array, regardless of count + if ($isArrayFormat) { $uploadedFiles[$keyName][] = $UploadedFile; } else { $uploadedFiles[$keyName] = $UploadedFile; @@ -508,10 +514,9 @@ class Request */ protected function reArrayFiles(Collection $filesCollection): array { - $fileArray = []; foreach ($filesCollection as $fileKeyName => $file) { - $isMulti = is_array($file['name']) === true && count($file['name']) > 1; + $isMulti = is_array($file['name']) === true ; $fileCount = $isMulti === true ? count($file['name']) : 1; $fileKeys = array_keys($file); diff --git a/tests/RequestTest.php b/tests/RequestTest.php index 5dc1f1a..38321ea 100644 --- a/tests/RequestTest.php +++ b/tests/RequestTest.php @@ -332,13 +332,20 @@ class RequestTest extends TestCase { // Arrange: Setup multiple file upload arrays $_FILES['files_1'] = [ - 'name' => ['file1.txt', 'file2.txt'], - 'type' => ['text/plain', 'text/plain'], - 'size' => [123, 456], - 'tmp_name' => ['/tmp/php123', '/tmp/php456'], - 'error' => [0, 0] + 'name' => 'file1.txt', + 'type' => 'text/plain', + 'size' => 123, + 'tmp_name' => '/tmp/php123', + 'error' => 0 ]; $_FILES['files_2'] = [ + 'name' => ['file2.txt'], + 'type' => ['text/plain'], + 'size' => [456], + 'tmp_name' => ['/tmp/php456'], + 'error' => [0] + ]; + $_FILES['files_3'] = [ 'name' => ['file3.txt', 'file4.txt'], 'type' => ['text/html', 'application/json'], 'size' => [789, 321], @@ -350,19 +357,33 @@ class RequestTest extends TestCase $request = new Request(); $uploadedFiles = $request->getUploadedFiles(); - // Assert: Verify first file group - $firstGroup = $uploadedFiles['files_1'] ?? []; - $this->assertCount(2, $firstGroup, 'First file group should contain 2 files'); + // Assert: Verify first file group (single file) + /* + + */ + $firstFile = $uploadedFiles['files_1'] ?? null; + $this->assertNotNull($firstFile, 'First file should exist'); + $this->assertUploadedFile($firstFile, 'file1.txt', 'text/plain', 123, '/tmp/php123', 0); + + // Assert: Verify second file group (array format with single file) + /* + + */ + $secondGroup = $uploadedFiles['files_2'] ?? []; + $this->assertCount(1, $secondGroup, 'Second file group should contain 1 file in array format'); - $this->assertUploadedFile($firstGroup[0], 'file1.txt', 'text/plain', 123, '/tmp/php123', 0); - $this->assertUploadedFile($firstGroup[1], 'file2.txt', 'text/plain', 456, '/tmp/php456', 0); + $this->assertUploadedFile($secondGroup[0], 'file2.txt', 'text/plain', 456, '/tmp/php456', 0); - // Assert: Verify second file group - $secondGroup = $uploadedFiles['files_2'] ?? []; - $this->assertCount(2, $secondGroup, 'Second file group should contain 2 files'); + // Assert: Verify third file group (multiple files) + /* + + + */ + $thirdGroup = $uploadedFiles['files_3'] ?? []; + $this->assertCount(2, $thirdGroup, 'Third file group should contain 2 files'); - $this->assertUploadedFile($secondGroup[0], 'file3.txt', 'text/html', 789, '/tmp/php789', 0); - $this->assertUploadedFile($secondGroup[1], 'file4.txt', 'application/json', 321, '/tmp/php321', 0); + $this->assertUploadedFile($thirdGroup[0], 'file3.txt', 'text/html', 789, '/tmp/php789', 0); + $this->assertUploadedFile($thirdGroup[1], 'file4.txt', 'application/json', 321, '/tmp/php321', 0); } /** From 106955a82e3e6f7ca78a67ee5307acf494357c05 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Thu, 16 Oct 2025 18:31:26 +0900 Subject: [PATCH 03/18] add multipart parsing for PUT/PATCH/DELETE methods and comprehensive tests --- flight/net/Request.php | 211 ++++++++++++++++++++++- tests/RequestBodyParserTest.php | 286 ++++++++++++++++++++++++++++++++ 2 files changed, 489 insertions(+), 8 deletions(-) create mode 100644 tests/RequestBodyParserTest.php diff --git a/flight/net/Request.php b/flight/net/Request.php index 829356f..5d2b283 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -233,14 +233,9 @@ class Request $this->data->setData($data); } } - // Check PUT, PATCH, DELETE for application/x-www-form-urlencoded data - } elseif (in_array($this->method, ['PUT', 'DELETE', 'PATCH'], true) === true) { - $body = $this->getBody(); - if ($body !== '') { - $data = []; - parse_str($body, $data); - $this->data->setData($data); - } + // Check PUT, PATCH, DELETE for application/x-www-form-urlencoded data or multipart/form-data + } elseif (in_array($this->method, [ 'PUT', 'DELETE', 'PATCH' ], true) === true) { + $this->parseRequestBodyForHttpMethods(); } return $this; @@ -533,4 +528,204 @@ class Request return $fileArray; } + + /** + * Parse request body data for HTTP methods that don't natively support form data (PUT, DELETE, PATCH) + * @return void + */ + protected function parseRequestBodyForHttpMethods(): void { + $body = $this->getBody(); + + // Empty body + if ($body === '') { + return; + } + + // Check Content-Type for multipart/form-data + $contentType = strtolower(trim($this->type)); + $isMultipart = strpos($contentType, 'multipart/form-data') === 0; + $boundary = null; + + if ($isMultipart) { + // Extract boundary more safely + if (preg_match('/boundary=(["\']?)([^"\';,\s]+)\1/i', $this->type, $matches)) { + $boundary = $matches[2]; + } + + // If no boundary found, it's not valid multipart + if (empty($boundary)) { + $isMultipart = false; + } + } + + $data = []; + $file = []; + + // Parse application/x-www-form-urlencoded + if ($isMultipart === false) { + parse_str($body, $data); + $this->data->setData($data); + + return; + } + + // Parse multipart/form-data + $bodyParts = preg_split('/\\R?-+' . preg_quote($boundary, '/') . '/s', $body); + array_pop($bodyParts); // Remove last element (empty) + + foreach ($bodyParts as $bodyPart) { + if (empty($bodyPart)) { + continue; + } + + // Get the headers and value + [$header, $value] = preg_split('/\\R\\R/', $bodyPart, 2); + + // Check if the header is normal + if (strpos(strtolower($header), 'content-disposition') === false) { + continue; + } + + $value = ltrim($value, "\r\n"); + + /** + * Process Header + */ + $headers = []; + // split the headers + $headerParts = preg_split('/\\R/', $header); + foreach ($headerParts as $headerPart) { + if (strpos($headerPart, ':') === false) { + continue; + } + + // Process the header + [$headerKey, $headerValue] = explode(':', $headerPart, 2); + + $headerKey = strtolower(trim($headerKey)); + $headerValue = trim($headerValue); + + if (strpos($headerValue, ';') !== false) { + $headers[$headerKey] = []; + foreach (explode(';', $headerValue) as $headerValuePart) { + preg_match_all('/(\w+)=\"?([^";]+)\"?/', $headerValuePart, $headerMatches, PREG_SET_ORDER); + + foreach ($headerMatches as $headerMatch) { + $headerSubKey = strtolower($headerMatch[1]); + $headerSubValue = $headerMatch[2]; + + $headers[$headerKey][$headerSubKey] = $headerSubValue; + } + } + } else { + $headers[$headerKey] = $headerValue; + } + } + + /** + * Process Value + */ + if (!isset($headers['content-disposition']) || !isset($headers['content-disposition']['name'])) { + continue; + } + + $keyName = str_replace("[]", "", $headers['content-disposition']['name']); + + // if is not file + if (!isset($headers['content-disposition']['filename'])) { + if (isset($data[$keyName])) { + if (!is_array($data[$keyName])) { + $data[$keyName] = [$data[$keyName]]; + } + $data[$keyName][] = $value; + } + else { + $data[$keyName] = $value; + } + continue; + } + + $tmpFile = [ + 'name' => $headers['content-disposition']['filename'], + 'type' => $headers['content-type'] ?? 'application/octet-stream', + 'size' => mb_strlen($value, '8bit'), + 'tmp_name' => null, + 'error' => UPLOAD_ERR_OK, + ]; + + if ($tmpFile['size'] > $this->getUploadMaxFileSize()) { + $tmpFile['error'] = UPLOAD_ERR_INI_SIZE; + } else { + // Create a temporary file + $tmpName = tempnam(sys_get_temp_dir(), 'flight_tmp_'); + if ($tmpName === false) { + $tmpFile['error'] = UPLOAD_ERR_CANT_WRITE; + } else { + // Write the value to a temporary file + $bytes = file_put_contents($tmpName, $value); + + if ($bytes === false) { + $tmpFile['error'] = UPLOAD_ERR_CANT_WRITE; + } + else { + // delete the temporary file before ended script + register_shutdown_function(function () use ($tmpName): void { + if (file_exists($tmpName)) { + unlink($tmpName); + } + }); + + $tmpFile['tmp_name'] = $tmpName; + } + } + } + + foreach ($tmpFile as $key => $value) { + if (isset($file[$keyName][$key])) { + if (!is_array($file[$keyName][$key])) { + $file[$keyName][$key] = [$file[$keyName][$key]]; + } + $file[$keyName][$key][] = $value; + } else { + $file[$keyName][$key] = $value; + } + } + } + + $this->data->setData($data); + $this->files->setData($file); + } + + /** + * Get the maximum file size that can be uploaded. + * @return int The maximum file size in bytes. + */ + protected function getUploadMaxFileSize() { + $value = ini_get('upload_max_filesize'); + + $unit = strtolower(preg_replace('/[^a-zA-Z]/', '', $value)); + $value = preg_replace('/[^\d.]/', '', $value); + + switch ($unit) { + case 'p': // PentaByte + case 'pb': + $value *= 1024; + case 't': // Terabyte + case 'tb': + $value *= 1024; + case 'g': // Gigabyte + case 'gb': + $value *= 1024; + case 'm': // Megabyte + case 'mb': + $value *= 1024; + case 'k': // Kilobyte + case 'kb': + $value *= 1024; + case 'b': // Byte + return $value *= 1; + default: + return 0; + } + } } diff --git a/tests/RequestBodyParserTest.php b/tests/RequestBodyParserTest.php new file mode 100644 index 0000000..6b7ede3 --- /dev/null +++ b/tests/RequestBodyParserTest.php @@ -0,0 +1,286 @@ + '/', + 'base' => '/', + 'method' => $method, + 'referrer' => '', + 'ip' => '127.0.0.1', + 'ajax' => false, + 'scheme' => 'http', + 'user_agent' => 'Test', + 'type' => $contentType, + 'length' => strlen($body), + 'secure' => false, + 'accept' => '', + 'proxy_ip' => '', + 'host' => 'localhost', + 'servername' => 'localhost', + 'stream_path' => $stream_path, + 'data' => new Collection(), + 'query' => new Collection(), + 'cookies' => new Collection(), + 'files' => new Collection() + ]; + } + + private function assertUrlEncodedParsing(string $method): void + { + $body = 'foo=bar&baz=qux&key=value'; + $tmpfile = tmpfile(); + $stream_path = stream_get_meta_data($tmpfile)['uri']; + file_put_contents($stream_path, $body); + + $config = [ + 'url' => '/', + 'base' => '/', + 'method' => $method, + 'referrer' => '', + 'ip' => '127.0.0.1', + 'ajax' => false, + 'scheme' => 'http', + 'user_agent' => 'Test', + 'type' => 'application/x-www-form-urlencoded', + 'length' => strlen($body), + 'secure' => false, + 'accept' => '', + 'proxy_ip' => '', + 'host' => 'localhost', + 'servername' => 'localhost', + 'stream_path' => $stream_path, + 'data' => new Collection(), + 'query' => new Collection(), + 'cookies' => new Collection(), + 'files' => new Collection() + ]; + + $request = new Request($config); + + $expectedData = [ + 'foo' => 'bar', + 'baz' => 'qux', + 'key' => 'value' + ]; + $this->assertEquals($expectedData, $request->data->getData()); + + fclose($tmpfile); + } + + private function createMultipartBody(string $boundary, array $fields, array $files = []): string + { + $body = ''; + + // Add form fields + foreach ($fields as $name => $value) { + if (is_array($value)) { + foreach ($value as $item) { + $body .= "--{$boundary}\r\n"; + $body .= "Content-Disposition: form-data; name=\"{$name}\"\r\n"; + $body .= "\r\n"; + $body .= "{$item}\r\n"; + } + } else { + $body .= "--{$boundary}\r\n"; + $body .= "Content-Disposition: form-data; name=\"{$name}\"\r\n"; + $body .= "\r\n"; + $body .= "{$value}\r\n"; + } + } + + // Add files + foreach ($files as $name => $file) { + $body .= "--{$boundary}\r\n"; + $body .= "Content-Disposition: form-data; name=\"{$name}\"; filename=\"{$file['filename']}\"\r\n"; + $body .= "Content-Type: {$file['type']}\r\n"; + $body .= "\r\n"; + $body .= "{$file['content']}\r\n"; + } + + $body .= "--{$boundary}--\r\n"; + + return $body; + } + + public function testParseUrlEncodedBodyForPutMethod(): void + { + $this->assertUrlEncodedParsing('PUT'); + } + + public function testParseUrlEncodedBodyForPatchMethod(): void + { + $this->assertUrlEncodedParsing('PATCH'); + } + + public function testParseUrlEncodedBodyForDeleteMethod(): void + { + $this->assertUrlEncodedParsing('DELETE'); + } + + public function testParseMultipartFormDataWithFiles(): void + { + $boundary = 'boundary123456789'; + $fields = ['title' => 'Test Document']; + $files = [ + 'file' => [ + 'filename' => 'file.txt', + 'type' => 'text/plain', + 'content' => 'This is test file content' + ] + ]; + + $body = $this->createMultipartBody($boundary, $fields, $files); + $config = $this->createRequestConfig('PUT', "multipart/form-data; boundary={$boundary}", $body, $tmpfile); + $request = new Request($config); + + $this->assertEquals(['title' => 'Test Document'], $request->data->getData()); + + $file = $request->getUploadedFiles()['file']; + $this->assertEquals('file.txt', $file->getClientFilename()); + $this->assertEquals('text/plain', $file->getClientMediaType()); + $this->assertEquals(strlen('This is test file content'), $file->getSize()); + $this->assertEquals(UPLOAD_ERR_OK, $file->getError()); + $this->assertNotNull($file->getTempName()); + + fclose($tmpfile); + } + + public function testParseMultipartFormDataWithQuotedBoundary(): void + { + $boundary = 'boundary123456789'; + $fields = ['foo' => 'bar']; + + $body = $this->createMultipartBody($boundary, $fields); + $config = $this->createRequestConfig('PATCH', "multipart/form-data; boundary=\"{$boundary}\"", $body, $tmpfile); + $request = new Request($config); + + $this->assertEquals($fields, $request->data->getData()); + + fclose($tmpfile); + } + + public function testParseMultipartFormDataWithArrayFields(): void + { + $boundary = 'boundary123456789'; + $fields = ['name[]' => ['foo', 'bar']]; + $expectedData = ['name' => ['foo', 'bar']]; + + $body = $this->createMultipartBody($boundary, $fields); + $config = $this->createRequestConfig('PUT', "multipart/form-data; boundary={$boundary}", $body, $tmpfile); + $request = new Request($config); + + $this->assertEquals($expectedData, $request->data->getData()); + + fclose($tmpfile); + } + + public function testParseEmptyBody(): void + { + $config = $this->createRequestConfig('PUT', 'application/x-www-form-urlencoded', '', $tmpfile); + $request = new Request($config); + + $this->assertEquals([], $request->data->getData()); + + fclose($tmpfile); + } + + public function testParseInvalidMultipartWithoutBoundary(): void + { + $originalData = ['foo foo' => 'bar bar', 'baz baz' => 'qux']; + $body = http_build_query($originalData); + $expectedData = ['foo_foo' => 'bar bar', 'baz_baz' => 'qux']; + + $config = $this->createRequestConfig('PUT', 'multipart/form-data', $body, $tmpfile); // no boundary + $request = new Request($config); + + // should fall back to URL encoding and parse correctly + $this->assertEquals($expectedData, $request->data->getData()); + + fclose($tmpfile); + } + + public function testParseMultipartWithLargeFile(): void + { + $boundary = 'boundary123456789'; + $largeContent = str_repeat('A', 10000); // 10KB content + $files = [ + 'file' => [ + 'filename' => 'large.txt', + 'type' => 'text/plain', + 'content' => $largeContent + ] + ]; + + $body = $this->createMultipartBody($boundary, [], $files); + $config = $this->createRequestConfig('PUT', "multipart/form-data; boundary={$boundary}", $body, $tmpfile); + $request = new Request($config); + + $file = $request->getUploadedFiles()['file']; + $this->assertArrayHasKey('file', $request->getUploadedFiles()); + $this->assertEquals('large.txt', $file->getClientFilename()); + $this->assertEquals(10000, $file->getSize()); + $this->assertEquals(UPLOAD_ERR_OK, $file->getError()); + $this->assertNotNull($file->getTempName()); + + fclose($tmpfile); + } + + public function testGetMethodDoesNotTriggerParsing(): void + { + $body = 'foo=bar&baz=qux&key=value'; + $config = $this->createRequestConfig('GET', 'application/x-www-form-urlencoded', $body, $tmpfile); + $request = new Request($config); + + // GET method should not trigger parsing + $this->assertEquals([], $request->data->getData()); + + fclose($tmpfile); + } + + public function testPostMethodDoesNotTriggerParsing(): void + { + $body = 'foo=bar&baz=qux&key=value'; + $config = $this->createRequestConfig('POST', 'application/x-www-form-urlencoded', $body, $tmpfile); + $request = new Request($config); + + // POST method should not trigger this parsing (uses $_POST instead) + $this->assertEquals([], $request->data->getData()); + + fclose($tmpfile); + } +} \ No newline at end of file From 68b9306d308bdce96056ba542a9ea76a5e208513 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 10:39:26 +0900 Subject: [PATCH 04/18] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- flight/net/Request.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 5d2b283..08090cb 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -511,7 +511,7 @@ class Request { $fileArray = []; foreach ($filesCollection as $fileKeyName => $file) { - $isMulti = is_array($file['name']) === true ; + $isMulti = is_array($file['name']) === true; $fileCount = $isMulti === true ? count($file['name']) : 1; $fileKeys = array_keys($file); @@ -533,7 +533,8 @@ class Request * Parse request body data for HTTP methods that don't natively support form data (PUT, DELETE, PATCH) * @return void */ - protected function parseRequestBodyForHttpMethods(): void { + protected function parseRequestBodyForHttpMethods(): void + { $body = $this->getBody(); // Empty body @@ -571,7 +572,7 @@ class Request // Parse multipart/form-data $bodyParts = preg_split('/\\R?-+' . preg_quote($boundary, '/') . '/s', $body); - array_pop($bodyParts); // Remove last element (empty) + array_pop($bodyParts); // Remove last element (empty) foreach ($bodyParts as $bodyPart) { if (empty($bodyPart)) { @@ -700,7 +701,7 @@ class Request * Get the maximum file size that can be uploaded. * @return int The maximum file size in bytes. */ - protected function getUploadMaxFileSize() { + protected function getUploadMaxFileSize(): int { $value = ini_get('upload_max_filesize'); $unit = strtolower(preg_replace('/[^a-zA-Z]/', '', $value)); @@ -723,7 +724,7 @@ class Request case 'kb': $value *= 1024; case 'b': // Byte - return $value *= 1; + return (int)$value; default: return 0; } From 847a0d51f0f0c35ce71525607cc3642a29fbb5a7 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 10:49:56 +0900 Subject: [PATCH 05/18] fix: add null coalescing for undefined superglobals --- flight/net/Request.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 08090cb..4341b08 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -177,10 +177,10 @@ class Request 'user_agent' => $this->getVar('HTTP_USER_AGENT'), 'type' => $this->getVar('CONTENT_TYPE'), 'length' => intval($this->getVar('CONTENT_LENGTH', 0)), - 'query' => new Collection($_GET), - 'data' => new Collection($_POST), - 'cookies' => new Collection($_COOKIE), - 'files' => new Collection($_FILES), + 'query' => new Collection($_GET ?? []), + 'data' => new Collection($_POST ?? []), + 'cookies' => new Collection($_COOKIE ?? []), + 'files' => new Collection($_FILES ?? []), 'secure' => $scheme === 'https', 'accept' => $this->getVar('HTTP_ACCEPT'), 'proxy_ip' => $this->getProxyIpAddress(), @@ -219,7 +219,7 @@ class Request $this->url = '/'; } else { // Merge URL query parameters with $_GET - $_GET = array_merge($_GET, self::parseQuery($this->url)); + $_GET = array_merge($_GET ?? [], self::parseQuery($this->url)); $this->query->setData($_GET); } @@ -546,7 +546,7 @@ class Request $contentType = strtolower(trim($this->type)); $isMultipart = strpos($contentType, 'multipart/form-data') === 0; $boundary = null; - + if ($isMultipart) { // Extract boundary more safely if (preg_match('/boundary=(["\']?)([^"\';,\s]+)\1/i', $this->type, $matches)) { From 41dc0e36789f9fc3d977fcd879449ea7b8e8233f Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:13:01 +0900 Subject: [PATCH 06/18] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- flight/net/Request.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 4341b08..134a83d 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -711,18 +711,19 @@ class Request case 'p': // PentaByte case 'pb': $value *= 1024; + return (int)$value; case 't': // Terabyte - case 'tb': $value *= 1024; + return (int)$value; case 'g': // Gigabyte - case 'gb': $value *= 1024; + return (int)$value; case 'm': // Megabyte - case 'mb': $value *= 1024; + return (int)$value; case 'k': // Kilobyte - case 'kb': $value *= 1024; + return (int)$value; case 'b': // Byte return (int)$value; default: From bbf5204da675491e6ad5a3f117add405342d1469 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:16:52 +0900 Subject: [PATCH 07/18] fix: initialize tmp_name as empty string instead of null --- flight/net/Request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 134a83d..4dd81e7 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -650,7 +650,7 @@ class Request 'name' => $headers['content-disposition']['filename'], 'type' => $headers['content-type'] ?? 'application/octet-stream', 'size' => mb_strlen($value, '8bit'), - 'tmp_name' => null, + 'tmp_name' => '', 'error' => UPLOAD_ERR_OK, ]; From 2c980eb7edd2e8a7e2dcf3b3d582864aee89f5b3 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:27:17 +0900 Subject: [PATCH 08/18] Revert to previous code --- flight/net/Request.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flight/net/Request.php b/flight/net/Request.php index 4341b08..9df43e7 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -728,5 +728,7 @@ class Request default: return 0; } + + return (int)$value; } } From 66a91e042ce8bc917e4a400b4af0abe0148276a2 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:28:08 +0900 Subject: [PATCH 09/18] Revert to previous code --- flight/net/Request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 9df43e7..f3764fd 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -724,7 +724,7 @@ class Request case 'kb': $value *= 1024; case 'b': // Byte - return (int)$value; + break; default: return 0; } From 144327da78ccd098331e999981376165806b4812 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:31:20 +0900 Subject: [PATCH 10/18] Revert "fix: add null coalescing for undefined superglobals" This reverts commit 847a0d51f0f0c35ce71525607cc3642a29fbb5a7. --- flight/net/Request.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index aebe665..ac6f013 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -177,10 +177,10 @@ class Request 'user_agent' => $this->getVar('HTTP_USER_AGENT'), 'type' => $this->getVar('CONTENT_TYPE'), 'length' => intval($this->getVar('CONTENT_LENGTH', 0)), - 'query' => new Collection($_GET ?? []), - 'data' => new Collection($_POST ?? []), - 'cookies' => new Collection($_COOKIE ?? []), - 'files' => new Collection($_FILES ?? []), + 'query' => new Collection($_GET), + 'data' => new Collection($_POST), + 'cookies' => new Collection($_COOKIE), + 'files' => new Collection($_FILES), 'secure' => $scheme === 'https', 'accept' => $this->getVar('HTTP_ACCEPT'), 'proxy_ip' => $this->getProxyIpAddress(), @@ -219,7 +219,7 @@ class Request $this->url = '/'; } else { // Merge URL query parameters with $_GET - $_GET = array_merge($_GET ?? [], self::parseQuery($this->url)); + $_GET = array_merge($_GET, self::parseQuery($this->url)); $this->query->setData($_GET); } @@ -546,7 +546,7 @@ class Request $contentType = strtolower(trim($this->type)); $isMultipart = strpos($contentType, 'multipart/form-data') === 0; $boundary = null; - + if ($isMultipart) { // Extract boundary more safely if (preg_match('/boundary=(["\']?)([^"\';,\s]+)\1/i', $this->type, $matches)) { From 42ad8ae23fedfbbfae8a06f9faf9e5a7e0de1d4c Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:35:19 +0900 Subject: [PATCH 11/18] refactor: simplify upload_max_filesize unit conversion with fallthrough switch --- flight/net/Request.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index ac6f013..d6756c8 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -711,19 +711,14 @@ class Request case 'p': // PentaByte case 'pb': $value *= 1024; - return (int)$value; case 't': // Terabyte $value *= 1024; - return (int)$value; case 'g': // Gigabyte $value *= 1024; - return (int)$value; case 'm': // Megabyte $value *= 1024; - return (int)$value; case 'k': // Kilobyte $value *= 1024; - return (int)$value; case 'b': // Byte break; default: From 9ab2695d48940be11196834eaae2f427d6ace25c Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:36:50 +0900 Subject: [PATCH 12/18] fix: add null coalescing for undefined superglobals --- flight/net/Request.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index d6756c8..67e416d 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -177,10 +177,10 @@ class Request 'user_agent' => $this->getVar('HTTP_USER_AGENT'), 'type' => $this->getVar('CONTENT_TYPE'), 'length' => intval($this->getVar('CONTENT_LENGTH', 0)), - 'query' => new Collection($_GET), - 'data' => new Collection($_POST), - 'cookies' => new Collection($_COOKIE), - 'files' => new Collection($_FILES), + 'query' => new Collection($_GET ?? []), + 'data' => new Collection($_POST ?? []), + 'cookies' => new Collection($_COOKIE ?? []), + 'files' => new Collection($_FILES ?? []), 'secure' => $scheme === 'https', 'accept' => $this->getVar('HTTP_ACCEPT'), 'proxy_ip' => $this->getProxyIpAddress(), @@ -219,7 +219,7 @@ class Request $this->url = '/'; } else { // Merge URL query parameters with $_GET - $_GET = array_merge($_GET, self::parseQuery($this->url)); + $_GET = array_merge($_GET ?? [], self::parseQuery($this->url)); $this->query->setData($_GET); } From 82daf71d0a65df5e9a18119cfe2f7bf000691b48 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 17 Oct 2025 11:43:11 +0900 Subject: [PATCH 13/18] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- flight/net/Request.php | 6 ++---- tests/RequestBodyParserTest.php | 28 ++-------------------------- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 67e416d..7233758 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -639,8 +639,7 @@ class Request $data[$keyName] = [$data[$keyName]]; } $data[$keyName][] = $value; - } - else { + } else { $data[$keyName] = $value; } continue; @@ -667,8 +666,7 @@ class Request if ($bytes === false) { $tmpFile['error'] = UPLOAD_ERR_CANT_WRITE; - } - else { + } else { // delete the temporary file before ended script register_shutdown_function(function () use ($tmpName): void { if (file_exists($tmpName)) { diff --git a/tests/RequestBodyParserTest.php b/tests/RequestBodyParserTest.php index 6b7ede3..55e3b5a 100644 --- a/tests/RequestBodyParserTest.php +++ b/tests/RequestBodyParserTest.php @@ -63,32 +63,8 @@ class RequestBodyParserTest extends TestCase private function assertUrlEncodedParsing(string $method): void { $body = 'foo=bar&baz=qux&key=value'; - $tmpfile = tmpfile(); - $stream_path = stream_get_meta_data($tmpfile)['uri']; - file_put_contents($stream_path, $body); - - $config = [ - 'url' => '/', - 'base' => '/', - 'method' => $method, - 'referrer' => '', - 'ip' => '127.0.0.1', - 'ajax' => false, - 'scheme' => 'http', - 'user_agent' => 'Test', - 'type' => 'application/x-www-form-urlencoded', - 'length' => strlen($body), - 'secure' => false, - 'accept' => '', - 'proxy_ip' => '', - 'host' => 'localhost', - 'servername' => 'localhost', - 'stream_path' => $stream_path, - 'data' => new Collection(), - 'query' => new Collection(), - 'cookies' => new Collection(), - 'files' => new Collection() - ]; + $tmpfile = null; + $config = $this->createRequestConfig($method, 'application/x-www-form-urlencoded', $body, $tmpfile); $request = new Request($config); From dc56ca2c4ad378b3a29217e22e2904b411189c3e Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Tue, 21 Oct 2025 16:01:45 +0900 Subject: [PATCH 14/18] fix: allow non-string types in Response::write() to prevent TypeError --- flight/net/Response.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flight/net/Response.php b/flight/net/Response.php index 9587ddf..6d3da1c 100644 --- a/flight/net/Response.php +++ b/flight/net/Response.php @@ -237,7 +237,7 @@ class Response * * @return $this Self reference */ - public function write(string $str, bool $overwrite = false): self + public function write($str, bool $overwrite = false): self { if ($overwrite === true) { $this->clearBody(); From 671fea075c8e207d1aee73144c2adda49603195c Mon Sep 17 00:00:00 2001 From: n0nag0n Date: Tue, 21 Oct 2025 08:32:10 -0600 Subject: [PATCH 15/18] adjustments for security, 100% coverage and phpcs --- flight/net/Request.php | 354 +++++++++++++++++++++----------- tests/RequestBodyParserTest.php | 205 +++++++++++++++++- 2 files changed, 427 insertions(+), 132 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 7233758..4aa51f4 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -147,6 +147,13 @@ class Request */ public string $body = ''; + /** + * Hold tmp file handles created via tmpfile() so they persist for request lifetime + * + * @var array + */ + private array $tmpFileHandles = []; + /** * Constructor. * @@ -177,10 +184,10 @@ class Request 'user_agent' => $this->getVar('HTTP_USER_AGENT'), 'type' => $this->getVar('CONTENT_TYPE'), 'length' => intval($this->getVar('CONTENT_LENGTH', 0)), - 'query' => new Collection($_GET ?? []), - 'data' => new Collection($_POST ?? []), - 'cookies' => new Collection($_COOKIE ?? []), - 'files' => new Collection($_FILES ?? []), + 'query' => new Collection($_GET), + 'data' => new Collection($_POST), + 'cookies' => new Collection($_COOKIE), + 'files' => new Collection($_FILES), 'secure' => $scheme === 'https', 'accept' => $this->getVar('HTTP_ACCEPT'), 'proxy_ip' => $this->getProxyIpAddress(), @@ -219,7 +226,7 @@ class Request $this->url = '/'; } else { // Merge URL query parameters with $_GET - $_GET = array_merge($_GET ?? [], self::parseQuery($this->url)); + $_GET = array_merge($_GET, self::parseQuery($this->url)); $this->query->setData($_GET); } @@ -233,7 +240,7 @@ class Request $this->data->setData($data); } } - // Check PUT, PATCH, DELETE for application/x-www-form-urlencoded data or multipart/form-data + // Check PUT, PATCH, DELETE for application/x-www-form-urlencoded data or multipart/form-data } elseif (in_array($this->method, [ 'PUT', 'DELETE', 'PATCH' ], true) === true) { $this->parseRequestBodyForHttpMethods(); } @@ -468,7 +475,7 @@ class Request /** * Retrieves the array of uploaded files. * - * @return array|array>> The array of uploaded files. + * @return array> Key is field name; value is either a single UploadedFile or an array of UploadedFile when multiple were uploaded. */ public function getUploadedFiles(): array { @@ -478,7 +485,7 @@ class Request // Check if original data was array format (files_name[] style) $originalFile = $this->files->getData()[$keyName] ?? null; $isArrayFormat = $originalFile && is_array($originalFile['name']); - + foreach ($files as $file) { $UploadedFile = new UploadedFile( $file['name'], @@ -487,9 +494,9 @@ class Request $file['tmp_name'], $file['error'] ); - + // Always use array format if original data was array, regardless of count - if ($isArrayFormat) { + if ($isArrayFormat === true) { $uploadedFiles[$keyName][] = $UploadedFile; } else { $uploadedFiles[$keyName] = $UploadedFile; @@ -531,198 +538,299 @@ class Request /** * Parse request body data for HTTP methods that don't natively support form data (PUT, DELETE, PATCH) + * * @return void */ protected function parseRequestBodyForHttpMethods(): void { $body = $this->getBody(); - + // Empty body if ($body === '') { return; } - + // Check Content-Type for multipart/form-data $contentType = strtolower(trim($this->type)); $isMultipart = strpos($contentType, 'multipart/form-data') === 0; $boundary = null; - - if ($isMultipart) { + + if ($isMultipart === true) { // Extract boundary more safely if (preg_match('/boundary=(["\']?)([^"\';,\s]+)\1/i', $this->type, $matches)) { $boundary = $matches[2]; } - + // If no boundary found, it's not valid multipart if (empty($boundary)) { $isMultipart = false; } - } - $data = []; - $file = []; + $firstLine = strtok($body, "\r\n"); + if ($firstLine === false || strpos($firstLine, '--' . $boundary) !== 0) { + // Does not start with the boundary marker; fall back + $isMultipart = false; + } + } // Parse application/x-www-form-urlencoded if ($isMultipart === false) { parse_str($body, $data); $this->data->setData($data); - return; } - + + $this->setParsedRequestBodyMultipartFormData($body, $boundary); + } + + /** + * Sets the parsed request body for multipart form data requests + * + * This method processes and stores multipart form data from the request body, + * parsing it according to the specified boundary delimiter. It handles the + * complex parsing of multipart data including file uploads and form fields. + * + * @param string $body The raw multipart request body content + * @param string $boundary The boundary string used to separate multipart sections + * + * @return void + */ + protected function setParsedRequestBodyMultipartFormData(string $body, string $boundary): void + { + + $data = []; + $file = []; + // Parse multipart/form-data - $bodyParts = preg_split('/\\R?-+' . preg_quote($boundary, '/') . '/s', $body); - array_pop($bodyParts); // Remove last element (empty) - + $bodyParts = preg_split('/\R?-+' . preg_quote($boundary, '/') . '/s', $body); + array_pop($bodyParts); // Remove last element (closing boundary or empty) + + $partsProcessed = 0; + $filesTotalBytes = 0; + // Use ini values directly + $maxParts = (int) ini_get('max_file_uploads'); + if ($maxParts <= 0) { + // unlimited parts if not specified + $maxParts = PHP_INT_MAX; // @codeCoverageIgnore + } + $maxTotalBytes = self::derivePostMaxSizeBytes(); + foreach ($bodyParts as $bodyPart) { - if (empty($bodyPart)) { + if ($partsProcessed >= $maxParts) { + // reached part limit from ini + break; // @codeCoverageIgnore + } + if ($bodyPart === '' || $bodyPart === null) { + continue; // skip empty segments + } + $partsProcessed++; + + // Split headers and value; if format invalid, skip early + $split = preg_split('/\R\R/', $bodyPart, 2); + if ($split === false || count($split) < 2) { continue; } + [$header, $value] = $split; - // Get the headers and value - [$header, $value] = preg_split('/\\R\\R/', $bodyPart, 2); - - // Check if the header is normal - if (strpos(strtolower($header), 'content-disposition') === false) { + // Fast header sanity checks + if (stripos($header, 'content-disposition') === false) { + continue; + } + if (strlen($header) > 16384) { // 16KB header block guard continue; } $value = ltrim($value, "\r\n"); - /** - * Process Header - */ - $headers = []; - // split the headers - $headerParts = preg_split('/\\R/', $header); - foreach ($headerParts as $headerPart) { - if (strpos($headerPart, ':') === false) { - continue; - } + // Parse headers (simple approach, fail-fast on anomalies) + $headers = $this->parseRequestBodyHeadersFromMultipartFormData($header); - // Process the header - [$headerKey, $headerValue] = explode(':', $headerPart, 2); - - $headerKey = strtolower(trim($headerKey)); - $headerValue = trim($headerValue); - - if (strpos($headerValue, ';') !== false) { - $headers[$headerKey] = []; - foreach (explode(';', $headerValue) as $headerValuePart) { - preg_match_all('/(\w+)=\"?([^";]+)\"?/', $headerValuePart, $headerMatches, PREG_SET_ORDER); - - foreach ($headerMatches as $headerMatch) { - $headerSubKey = strtolower($headerMatch[1]); - $headerSubValue = $headerMatch[2]; - - $headers[$headerKey][$headerSubKey] = $headerSubValue; - } - } - } else { - $headers[$headerKey] = $headerValue; - } - } - - /** - * Process Value - */ - if (!isset($headers['content-disposition']) || !isset($headers['content-disposition']['name'])) { + // Required disposition/name + if (isset($headers['content-disposition']['name']) === false) { continue; } + $keyName = str_replace('[]', '', (string) $headers['content-disposition']['name']); + if ($keyName === '') { + continue; // avoid empty keys + } - $keyName = str_replace("[]", "", $headers['content-disposition']['name']); - - // if is not file - if (!isset($headers['content-disposition']['filename'])) { - if (isset($data[$keyName])) { - if (!is_array($data[$keyName])) { + // Non-file field + if (isset($headers['content-disposition']['filename']) === false) { + if (isset($data[$keyName]) === false) { + $data[$keyName] = $value; + } else { + if (is_array($data[$keyName]) === false) { $data[$keyName] = [$data[$keyName]]; } $data[$keyName][] = $value; - } else { - $data[$keyName] = $value; } - continue; + continue; // done with this part + } + + // Sanitize filename early + $rawFilename = (string) $headers['content-disposition']['filename']; + $rawFilename = str_replace(["\0", "\r", "\n"], '', $rawFilename); + $sanitizedFilename = basename($rawFilename); + $matchCriteria = preg_match('/^[A-Za-z0-9._-]{1,255}$/', $sanitizedFilename); + if ($sanitizedFilename === '' || $matchCriteria !== 1) { + $sanitizedFilename = 'upload_' . uniqid('', true); } + $size = mb_strlen($value, '8bit'); + $filesTotalBytes += $size; $tmpFile = [ - 'name' => $headers['content-disposition']['filename'], + 'name' => $sanitizedFilename, 'type' => $headers['content-type'] ?? 'application/octet-stream', - 'size' => mb_strlen($value, '8bit'), + 'size' => $size, 'tmp_name' => '', 'error' => UPLOAD_ERR_OK, ]; - if ($tmpFile['size'] > $this->getUploadMaxFileSize()) { - $tmpFile['error'] = UPLOAD_ERR_INI_SIZE; + // Fail fast on size constraints + if ($size > $this->getUploadMaxFileSize() || $filesTotalBytes > $maxTotalBytes) { + // individual file or total size exceeded + $tmpFile['error'] = UPLOAD_ERR_INI_SIZE; // @codeCoverageIgnore } else { - // Create a temporary file - $tmpName = tempnam(sys_get_temp_dir(), 'flight_tmp_'); - if ($tmpName === false) { - $tmpFile['error'] = UPLOAD_ERR_CANT_WRITE; - } else { - // Write the value to a temporary file - $bytes = file_put_contents($tmpName, $value); + $tempResult = $this->createTempFile($value); + $tmpFile['tmp_name'] = $tempResult['tmp_name']; + $tmpFile['error'] = $tempResult['error']; + } - if ($bytes === false) { - $tmpFile['error'] = UPLOAD_ERR_CANT_WRITE; - } else { - // delete the temporary file before ended script - register_shutdown_function(function () use ($tmpName): void { - if (file_exists($tmpName)) { - unlink($tmpName); - } - }); - - $tmpFile['tmp_name'] = $tmpName; - } + // Aggregate into synthetic files array + foreach ($tmpFile as $metaKey => $metaVal) { + if (!isset($file[$keyName][$metaKey])) { + $file[$keyName][$metaKey] = $metaVal; + continue; + } + if (!is_array($file[$keyName][$metaKey])) { + $file[$keyName][$metaKey] = [$file[$keyName][$metaKey]]; } + $file[$keyName][$metaKey][] = $metaVal; } + } - foreach ($tmpFile as $key => $value) { - if (isset($file[$keyName][$key])) { - if (!is_array($file[$keyName][$key])) { - $file[$keyName][$key] = [$file[$keyName][$key]]; + $this->data->setData($data); + $this->files->setData($file); + } + + /** + * Parses request body headers from multipart form data + * + * This method extracts and processes headers from a multipart form data section, + * typically used for file uploads or complex form submissions. It parses the + * header string and returns an associative array of header name-value pairs. + * + * @param string $header The raw header string from a multipart form data section + * + * @return array An associative array containing parsed header name-value pairs + */ + protected function parseRequestBodyHeadersFromMultipartFormData(string $header): array + { + $headers = []; + foreach (preg_split('/\R/', $header) as $headerLine) { + if (strpos($headerLine, ':') === false) { + continue; + } + [$headerKey, $headerValue] = explode(':', $headerLine, 2); + $headerKey = strtolower(trim($headerKey)); + $headerValue = trim($headerValue); + if (strpos($headerValue, ';') !== false) { + $headers[$headerKey] = []; + foreach (explode(';', $headerValue) as $hvPart) { + preg_match_all('/(\w+)="?([^";]+)"?/', $hvPart, $matches, PREG_SET_ORDER); + foreach ($matches as $m) { + $subKey = strtolower($m[1]); + $headers[$headerKey][$subKey] = $m[2]; } - $file[$keyName][$key][] = $value; - } else { - $file[$keyName][$key] = $value; } + } else { + $headers[$headerKey] = $headerValue; } } - - $this->data->setData($data); - $this->files->setData($file); + return $headers; } /** * Get the maximum file size that can be uploaded. + * * @return int The maximum file size in bytes. */ - protected function getUploadMaxFileSize(): int { + public function getUploadMaxFileSize(): int + { $value = ini_get('upload_max_filesize'); + return self::parsePhpSize($value); + } + + /** + * Parse a PHP shorthand size string (like "1K", "1.5M") into bytes. + * Returns 0 on unknown or unsupported unit (keeps existing behavior). + * + * @param string $size + * + * @return int + */ + public static function parsePhpSize(string $size): int + { + $unit = strtolower(preg_replace('/[^a-zA-Z]/', '', $size)); + $value = (int) preg_replace('/[^\d.]/', '', $size); - $unit = strtolower(preg_replace('/[^a-zA-Z]/', '', $value)); - $value = preg_replace('/[^\d.]/', '', $value); + // No unit => follow existing behavior and return value directly if > 1024 (1K) + if ($unit === '' && $value >= 1024) { + return $value; + } switch ($unit) { - case 'p': // PentaByte - case 'pb': - $value *= 1024; - case 't': // Terabyte - $value *= 1024; - case 'g': // Gigabyte + case 't': + case 'tb': + $value *= 1024; // Fall through + case 'g': + case 'gb': + $value *= 1024; // Fall through + case 'm': + case 'mb': + $value *= 1024; // Fall through + case 'k': $value *= 1024; - case 'm': // Megabyte - $value *= 1024; - case 'k': // Kilobyte - $value *= 1024; - case 'b': // Byte break; default: return 0; } - return (int)$value; + return $value; + } + + /** + * Derive post_max_size in bytes. Returns 0 when unlimited or unparsable. + */ + private static function derivePostMaxSizeBytes(): int + { + $postMax = (string) ini_get('post_max_size'); + $bytes = self::parsePhpSize($postMax); + return $bytes; // 0 means unlimited + } + + /** + * Create a temporary file for uploaded content using tmpfile(). + * Returns array with tmp_name and error code. + * + * @param string $content + * + * @return array + */ + private function createTempFile(string $content): array + { + $fp = tmpfile(); + if ($fp === false) { + return ['tmp_name' => '', 'error' => UPLOAD_ERR_CANT_WRITE]; // @codeCoverageIgnore + } + $bytes = fwrite($fp, $content); + if ($bytes === false) { + fclose($fp); // @codeCoverageIgnore + return ['tmp_name' => '', 'error' => UPLOAD_ERR_CANT_WRITE]; // @codeCoverageIgnore + } + $meta = stream_get_meta_data($fp); + $tmpName = isset($meta['uri']) ? $meta['uri'] : ''; + $this->tmpFileHandles[] = $fp; // retain handle for lifecycle + return ['tmp_name' => $tmpName, 'error' => UPLOAD_ERR_OK]; } } diff --git a/tests/RequestBodyParserTest.php b/tests/RequestBodyParserTest.php index 55e3b5a..1cc5bdf 100644 --- a/tests/RequestBodyParserTest.php +++ b/tests/RequestBodyParserTest.php @@ -24,10 +24,6 @@ class RequestBodyParserTest extends TestCase { unset($_REQUEST); unset($_SERVER); - unset($_GET); - unset($_POST); - unset($_COOKIE); - unset($_FILES); } private function createRequestConfig(string $method, string $contentType, string $body, &$tmpfile = null): array @@ -65,7 +61,7 @@ class RequestBodyParserTest extends TestCase $body = 'foo=bar&baz=qux&key=value'; $tmpfile = null; $config = $this->createRequestConfig($method, 'application/x-www-form-urlencoded', $body, $tmpfile); - + $request = new Request($config); $expectedData = [ @@ -81,7 +77,7 @@ class RequestBodyParserTest extends TestCase private function createMultipartBody(string $boundary, array $fields, array $files = []): string { $body = ''; - + // Add form fields foreach ($fields as $name => $value) { if (is_array($value)) { @@ -109,7 +105,7 @@ class RequestBodyParserTest extends TestCase } $body .= "--{$boundary}--\r\n"; - + return $body; } @@ -145,7 +141,7 @@ class RequestBodyParserTest extends TestCase $request = new Request($config); $this->assertEquals(['title' => 'Test Document'], $request->data->getData()); - + $file = $request->getUploadedFiles()['file']; $this->assertEquals('file.txt', $file->getClientFilename()); $this->assertEquals('text/plain', $file->getClientMediaType()); @@ -259,4 +255,195 @@ class RequestBodyParserTest extends TestCase fclose($tmpfile); } -} \ No newline at end of file + + /** + * Tests getUploadMaxFileSize parsing for various php.ini unit suffixes. + * We'll call the method in-process after setting ini values via ini_set + * and also simulate a value with unknown unit to hit the default branch. + */ + public function testGetUploadMaxFileSizeUnits(): void + { + // Use PHP CLI with -d to set upload_max_filesize (ini_set can't change this setting in many SAPIs) + $cases = [ + // No unit yields default branch which returns 0 in current implementation + ['1' , 0], // no unit and number too small + ['1K' , 1024], + ['2M' , 2 * 1024 * 1024], + ['1G' , 1024 * 1024 * 1024], + ['1T' , 1024 * 1024 * 1024 * 1024], + ['1Z' , 0 ], // Unknown unit and number too small + [ '1024', 1024 ] + ]; + + foreach ($cases as [$iniVal, $expected]) { + $actual = Request::parsePhpSize($iniVal); + $this->assertEquals($expected, $actual, "upload_max_filesize={$iniVal}"); + } + } + + /** + * Helper: run PHP CLI with -d upload_max_filesize and return the Request::getUploadMaxFileSize() result. + */ + // removed CLI helper; parsePhpSize covers unit parsing and is pure + + public function testMultipartBoundaryInvalidFallsBackToUrlEncoded(): void + { + // Body doesn't start with boundary marker => fallback to urlencoded branch + $body = 'field1=value1&field2=value2'; + $tmp = tmpfile(); + $path = stream_get_meta_data($tmp)['uri']; + file_put_contents($path, $body); + + $request = new Request([ + 'url' => '/upload', + 'base' => '/', + 'method' => 'PATCH', + 'type' => 'multipart/form-data; boundary=BOUNDARYXYZ', // claims multipart + 'stream_path' => $path, + 'data' => new Collection(), + 'query' => new Collection(), + 'files' => new Collection(), + ]); + + $this->assertEquals(['field1' => 'value1', 'field2' => 'value2'], $request->data->getData()); + $this->assertSame([], $request->files->getData()); + } + + public function testMultipartParsingEdgeCases(): void + { + $boundary = 'MBOUND123'; + $parts = []; + + // A: invalid split (no blank line) => skipped + $parts[] = "Content-Disposition: form-data; name=\"skipnosplit\""; // no value portion + + // B: missing content-disposition entirely => skipped + $parts[] = "Content-Type: text/plain\r\n\r\nignoredvalue"; + + // C: header too long (>16384) => skipped + $longHeader = 'Content-Disposition: form-data; name="toolong"; filename="toolong.txt"; ' . str_repeat('x', 16500); + $parts[] = $longHeader . "\r\n\r\nlongvalue"; + + // D: header line without colon gets skipped but rest processed; becomes non-file field + $parts[] = "BadHeaderLine\r\nContent-Disposition: form-data; name=\"fieldX\"\r\n\r\nvalueX"; + + // E: disposition without name => skipped + $parts[] = "Content-Disposition: form-data; filename=\"nofname.txt\"\r\n\r\nnoNameValue"; + + // F: empty name => skipped + $parts[] = "Content-Disposition: form-data; name=\"\"; filename=\"empty.txt\"\r\n\r\nemptyNameValue"; + + // G: invalid filename triggers sanitized fallback + $parts[] = "Content-Disposition: form-data; name=\"filebad\"; filename=\"a*b?.txt\"\r\nContent-Type: text/plain\r\n\r\nFILEBAD"; + + // H1 & H2: two files same key for aggregation logic (arrays) + $parts[] = "Content-Disposition: form-data; name=\"filemulti\"; filename=\"one.txt\"\r\nContent-Type: text/plain\r\n\r\nONE"; + $parts[] = "Content-Disposition: form-data; name=\"filemulti\"; filename=\"two.txt\"\r\nContent-Type: text/plain\r\n\r\nTWO"; + + // I: file exceeding total bytes triggers UPLOAD_ERR_INI_SIZE + $parts[] = "Content-Disposition: form-data; name=\"filebig\"; filename=\"big.txt\"\r\nContent-Type: text/plain\r\n\r\n" . str_repeat('A', 10); + + // Build full body + $body = ''; + foreach ($parts as $p) { + $body .= '--' . $boundary . "\r\n" . $p . "\r\n"; + } + $body .= '--' . $boundary . "--\r\n"; + + $tmp = tmpfile(); + $path = stream_get_meta_data($tmp)['uri']; + file_put_contents($path, $body); + + $request = new Request([ + 'url' => '/upload', + 'base' => '/', + 'method' => 'PATCH', + 'type' => 'multipart/form-data; boundary=' . $boundary, + 'stream_path' => $path, + 'data' => new Collection(), + 'query' => new Collection(), + 'files' => new Collection(), + ]); + + $data = $request->data->getData(); + $this->assertArrayHasKey('fieldX', $data); // only processed non-file field + $this->assertEquals('valueX', $data['fieldX']); + $files = $request->files->getData(); + + // filebad fallback name + $this->assertArrayHasKey('filebad', $files); + $this->assertMatchesRegularExpression('/^upload_/', $files['filebad']['name']); + + // filemulti aggregated arrays + $this->assertArrayHasKey('filemulti', $files); + $this->assertEquals(['one.txt', 'two.txt'], $files['filemulti']['name']); + $this->assertEquals(['text/plain', 'text/plain'], $files['filemulti']['type']); + + // filebig error path + $this->assertArrayHasKey('filebig', $files); + $uploadMax = Request::parsePhpSize(ini_get('upload_max_filesize')); + $postMax = Request::parsePhpSize(ini_get('post_max_size')); + $shouldError = ($uploadMax > 0 && $uploadMax < 10) || ($postMax > 0 && $postMax < 10); + if ($shouldError) { + $this->assertEquals(UPLOAD_ERR_INI_SIZE, $files['filebig']['error']); + } else { + $this->assertEquals(UPLOAD_ERR_OK, $files['filebig']['error']); + } + } + + + public function testMultipartEmptyArrayNameStripped(): void + { + // Covers line where keyName becomes empty after removing [] (name="[]") and header param extraction (preg_match_all) + $boundary = 'BOUNDARYEMPTY'; + $validFilePart = "Content-Disposition: form-data; name=\"fileok\"; filename=\"ok.txt\"\r\nContent-Type: text/plain\r\n\r\nOK"; + $emptyNameFilePart = "Content-Disposition: form-data; name=\"[]\"; filename=\"empty.txt\"\r\nContent-Type: text/plain\r\n\r\nSHOULD_SKIP"; + $body = '--' . $boundary . "\r\n" . $validFilePart . "\r\n" . '--' . $boundary . "\r\n" . $emptyNameFilePart . "\r\n" . '--' . $boundary . "--\r\n"; + + $tmp = tmpfile(); + $path = stream_get_meta_data($tmp)['uri']; + file_put_contents($path, $body); + + $request = new Request([ + 'url' => '/upload', + 'base' => '/', + 'method' => 'PATCH', + 'type' => 'multipart/form-data; boundary=' . $boundary, + 'stream_path' => $path, + 'data' => new Collection(), + 'query' => new Collection(), + 'files' => new Collection(), + ]); + + $files = $request->files->getData(); + // fileok processed + $this->assertArrayHasKey('fileok', $files); + // name="[]" stripped => keyName becomes empty -> skipped + $this->assertArrayNotHasKey('empty', $files); // just to show not mistakenly created + $this->assertCount(5, $files['fileok']); // meta keys name,type,size,tmp_name,error + } + + public function testMultipartMalformedBoundaryFallsBackToUrlEncoded(): void + { + // boundary has invalid characters (spaces) so regex validation fails -> line 589 path + $invalidBoundary = 'BAD BOUNDARY WITH SPACE'; + $body = 'alpha=1&beta=2'; // should parse as urlencoded after fallback + $tmp = tmpfile(); + $path = stream_get_meta_data($tmp)['uri']; + file_put_contents($path, $body); + + $request = new Request([ + 'url' => '/upload', + 'base' => '/', + 'method' => 'PATCH', + 'type' => 'multipart/form-data; boundary=' . $invalidBoundary, + 'stream_path' => $path, + 'data' => new Collection(), + 'query' => new Collection(), + 'files' => new Collection(), + ]); + + $this->assertEquals(['alpha' => '1', 'beta' => '2'], $request->data->getData()); + $this->assertSame([], $request->files->getData()); + } +} From 049d445c58cbaccff9965dff926fb3d8a3bbbdc2 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Wed, 22 Oct 2025 01:26:54 +0900 Subject: [PATCH 16/18] fix: Support file uploads for non-POST HTTP methods (PATCH, PUT, DELETE) --- flight/net/UploadedFile.php | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/flight/net/UploadedFile.php b/flight/net/UploadedFile.php index 2b3947b..07e0af2 100644 --- a/flight/net/UploadedFile.php +++ b/flight/net/UploadedFile.php @@ -114,15 +114,32 @@ class UploadedFile throw new Exception($this->getUploadErrorMessage($this->error)); } + // Check if this is a legitimate uploaded file (POST method uploads) $isUploadedFile = is_uploaded_file($this->tmpName) === true; - if ( - $isUploadedFile === true - && - move_uploaded_file($this->tmpName, $targetPath) === false - ) { - throw new Exception('Cannot move uploaded file'); // @codeCoverageIgnore - } elseif ($isUploadedFile === false && getenv('PHPUNIT_TEST')) { + + if ($isUploadedFile === true) { + // Standard POST upload - use move_uploaded_file for security + if (move_uploaded_file($this->tmpName, $targetPath) === false) { + throw new Exception('Cannot move uploaded file'); // @codeCoverageIgnore + } + } elseif (getenv('PHPUNIT_TEST')) { rename($this->tmpName, $targetPath); + } elseif (file_exists($this->tmpName) === true && is_readable($this->tmpName) === true) { + // Handle non-POST uploads (PATCH, PUT, DELETE) or other valid temp files + // Verify the file is in a valid temp directory for security + $tempDir = sys_get_temp_dir(); + $uploadTmpDir = ini_get('upload_tmp_dir') ?: $tempDir; + + if (strpos(realpath($this->tmpName), realpath($uploadTmpDir)) === 0 || + strpos(realpath($this->tmpName), realpath($tempDir)) === 0) { + if (rename($this->tmpName, $targetPath) === false) { + throw new Exception('Cannot move uploaded file'); + } + } else { + throw new Exception('Invalid temporary file location'); + } + } else { + throw new Exception('Temporary file does not exist or is not readable'); } } From b174bdbc6f4312b0bebb0f6172f00517249a8e26 Mon Sep 17 00:00:00 2001 From: n0nag0n Date: Mon, 27 Oct 2025 08:01:01 -0600 Subject: [PATCH 17/18] more changes with upload. Security enhancements. Also got rid of php-watcher --- composer.json | 3 +- flight/net/UploadedFile.php | 71 ++++++++++++++++++++++------------- tests/UploadedFileTest.php | 74 +++++++++++++++++++++++++++++++++++-- 3 files changed, 118 insertions(+), 30 deletions(-) diff --git a/composer.json b/composer.json index 96d45db..2e6ca74 100644 --- a/composer.json +++ b/composer.json @@ -51,7 +51,6 @@ "phpstan/phpstan": "^2.1", "phpunit/phpunit": "^9.6", "rregeer/phpunit-coverage-check": "^0.3.1", - "spatie/phpunit-watcher": "^1.23 || ^1.24", "squizlabs/php_codesniffer": "^3.11" }, "config": { @@ -62,7 +61,7 @@ "sort-packages": true }, "scripts": { - "test": "vendor/bin/phpunit-watcher watch", + "test": "phpunit", "test-ci": "phpunit", "test-coverage": "rm -f clover.xml && XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html=coverage --coverage-clover=clover.xml && vendor/bin/coverage-check clover.xml 100", "test-server": "echo \"Running Test Server\" && php -S localhost:8000 -t tests/server/", diff --git a/flight/net/UploadedFile.php b/flight/net/UploadedFile.php index 07e0af2..c2f3ad3 100644 --- a/flight/net/UploadedFile.php +++ b/flight/net/UploadedFile.php @@ -33,6 +33,11 @@ class UploadedFile */ private int $error; + /** + * @var bool $isPostUploadedFile Indicates if the file was uploaded via POST method. + */ + private bool $isPostUploadedFile = false; + /** * Constructs a new UploadedFile object. * @@ -41,14 +46,20 @@ class UploadedFile * @param int $size The size of the uploaded file in bytes. * @param string $tmpName The temporary name of the uploaded file. * @param int $error The error code associated with the uploaded file. + * @param bool|null $isPostUploadedFile Indicates if the file was uploaded via POST method. */ - public function __construct(string $name, string $mimeType, int $size, string $tmpName, int $error) + public function __construct(string $name, string $mimeType, int $size, string $tmpName, int $error, ?bool $isPostUploadedFile = null) { $this->name = $name; $this->mimeType = $mimeType; $this->size = $size; $this->tmpName = $tmpName; $this->error = $error; + if (is_uploaded_file($tmpName) === true) { + $this->isPostUploadedFile = true; // @codeCoverageIgnore + } else { + $this->isPostUploadedFile = $isPostUploadedFile ?? false; + } } /** @@ -114,32 +125,42 @@ class UploadedFile throw new Exception($this->getUploadErrorMessage($this->error)); } + if (is_writeable(dirname($targetPath)) === false) { + throw new Exception('Target directory is not writable'); + } + + // Prevent path traversal attacks + if (strpos($targetPath, '..') !== false) { + throw new Exception('Invalid target path: contains directory traversal'); + } + // Prevent absolute paths (basic check for Unix/Windows) + if ($targetPath[0] === '/' || (strlen($targetPath) > 1 && $targetPath[1] === ':')) { + throw new Exception('Invalid target path: absolute paths not allowed'); + } + + // Prevent overwriting existing files + if (file_exists($targetPath)) { + throw new Exception('Target file already exists'); + } + // Check if this is a legitimate uploaded file (POST method uploads) - $isUploadedFile = is_uploaded_file($this->tmpName) === true; - - if ($isUploadedFile === true) { + $isUploadedFile = $this->isPostUploadedFile; + + // Prevent symlink attacks for non-POST uploads + if (!$isUploadedFile && is_link($this->tmpName)) { + throw new Exception('Invalid temp file: symlink detected'); + } + + $uploadFunctionToCall = $isUploadedFile === true ? // Standard POST upload - use move_uploaded_file for security - if (move_uploaded_file($this->tmpName, $targetPath) === false) { - throw new Exception('Cannot move uploaded file'); // @codeCoverageIgnore - } - } elseif (getenv('PHPUNIT_TEST')) { - rename($this->tmpName, $targetPath); - } elseif (file_exists($this->tmpName) === true && is_readable($this->tmpName) === true) { - // Handle non-POST uploads (PATCH, PUT, DELETE) or other valid temp files - // Verify the file is in a valid temp directory for security - $tempDir = sys_get_temp_dir(); - $uploadTmpDir = ini_get('upload_tmp_dir') ?: $tempDir; - - if (strpos(realpath($this->tmpName), realpath($uploadTmpDir)) === 0 || - strpos(realpath($this->tmpName), realpath($tempDir)) === 0) { - if (rename($this->tmpName, $targetPath) === false) { - throw new Exception('Cannot move uploaded file'); - } - } else { - throw new Exception('Invalid temporary file location'); - } - } else { - throw new Exception('Temporary file does not exist or is not readable'); + 'move_uploaded_file' : + // Handle non-POST uploads (PATCH, PUT, DELETE) or other valid temp files + 'rename'; + + $result = $uploadFunctionToCall($this->tmpName, $targetPath); + + if ($result === false) { + throw new Exception('Cannot move uploaded file'); } } diff --git a/tests/UploadedFileTest.php b/tests/UploadedFileTest.php index 05cdc95..f64e155 100644 --- a/tests/UploadedFileTest.php +++ b/tests/UploadedFileTest.php @@ -18,14 +18,24 @@ class UploadedFileTest extends TestCase if (file_exists('tmp_name')) { unlink('tmp_name'); } + if (file_exists('existing.txt')) { + unlink('existing.txt'); + } + if (file_exists('real_file')) { + unlink('real_file'); + } + + // not found with file_exists...just delete it brute force + @unlink('tmp_symlink'); } - public function testMoveToSuccess(): void + public function testMoveToFalseSuccess(): void { + // This test would have passed in the real world but we can't actually force a post request in unit tests file_put_contents('tmp_name', 'test'); - $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK); + $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, true); + $this->expectExceptionMessage('Cannot move uploaded file'); $uploadedFile->moveTo('file.txt'); - $this->assertFileExists('file.txt'); } public function getFileErrorMessageTests(): array @@ -53,4 +63,62 @@ class UploadedFileTest extends TestCase $this->expectExceptionMessage($message); $uploadedFile->moveTo('file.txt'); } + + public function testMoveToBadLocation(): void + { + file_put_contents('tmp_name', 'test'); + $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, true); + $this->expectExceptionMessage('Target directory is not writable'); + $uploadedFile->moveTo('/root/file.txt'); + } + + public function testMoveToSuccessNonPost(): void + { + file_put_contents('tmp_name', 'test'); + $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false); + $uploadedFile->moveTo('file.txt'); + $this->assertFileExists('file.txt'); + $this->assertEquals('test', file_get_contents('file.txt')); + } + + public function testMoveToPathTraversal(): void + { + file_put_contents('tmp_name', 'test'); + $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false); + $this->expectException(Exception::class); + $this->expectExceptionMessage('Invalid target path: contains directory traversal'); + $uploadedFile->moveTo('../file.txt'); + } + + public function testMoveToAbsolutePath(): void + { + file_put_contents('tmp_name', 'test'); + $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false); + $this->expectException(Exception::class); + $this->expectExceptionMessage('Invalid target path: absolute paths not allowed'); + $uploadedFile->moveTo('/tmp/file.txt'); + } + + public function testMoveToOverwrite(): void + { + file_put_contents('tmp_name', 'test'); + file_put_contents('existing.txt', 'existing'); + $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false); + $this->expectException(Exception::class); + $this->expectExceptionMessage('Target file already exists'); + $uploadedFile->moveTo('existing.txt'); + } + + public function testMoveToSymlinkNonPost(): void + { + file_put_contents('real_file', 'test'); + if (file_exists('tmp_symlink')) { + unlink('tmp_symlink'); + } + symlink('real_file', 'tmp_symlink'); + $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_symlink', UPLOAD_ERR_OK, false); + $this->expectException(Exception::class); + $this->expectExceptionMessage('Invalid temp file: symlink detected'); + $uploadedFile->moveTo('file.txt'); + } } From 09cb95c6e88b724bf8910c5fba749d07fb3ba501 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Tue, 28 Oct 2025 10:45:04 +0900 Subject: [PATCH 18/18] revert: Re-add string type hint to Response::write() parameter --- flight/net/Response.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flight/net/Response.php b/flight/net/Response.php index 6d3da1c..9587ddf 100644 --- a/flight/net/Response.php +++ b/flight/net/Response.php @@ -237,7 +237,7 @@ class Response * * @return $this Self reference */ - public function write($str, bool $overwrite = false): self + public function write(string $str, bool $overwrite = false): self { if ($overwrite === true) { $this->clearBody();