From dc1c89a08e8bab83ac90049f5232e29115b9f55e Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Tue, 22 Aug 2023 06:15:31 +0200 Subject: [PATCH] Remove FormMiddleware / FormException These are not handled via HttpErrorException. --- composer.json | 2 +- src/FormException.php | 7 ---- src/FormMiddleware.php | 39 ------------------- src/FormParser.php | 13 ++++--- src/StreamingFormParser.php | 23 +++++------ test/FormMiddlewareTest.php | 76 ------------------------------------- test/FormTest.php | 64 +++++++++++++++++++++++++++++++ 7 files changed, 84 insertions(+), 140 deletions(-) delete mode 100644 src/FormException.php delete mode 100644 src/FormMiddleware.php delete mode 100644 test/FormMiddlewareTest.php diff --git a/composer.json b/composer.json index 601924d..650b1c7 100644 --- a/composer.json +++ b/composer.json @@ -34,7 +34,7 @@ "amphp/amp": "^3", "amphp/byte-stream": "^2", "amphp/http": "^2", - "amphp/http-server": "^3", + "amphp/http-server": "^3.1", "amphp/pipeline": "^1", "revolt/event-loop": "^1" }, diff --git a/src/FormException.php b/src/FormException.php deleted file mode 100644 index 286751c..0000000 --- a/src/FormException.php +++ /dev/null @@ -1,7 +0,0 @@ -parser = new FormParser($fieldCountLimit); - } - - public function handleRequest(Request $request, RequestHandler $requestHandler): Response - { - try { - $request->setAttribute(Form::class, $this->parser->parseForm($request, $this->bodySizeLimit)); - } catch (FormException) { - return $this->errorHandler->handleError(HttpStatus::BAD_REQUEST, request: $request); - } - - return $requestHandler->handleRequest($request); - } -} diff --git a/src/FormParser.php b/src/FormParser.php index b8e1563..7d16cc4 100644 --- a/src/FormParser.php +++ b/src/FormParser.php @@ -6,6 +6,7 @@ use Amp\ForbidSerialization; use Amp\Http\Http1\Rfc7230; use Amp\Http\InvalidHeaderException; +use Amp\Http\Server\HttpErrorException; use Amp\Http\Server\Request; /** @@ -86,14 +87,14 @@ public function parseUrlEncodedBody(string $body): Form $value = \urldecode($pair[1] ?? ""); if ($field === '') { - throw new FormException("Empty field name in form data"); + throw new HttpErrorException(400, "Empty field name in form data"); } $fields[$field][] = $value; } if (\str_contains($pair[1] ?? "", "&")) { - throw new FormException("Maximum number of variables exceeded"); + throw new HttpErrorException(400, "Maximum number of variables exceeded"); } return new Form($fields); @@ -122,13 +123,13 @@ public function parseMultipartBody(string $body, string $boundary): Form foreach ($exp as $entry) { if (($position = \strpos($entry, "\r\n\r\n")) === false) { - throw new FormException("No header/body boundary found"); + throw new HttpErrorException(400, "No header/body boundary found"); } try { $headers = Rfc7230::parseHeaderPairs(\substr($entry, 0, $position + 2)); } catch (InvalidHeaderException $e) { - throw new FormException("Invalid headers in body part", 0, $e); + throw new HttpErrorException(400, "Invalid headers in body part", 0, $e); } $headerMap = []; @@ -145,7 +146,7 @@ public function parseMultipartBody(string $body, string $boundary): Form ); if (!$count || !isset($matches[1])) { - throw new FormException("Missing or invalid content disposition"); + throw new HttpErrorException(400, "Missing or invalid content disposition"); } // Ignore Content-Transfer-Encoding as deprecated and hence we won't support it @@ -162,7 +163,7 @@ public function parseMultipartBody(string $body, string $boundary): Form } if (\str_contains($entry, "--$boundary")) { - throw new FormException("Maximum number of variables exceeded"); + throw new HttpErrorException(400, "Maximum number of variables exceeded"); } return new Form($fields, $files); diff --git a/src/StreamingFormParser.php b/src/StreamingFormParser.php index 39c7eb8..4ad101f 100644 --- a/src/StreamingFormParser.php +++ b/src/StreamingFormParser.php @@ -8,6 +8,7 @@ use Amp\ForbidSerialization; use Amp\Http\Http1\Rfc7230; use Amp\Http\InvalidHeaderException; +use Amp\Http\Server\HttpErrorException; use Amp\Http\Server\Request; use Amp\Pipeline\DisposedException; use Amp\Pipeline\Pipeline; @@ -74,13 +75,13 @@ private function incrementalBoundaryParse(Queue $source, ReadableStream $body, s $buffer .= $chunk = $body->read(); if ($chunk === null) { - throw new FormException("Request body ended unexpectedly"); + throw new HttpErrorException(400, "Request body ended unexpectedly"); } } $offset = \strlen($boundarySeparator); if (\strncmp($buffer, $boundarySeparator, $offset)) { - throw new FormException("Invalid boundary"); + throw new HttpErrorException(400, "Invalid boundary"); } $boundarySeparator = "\r\n$boundarySeparator"; @@ -93,18 +94,18 @@ private function incrementalBoundaryParse(Queue $source, ReadableStream $body, s $buffer .= $chunk = $body->read(); if ($chunk === null) { - throw new FormException("Request body ended unexpectedly"); + throw new HttpErrorException(400, "Request body ended unexpectedly"); } } if ($fieldCount++ === $this->fieldCountLimit) { - throw new FormException("Maximum number of variables exceeded"); + throw new HttpErrorException(400, "Maximum number of variables exceeded"); } try { $headers = Rfc7230::parseHeaderPairs(\substr($buffer, $offset, $end + 2 - $offset)); } catch (InvalidHeaderException $e) { - throw new FormException("Invalid headers in body part", 0, $e); + throw new HttpErrorException(400, "Invalid headers in body part", 0, $e); } $headerMap = []; @@ -119,7 +120,7 @@ private function incrementalBoundaryParse(Queue $source, ReadableStream $body, s ); if (!$count || !isset($matches[1])) { - throw new FormException("Invalid content-disposition header within multipart form"); + throw new HttpErrorException(400, "Invalid content-disposition header within multipart form"); } $fieldName = $matches[1]; @@ -150,7 +151,7 @@ private function incrementalBoundaryParse(Queue $source, ReadableStream $body, s $buffer .= $chunk = $body->read(); if ($chunk === null) { - throw new FormException("Request body ended unexpectedly"); + throw new HttpErrorException(400, "Request body ended unexpectedly"); } } @@ -163,7 +164,7 @@ private function incrementalBoundaryParse(Queue $source, ReadableStream $body, s $buffer .= $chunk = $body->read(); if ($chunk === null) { - throw new FormException("Request body ended unexpectedly"); + throw new HttpErrorException(400, "Request body ended unexpectedly"); } } @@ -201,7 +202,7 @@ private function incrementalFieldParse(Queue $source, ReadableStream $body): voi $queue = new Queue(); if ($fieldCount++ === $this->fieldCountLimit) { - throw new FormException("Maximum number of variables exceeded"); + throw new HttpErrorException(400, "Maximum number of variables exceeded"); } $future = $source->pushAsync(new StreamedField( @@ -266,7 +267,7 @@ private function incrementalFieldParse(Queue $source, ReadableStream $body): voi $buffer = \substr($buffer, $nextPos + 1); if ($fieldCount++ === $this->fieldCountLimit) { - throw new FormException("Maximum number of variables exceeded"); + throw new HttpErrorException(400, "Maximum number of variables exceeded"); } $source->push(new StreamedField($fieldName)); @@ -276,7 +277,7 @@ private function incrementalFieldParse(Queue $source, ReadableStream $body): voi if ($buffer) { if ($fieldCount + 1 === $this->fieldCountLimit) { - throw new FormException("Maximum number of variables exceeded"); + throw new HttpErrorException(400, "Maximum number of variables exceeded"); } $source->push(new StreamedField(\urldecode($buffer))); diff --git a/test/FormMiddlewareTest.php b/test/FormMiddlewareTest.php deleted file mode 100644 index 8674b24..0000000 --- a/test/FormMiddlewareTest.php +++ /dev/null @@ -1,76 +0,0 @@ -middleware = new FormMiddleware(new DefaultErrorHandler()); - } - - public function testWwwFormUrlencoded(): void - { - $callback = $this->createCallback(1); - - $handler = stackMiddleware(new ClosureRequestHandler(function (Request $request) use ($callback): Response { - if ($request->hasAttribute(Form::class)) { - $callback(); - - $form = $request->getAttribute(Form::class); - - $this->assertSame('bar', $form->getValue('foo')); - $this->assertSame('y', $form->getValue('x')); - } - - return new Response; - }), $this->middleware); - - $request = new Request($this->createMock(Client::class), 'GET', Uri\Http::createFromString('/'), [ - 'content-type' => 'application/x-www-form-urlencoded', - ], 'foo=bar&x=y'); - - $handler->handleRequest($request); - } - - public function testNonForm(): void - { - $handler = stackMiddleware(new ClosureRequestHandler(function (Request $request): Response { - $this->assertTrue($request->hasAttribute(Form::class)); // attribute is set either way - $this->assertSame('{}', $request->getBody()->buffer()); - return new Response; - }), $this->middleware); - - $request = new Request($this->createMock(Client::class), 'GET', Uri\Http::createFromString('/'), [ - 'content-type' => 'application/json', - ], '{}'); - - $handler->handleRequest($request); - } - - public function testNone(): void - { - $handler = stackMiddleware(new ClosureRequestHandler(function (Request $request): Response { - $this->assertTrue($request->hasAttribute(Form::class)); // attribute is set either way - return new Response; - }), $this->middleware); - - $request = new Request($this->createMock(Client::class), 'GET', Uri\Http::createFromString('/'), [], '{}'); - - $handler->handleRequest($request); - } -} diff --git a/test/FormTest.php b/test/FormTest.php index 440f445..80ba18f 100644 --- a/test/FormTest.php +++ b/test/FormTest.php @@ -2,9 +2,15 @@ namespace Amp\Http\Server\FormParser\Test; +use Amp\Http\Server\Driver\Client; use Amp\Http\Server\FormParser\BufferedFile; use Amp\Http\Server\FormParser\Form; +use Amp\Http\Server\Request; +use Amp\Http\Server\RequestHandler\ClosureRequestHandler; +use Amp\Http\Server\Response; use Amp\PHPUnit\AsyncTestCase; +use League\Uri\Http; +use function Amp\Http\Server\FormParser\parseForm; class FormTest extends AsyncTestCase { @@ -41,4 +47,62 @@ public function testFormWithFiles(): void $this->assertNull($form->getFile("file_not_found")); $this->assertTrue($form->hasFile("file")); } + + public function testWwwFormUrlencoded(): void + { + $callback = $this->createCallback(1); + + $handler = new ClosureRequestHandler(function (Request $request) use ($callback): Response { + $callback(); + + $form = parseForm($request); + + $this->assertSame('bar', $form->getValue('foo')); + $this->assertSame('y', $form->getValue('x')); + + return new Response; + }); + + $request = new Request($this->createMock(Client::class), 'GET', Http::createFromString('/'), [ + 'content-type' => 'application/x-www-form-urlencoded', + ], 'foo=bar&x=y'); + + $handler->handleRequest($request); + } + + public function testNonForm(): void + { + $handler = new ClosureRequestHandler(function (Request $request): Response { + parseForm($request); + + $this->assertTrue($request->hasAttribute(Form::class)); // attribute is set either way + $this->assertSame('{}', $request->getBody()->buffer()); + + return new Response; + }); + + $request = new Request($this->createMock(Client::class), 'GET', Http::createFromString('/'), [ + 'content-type' => 'application/json', + ], '{}'); + + $handler->handleRequest($request); + } + + public function testNone(): void + { + $handler = new ClosureRequestHandler(function (Request $request): Response { + $this->assertFalse($request->hasAttribute(Form::class)); + + $form = parseForm($request); + self::assertSame([], $form->getNames()); + + $this->assertTrue($request->hasAttribute(Form::class)); // attribute is set either way + + return new Response; + }); + + $request = new Request($this->createMock(Client::class), 'GET', Http::createFromString('/'), [], '{}'); + + $handler->handleRequest($request); + } }