Remove FormMiddleware / FormException

These are not handled via HttpErrorException.
amphp · Aug 22, 2023 · dc1c89a · dc1c89a
1 parent 7e8772e
commit dc1c89a
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 140 deletions.
diff --git 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
diff --git a/src/FormMiddleware.php b/src/FormMiddleware.php
diff --git 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
@@ -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
diff --git 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);
+    }
 }