Skip to content

Commit

Permalink
Fix psalm issues
Browse files Browse the repository at this point in the history
  • Loading branch information
bwoebi committed Feb 6, 2024
1 parent d5da228 commit 6409275
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 28 deletions.
2 changes: 2 additions & 0 deletions src/Driver/DefaultHttpDriverFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
final class DefaultHttpDriverFactory implements HttpDriverFactory
{
/**
* @param positive-int $headerSizeLimit
* @param non-negative-int $bodySizeLimit
* @param bool $allowHttp2Upgrade Requires HTTP/2 support to be enabled.
* @param bool $pushEnabled Requires HTTP/2 support to be enabled.
*/
Expand Down
29 changes: 22 additions & 7 deletions src/Driver/Http3Driver.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,15 @@

class Http3Driver extends ConnectionHttpDriver
{
/** @psalm-suppress PropertyNotSetInConstructor */
private Client $client;

/** @var \WeakMap<Request, QuicSocket> */
private \WeakMap $requestStreams;

/** @psalm-suppress PropertyNotSetInConstructor */
private Http3Parser $parser;
/** @psalm-suppress PropertyNotSetInConstructor */
private Http3Writer $writer;
private QPack $qpack;
private int $highestStreamId = 0;
Expand All @@ -55,9 +58,13 @@ class Http3Driver extends ConnectionHttpDriver
private array $settings = [];
/** @var DeferredFuture<array<int, int>> */
private DeferredFuture $parsedSettings;
/** @var array<int, \Closure(string $buf, QuicSocket $stream): void */
private array $streamHandlers;
/** @var array<int, \Closure(string $buf, QuicSocket $stream): void> */
private array $streamHandlers = [];

/**
* @param positive-int $headerSizeLimit
* @param non-negative-int $bodySizeLimit
*/
public function __construct(
RequestHandler $requestHandler,
ErrorHandler $errorHandler,
Expand All @@ -69,6 +76,7 @@ public function __construct(
parent::__construct($requestHandler, $errorHandler, $logger);

$this->qpack = new QPack;
/** @var \WeakMap<Request, QuicSocket> See https://github.com/vimeo/psalm/issues/7131 */
$this->requestStreams = new \WeakMap;
$this->closeCancellation = new DeferredCancellation;
$this->settings[Http3Settings::MAX_FIELD_SECTION_SIZE->value] = $this->headerSizeLimit;
Expand All @@ -82,13 +90,13 @@ public function getSettings(): \array
return $this->parsedSettings->getFuture()->await();
}

public function addSetting(Http3Settings|int $setting, int $value)
public function addSetting(Http3Settings|int $setting, int $value): void
{
$this->settings[\is_int($setting) ? $setting : $setting->value] = $value;
}

/** @param \Closure(string $buf, QuicSocket $stream): void $handler */
public function addStreamHandler(int $type, \Closure $handler)
public function addStreamHandler(int $type, \Closure $handler): void
{
$this->streamHandlers[$type] = $handler;
}
Expand Down Expand Up @@ -226,6 +234,7 @@ public function handleConnection(Client $client, QuicConnection|Socket $connecti
{
/** @psalm-suppress RedundantPropertyInitializationCheck */
\assert(!isset($this->client), "The driver has already been setup");
\assert($connection instanceof QuicConnection);

$this->client = $client;
$this->writer = new Http3Writer($connection, $this->settings);
Expand Down Expand Up @@ -404,7 +413,9 @@ public function handleConnection(Client $client, QuicConnection|Socket $connecti
$this->updatePriority($stream, $headers["priority"]);
}

/** @var EventLoop\Suspension|null $dataSuspension */
$dataSuspension = null;
$bodySizeLimit = $this->bodySizeLimit;
$body = new RequestBody(
new ReadableIterableStream($bodyQueue->pipe()),
function (int $bodySize) use (&$bodySizeLimit, &$dataSuspension) {
Expand Down Expand Up @@ -459,6 +470,8 @@ function (int $bodySize) use (&$bodySizeLimit, &$dataSuspension) {
$dataSuspension->suspend();
}
} elseif ($type === Http3Frame::HEADERS) {
[$headers, $pseudo] = $data;

// Trailers must not contain pseudo-headers.
if (!empty($pseudo)) {
throw new Http3StreamException(
Expand Down Expand Up @@ -576,10 +589,12 @@ function (int $bodySize) use (&$bodySizeLimit, &$dataSuspension) {
}
}

public function updatePriority(QuicSocket $socket, array|string $headers)
public function updatePriority(QuicSocket $socket, array|string $headers): void
{
[$urgency, $incremental] = Http3Parser::parsePriority($headers);
$socket->setPriority($urgency + 124 /* 127 is default for QUIC, 3 is default for HTTP */, $incremental);
if ([$urgency, $incremental] = Http3Parser::parsePriority($headers)) {
/** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
$socket->setPriority($urgency + 124 /* 127 is default for QUIC, 3 is default for HTTP */, $incremental);
}
}

public function getPendingRequestCount(): int
Expand Down
2 changes: 1 addition & 1 deletion src/Driver/Internal/Capsules/CapsuleWriter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@

interface CapsuleWriter
{
public function write(int $type, string $buf);
public function write(int $type, string $buf): void;
}
6 changes: 4 additions & 2 deletions src/Driver/Internal/Capsules/DatagramCapsule.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class DatagramCapsule implements CapsuleReader, DatagramStream
{
const TYPE = 0;
private ?Suspension $datagramSuspension = null;
private ?Cancellation $cancellation;
private ?Cancellation $cancellation = null;
private ?string $cancellationId = null;
private ?string $nextDatagram = null;

Expand All @@ -30,14 +30,15 @@ public function read(): ?array
return null;
}

$buf = \implode(\iterator_to_array($content));
$buf = \implode(\is_array($content) ? $content : \iterator_to_array($content));
if (\strlen($buf) < $len) {
return null;
}

if ($this->datagramSuspension) {
$this->datagramSuspension->resume($buf);
$this->datagramSuspension = null;
\assert($this->cancellationId !== null);
$this->cancellation?->unsubscribe($this->cancellationId);
$this->cancellation = null;
} else {
Expand Down Expand Up @@ -75,6 +76,7 @@ public function receive(?Cancellation $cancellation = null): ?string
$this->datagramSuspension = EventLoop::getSuspension();
$this->cancellation = $cancellation;
$this->cancellationId = $cancellation?->subscribe(function ($e) {
\assert($this->datagramSuspension !== null);
$this->datagramSuspension->throw($e);
$this->datagramSuspension = null;
});
Expand Down
5 changes: 3 additions & 2 deletions src/Driver/Internal/Capsules/Rfc9297Reader.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ public function read(): ?array
yield $this->buf;
$length -= \strlen($this->buf);

if (null === ($this->buf = $this->read())) {
if (null === $buf = $this->stream->read()) {
return;
}
$this->buf = $buf;
}
yield \substr($this->buf, 0, $length);
$this->buf = \substr($this->buf, $length);
$this->activeReader = false;
};
return [$type, $length, $reader];
return [$type, $length, $reader()];
}
}
2 changes: 1 addition & 1 deletion src/Driver/Internal/Capsules/Rfc9297Writer.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public function __construct(private WritableStream $writer)
{
}

public function write(int $type, string $buf)
public function write(int $type, string $buf): void
{
$this->writer->write(Http3Writer::encodeVarint($type) . Http3Writer::encodeVarint(\strlen($buf)) . $buf);
}
Expand Down
47 changes: 36 additions & 11 deletions src/Driver/Internal/Http3/Http3Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class Http3Parser
private Queue $queue;
/** @var array<int, EventLoop\Suspension> */
private array $datagramReceivers = [];
/** @psalm-suppress PropertyNotSetInConstructor */
private DeferredCancellation $datagramReceiveEmpty;
/** @var array<int, true> */
private array $datagramCloseHandlerInstalled = [];
Expand Down Expand Up @@ -66,9 +67,11 @@ public static function decodeVarintFromStream(ReadableStream $stream, string &$b
}
$buf .= $chunk;
}
/** @psalm-suppress PossiblyUndefinedVariable https://github.com/vimeo/psalm/issues/10548 */
return $int;
}

/** @param positive-int $headerSizeLimit */
public function __construct(private QuicConnection $connection, private int $headerSizeLimit, private QPack $qpack)
{
$this->queue = new Queue;
Expand All @@ -94,19 +97,27 @@ public static function decodeFrameTypeFromStream(QuicSocket $stream, string &$bu
$off += $length;
$frametype = self::decodeVarintFromStream($stream, $buf, $off);
}
/** @psalm-suppress PossiblyUndefinedVariable https://github.com/vimeo/psalm/issues/10548 */
return $frame;
}

public static function readFullFrame(QuicSocket $stream, string &$buf, int &$off, $maxSize): ?array
/**
* @param positive-int $maxSize
* @return list{Http3Frame, string}|null
*/
public static function readFullFrame(QuicSocket $stream, string &$buf, int &$off, int $maxSize): ?array
{
$type = self::decodeFrameTypeFromStream($stream, $buf, $off);
if (null === $type = self::decodeFrameTypeFromStream($stream, $buf, $off)) {
return null;
}
if (null === $frame = self::readFrameWithoutType($stream, $buf, $off, $maxSize)) {
return null;
}
return [$type, $frame];
}

public static function readFrameWithoutType(QuicSocket $stream, string &$buf, int &$off, $maxSize): ?string
/** @param positive-int $maxSize */
public static function readFrameWithoutType(QuicSocket $stream, string &$buf, int &$off, int $maxSize): ?string
{
$length = self::decodeVarintFromStream($stream, $buf, $off);
if ($length < 0) {
Expand Down Expand Up @@ -168,6 +179,7 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
return;
}
if ($frame === Http3Frame::PUSH_PROMISE) {
/** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
yield Http3Frame::PUSH_PROMISE => $this->parsePushPromise($contents);
} else {
break;
Expand All @@ -177,6 +189,7 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
throw new Http3ConnectionException("A request or response stream may not start with any other frame than HEADERS", Http3Error::H3_FRAME_UNEXPECTED);
}
$headerOff = 0;
/** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
yield Http3Frame::HEADERS => self::processHeaders($this->qpack->decode($contents, $headerOff));
$hadData = false;
while (null !== $type = self::decodeFrameTypeFromStream($stream, $buf, $off)) {
Expand Down Expand Up @@ -208,19 +221,20 @@ private function readHttpMessage(QuicSocket $stream, string &$buf, int &$off): \
$buf = "";
$off = 0;
while (true) {
if (null === $buf = $stream->read()) {
if (null === $chunk = $stream->read()) {
if ($stream->wasReset()) {
yield null => null;
} else {
throw new Http3ConnectionException("Received an incomplete data frame", Http3Error::H3_FRAME_ERROR);
}
return;
}
if (\strlen($buf) < $length) {
yield Http3Frame::DATA => $buf;
$length -= \strlen($buf);
if (\strlen($chunk) < $length) {
yield Http3Frame::DATA => $chunk;
$length -= \strlen($chunk);
} else {
yield Http3Frame::DATA => \substr($buf, 0, $length);
yield Http3Frame::DATA => \substr($chunk, 0, $length);
$buf = $chunk;
$off = $length;
break;
}
Expand All @@ -244,9 +258,11 @@ 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);
} else {
throw new Http3ConnectionException("Expecting only push promises after a message frame, found {$frame->type}", Http3Error::H3_FRAME_UNEXPECTED);
/** @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);
}
}
}
Expand Down Expand Up @@ -281,7 +297,8 @@ private static function processHeaders(array $decoded): ?array
return [$headers, $pseudo];
}

/** @return ConcurrentIterator<list{Http3Frame::HEADERS, QuicSocket, \Generator}|list{Http3Frame::GOAWAY|Http3Frame::MAX_PUSH_ID|Http3Frame::CANCEL_PUSH, int}|list{Http3Frame::PRIORITY_UPDATE_Push|Http3Frame::PRIORITY_UPDATE_Request, int, string}|list{Http3Frame::PUSH_PROMISE, int, callable(): \Generator}|list{int, string, QuicSocket}> */
// I'm unable to suppress https://github.com/vimeo/psalm/issues/10669
/* @return ConcurrentIterator<list{Http3Frame::HEADERS, QuicSocket, \Generator}|list{Http3Frame::GOAWAY|Http3Frame::MAX_PUSH_ID|Http3Frame::CANCEL_PUSH, int}|list{Http3Frame::PRIORITY_UPDATE_Push|Http3Frame::PRIORITY_UPDATE_Request, int, string}|list{Http3Frame::PUSH_PROMISE, int, callable(): \Generator}|list{int, string, QuicSocket}> */
public function process(): ConcurrentIterator
{
EventLoop::queue(function () {
Expand All @@ -290,6 +307,10 @@ public function process(): ConcurrentIterator
try {
$off = 0;
$buf = $stream->read();
if ($buf === null) {
return; // Nothing happens. That's allowed. Just bye then.
}

$type = self::decodeVarintFromStream($stream, $buf, $off);
if ($stream->isWritable()) {
// client-initiated bidirectional stream
Expand Down Expand Up @@ -320,6 +341,7 @@ public function process(): ConcurrentIterator
if ($frame !== Http3Frame::SETTINGS) {
throw new Http3ConnectionException("A settings frame must be the first frame on the control stream", Http3Error::H3_MISSING_SETTINGS);
}
/** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
$this->parseSettings($contents);

while (true) {
Expand All @@ -335,6 +357,7 @@ public function process(): ConcurrentIterator
}

$tmpOff = 0;
/** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10668 */
if (0 > $id = self::decodeVarint($contents, $tmpOff)) {
if (!$stream->getConnection()->isClosed()) {
throw new Http3ConnectionException("The control stream was closed", Http3Error::H3_CLOSED_CRITICAL_STREAM);
Expand Down Expand Up @@ -394,6 +417,7 @@ public function process(): ConcurrentIterator
}

// Note: format is shared with HTTP/2
/** @return list{int<0, 7>, bool}|null */
public static function parsePriority(array|string $headers): ?array
{
$urgency = 3;
Expand All @@ -404,7 +428,7 @@ public static function parsePriority(array|string $headers): ?array
if ($number instanceof Number) {
$value = $number->item;
if (\is_int($value) && $value >= 0 && $value <= 7) {
$urgency = $number->item;
$urgency = $value;
}
}
}
Expand Down Expand Up @@ -491,6 +515,7 @@ public function receiveDatagram(QuicSocket $stream, ?Cancellation $cancellation
try {
return $suspension->suspend();
} finally {
/** @psalm-suppress PossiblyNullArgument https://github.com/vimeo/psalm/issues/10553 */
$cancellation?->unsubscribe($cancellationId);
}
}
Expand Down
12 changes: 10 additions & 2 deletions src/Driver/Internal/Http3/QPack.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ class QPack
["x-frame-options", "sameorigin"],
];

/** @return positive-int */
/**
* @psalm-suppress MoreSpecificReturnType
* @return non-negative-int
*/
private static function decodeDynamicInteger(string $input, int $maxBits, int &$off): int
{
if (!isset($input[$off])) {
Expand All @@ -123,6 +126,7 @@ private static function decodeDynamicInteger(string $input, int $maxBits, int &$

$int = \ord($input[$off++]) & $maxBits;
if ($maxBits !== $int) {
/** @psalm-suppress LessSpecificReturnStatement https://github.com/vimeo/psalm/issues/10667 */
return $int;
}

Expand All @@ -141,6 +145,7 @@ private static function decodeDynamicInteger(string $input, int $maxBits, int &$
}
} while ($c & 0x80);

/** @psalm-suppress InvalidReturnStatement https://github.com/vimeo/psalm/issues/9902 */
return $int;
}

Expand All @@ -156,7 +161,10 @@ public static function decodeDynamicField(string $input, int $startBits, int &$o
$off += $length;

if ($huffman) {
return HPackNative::huffmanDecode($string);
$string = HPackNative::huffmanDecode($string);
if ($string === null) {
throw new QPackException(Http3Error::QPACK_DECOMPRESSION_FAILED, 'Invalid huffman encoded sequence');
}
}

return $string;
Expand Down
2 changes: 1 addition & 1 deletion src/Driver/Internal/Http3/QPackException.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

class QPackException extends \Exception
{
public function __construct(public Http3Error $error, $message = null)
public function __construct(public Http3Error $error, string $message = null)
{
parent::__construct($message ?? "Encoding error of type: " . $error->name);
}
Expand Down
Loading

0 comments on commit 6409275

Please sign in to comment.