Add tests for Http3Driver

upgrading() tests still pending
amphp · Feb 11, 2024 · e0e2bc1 · e0e2bc1
1 parent 12aecf8
commit e0e2bc1
Show file tree

Hide file tree

Showing 8 changed files with 609 additions and 130 deletions.
diff --git a/src/Driver/Http2Driver.php b/src/Driver/Http2Driver.php
@@ -888,8 +888,6 @@ public function handleHeaders(int $streamId, array $pseudo, array $headers, bool
                             Http2Parser::PROTOCOL_ERROR
                         );
                     }
-                    // Stuff it into the headers for applications to read
-                    $headers[":protocol"] = [$value];
                 } else {
                     throw new Http2StreamException(
                         "Invalid pseudo header",
@@ -1063,7 +1061,9 @@ public function handleHeaders(int $streamId, array $pseudo, array $headers, bool
                 $uri,
                 $headers,
                 "",
-                "2"
+                "2",
+                null,
+                $pseudo[":protocol"] ?? "",
             );
 
             $this->streamIdMap[\spl_object_hash($request)] = $streamId;
@@ -1139,7 +1139,8 @@ function (int $bodySize) use ($streamId) {
             $headers,
             $body,
             "2",
-            $trailers
+            $trailers,
+            $pseudo[":protocol"] ?? "",
         );
 
         $this->streamIdMap[\spl_object_hash($request)] = $streamId;

diff --git a/src/Driver/Http3Driver.php b/src/Driver/Http3Driver.php
diff --git a/src/Driver/Internal/Http3/Http3Parser.php b/src/Driver/Internal/Http3/Http3Parser.php
@@ -167,11 +167,11 @@ public function awaitHttpResponse(QuicSocket $stream): \Generator
         return $this->readHttpMessage($stream, $buf, $off);
     }
 
-    private function parsePushPromise(string $contents): array
+    private function parsePushPromise(QuicSocket $stream, string $contents): array
     {
         $pushOff = 0;
         $pushId = self::decodeVarint($contents, $pushOff);
-        return [$pushId, self::processHeaders($this->qpack->decode($contents, $pushOff))];
+        return [$pushId, self::processHeaders($stream, $this->qpack->decode($contents, $pushOff))];
     }
 
     private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \Generator
@@ -182,7 +182,7 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
             }
             if ($frame === Http3Frame::PUSH_PROMISE) {
                 /** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
-                yield Http3Frame::PUSH_PROMISE => $this->parsePushPromise($contents);
+                yield Http3Frame::PUSH_PROMISE => $this->parsePushPromise($stream, $contents);
             } else {
                 break;
             }
@@ -192,7 +192,7 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
         }
         $headerOff = 0;
         /** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
-        yield Http3Frame::HEADERS => self::processHeaders($this->qpack->decode($contents, $headerOff));
+        yield Http3Frame::HEADERS => self::processHeaders($stream, $this->qpack->decode($contents, $headerOff));
         $hadData = false;
         while (null !== $type = self::decodeFrameTypeFromStream($stream, $buf, $off)) {
             switch ($type) {
@@ -202,7 +202,7 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
                         return;
                     }
                     $headerOff = 0;
-                    yield Http3Frame::HEADERS => self::processHeaders($this->qpack->decode($headers, $headerOff));
+                    yield Http3Frame::HEADERS => self::processHeaders($stream, $this->qpack->decode($headers, $headerOff));
                     if ($hadData) {
                         break 2;
                     }
@@ -248,7 +248,7 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
                     if (!$headers = self::readFrameWithoutType($stream, $buf, $off, $this->headerSizeLimit)) {
                         return;
                     }
-                    yield Http3Frame::PUSH_PROMISE => $this->parsePushPromise($headers);
+                    yield Http3Frame::PUSH_PROMISE => $this->parsePushPromise($stream, $headers);
                     break;
 
                 default:
@@ -261,7 +261,7 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
         while ([$frame, $contents] = self::readFullFrame($stream, $buf, $off, $this->headerSizeLimit)) {
             if ($frame === Http3Frame::PUSH_PROMISE) {
                 /** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
-                yield Http3Frame::PUSH_PROMISE => $this->parsePushPromise($contents);
+                yield Http3Frame::PUSH_PROMISE => $this->parsePushPromise($stream, $contents);
             } else {
                 /** @psalm-suppress PossiblyNullPropertyFetch https://github.com/vimeo/psalm/issues/10668 */
                 throw new Http3ConnectionException("Expecting only push promises after a message frame, found {$frame->name}", Http3Error::H3_FRAME_UNEXPECTED);
@@ -270,23 +270,23 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
     }
 
     // TODO extracted from Http2Parser::parseHeaderBuffer. Same rules, make common method?
-    private static function processHeaders(array $decoded): ?array
+    private static function processHeaders(QuicSocket $stream, array $decoded): ?array
     {
         $headers = [];
         $pseudo = [];
 
         foreach ($decoded as [$name, $value]) {
             if (!\preg_match('/^[\x21-\x40\x5b-\x7e]+$/'/* Http2Parser::HEADER_NAME_REGEX */, $name)) {
-                return null;
+                throw new Http3StreamException("Invalid header name format", $stream, Http3Error::H3_MESSAGE_ERROR);
             }
 
             if ($name[0] === ':') {
                 if (!empty($headers)) {
-                    return null;
+                    throw new Http3StreamException("Found pseudo-headers after other headers", $stream, Http3Error::H3_MESSAGE_ERROR);
                 }
 
                 if (isset($pseudo[$name])) {
-                    return null;
+                    throw new Http3StreamException("Found duplicate pseudo-header", $stream, Http3Error::H3_MESSAGE_ERROR);
                 }
 
                 $pseudo[$name] = $value;
@@ -412,6 +412,8 @@ public function process(): ConcurrentIterator
                         }
                     } catch (Http3ConnectionException $e) {
                         $this->abort($e);
+                    } catch (Http3StreamException $e) {
+                        $e->getStream()->close($e->getCode());
                     }
                 });
             }

diff --git a/src/Driver/Internal/Http3/Http3Writer.php b/src/Driver/Internal/Http3/Http3Writer.php
@@ -5,11 +5,14 @@
 use Amp\Quic\QuicConnection;
 use Amp\Quic\QuicSocket;
 
+/**
+ * @psalm-import-type HeaderArray from \Amp\Http\Server\Driver\Internal\Http3\QPack
+ */
 class Http3Writer
 {
     private QuicSocket $controlStream;
 
-    public function __construct(private QuicConnection $connection, private array $settings)
+    public function __construct(private QuicConnection $connection, private array $settings, private QPack $qpack)
     {
         $this->startControlStream();
     }
@@ -38,9 +41,15 @@ private static function sendKnownFrame(QuicSocket $stream, Http3Frame $type, str
         self::sendFrame($stream, $type->value, $payload);
     }
 
-    public function sendHeaderFrame(QuicSocket $stream, string $payload): void
+    /** @param HeaderArray $headers */
+    public function sendHeaderFrame(QuicSocket $stream, array $headers): void
     {
-        self::sendKnownFrame($stream, Http3Frame::HEADERS, $payload);
+        self::sendKnownFrame($stream, Http3Frame::HEADERS, $this->qpack->encode($headers));
+    }
+
+    public function sendPushPromiseFrame(QuicSocket $stream, int $pushId, array $headers): void
+    {
+        self::sendKnownFrame($stream, Http3Frame::PUSH_PROMISE, self::encodeVarint($pushId) . $this->qpack->encode($headers));
     }
 
     public function sendData(QuicSocket $stream, string $payload): void

diff --git a/src/Driver/Internal/Http3/QPack.php b/src/Driver/Internal/Http3/QPack.php
@@ -6,7 +6,7 @@
 
 /**
  * @internal
- * @psalm-import-type HeaderArray from \Amp\Http\HPack
+ * @psalm-type HeaderArray = array<string, list<string>>
  */
 class QPack
 {
@@ -255,8 +255,10 @@ public function encode(array $headers): string
     {
         // @TODO implementation is deliberately primitive... [doesn't use any dynamic table...]
         $encodedHeaders = [];
-        foreach ($headers as [$name, $value]) {
-            $encodedHeaders[] = ("\x20" | self::encodeDynamicField(0x07, $name)) . self::encodeDynamicField(0x7F, $value);
+        foreach ($headers as $name => $values) {
+            foreach ($values as $value) {
+                $encodedHeaders[] = ("\x20" | self::encodeDynamicField(0x07, $name)) . self::encodeDynamicField(0x7F, $value);
+            }
         }
 
         return "\0\0" . \implode($encodedHeaders);

diff --git a/src/Request.php b/src/Request.php
@@ -30,7 +30,7 @@ final class Request extends HttpRequest
      * @param non-empty-string $method HTTP request method.
      * @param PsrUri $uri The full URI being requested, including host, port, and protocol.
      * @param array<non-empty-string, string|string[]> $headers An array of strings or an array of string arrays.
-     * @param string $protocol HTTP protocol version (e.g. 1.0, 1.1, or 2.0).
+     * @param string $protocolVersion HTTP protocol version (e.g. 1.0, 1.1, or 2.0).
      * @param Trailers|null $trailers Trailers if request has trailers, or null otherwise.
      */
     public function __construct(
@@ -39,8 +39,9 @@ public function __construct(
         PsrUri $uri,
         array $headers = [],
         ReadableStream|string $body = '',
-        private string $protocol = "1.1",
-        ?Trailers $trailers = null
+        private string $protocolVersion = "1.1",
+        ?Trailers $trailers = null,
+        private string $protocol = "",
     ) {
         parent::__construct($method, $uri);
 
@@ -86,14 +87,31 @@ public function setUri(PsrUri $uri): void
      * it has nothing to do with URI schemes like http:// or https:// ...
      */
     public function getProtocolVersion(): string
+    {
+        return $this->protocolVersion;
+    }
+
+    /**
+     * Sets a new protocol version number for the request.
+     */
+    public function setProtocolVersion(string $protocolVersion): void
+    {
+        $this->protocolVersion = $protocolVersion;
+    }
+
+    /**
+     * This method returns the :protocol pseudo-header;
+     * it has nothing to do with versions nor URI schemes.
+     */
+    public function getProtocol(): string
     {
         return $this->protocol;
     }
 
     /**
      * Sets a new protocol version number for the request.
      */
-    public function setProtocolVersion(string $protocol): void
+    public function setProtocol(string $protocol): void
     {
         $this->protocol = $protocol;
     }