diff --git a/packages/grpc/src/Metadata/Status.php b/packages/grpc/src/Metadata/Status.php index f3cc260..7f3720d 100644 --- a/packages/grpc/src/Metadata/Status.php +++ b/packages/grpc/src/Metadata/Status.php @@ -27,7 +27,11 @@ public function append(Metadata $md): Metadata { $md = $md ->replace(self::STATUS_HEADER, (string) $this->code->value) - ->replace(self::MESSAGE_HEADER, $this->message ?? ''); + ->replace(self::MESSAGE_HEADER, (string) preg_replace_callback( + '/[^\x20-\x24\x26-\x7e]/', + static fn(array $byte): string => \sprintf('%%%02X', \ord($byte[0])), + $this->message ?? '', + )); if ($this->details !== null && $this->details !== '') { $md = $md->replace(self::DETAILS_HEADER, $this->details); @@ -46,7 +50,7 @@ function parseStatus(Metadata $md): Status return new Status( $code, - $md->value(Status::MESSAGE_HEADER), + ($message = $md->value(Status::MESSAGE_HEADER)) !== null ? rawurldecode($message) : null, $md->value(Status::DETAILS_HEADER), ); } diff --git a/packages/server/src/Server/Internal/Http2/ServerRequestHandler.php b/packages/server/src/Server/Internal/Http2/ServerRequestHandler.php index 7e3500e..ed41275 100644 --- a/packages/server/src/Server/Internal/Http2/ServerRequestHandler.php +++ b/packages/server/src/Server/Internal/Http2/ServerRequestHandler.php @@ -115,10 +115,9 @@ public function handleRequest(Request $request): Response return new Response( status: HttpStatus::OK, headers: $headers->kv, - trailers: new Trailers(Future::complete([ - Metadata\Status::STATUS_HEADER => (string) Rpc\Code::UNIMPLEMENTED->value, - Metadata\Status::MESSAGE_HEADER => $e->getMessage(), - ])), + trailers: new Trailers(Future::complete( + new Metadata()->withKey(new Metadata\Status(Rpc\Code::UNIMPLEMENTED, $e->getMessage()))->kv, + )), ); } diff --git a/tests/Metadata/StatusTest.php b/tests/Metadata/StatusTest.php new file mode 100644 index 0000000..3900c24 --- /dev/null +++ b/tests/Metadata/StatusTest.php @@ -0,0 +1,89 @@ +withKey(new Status(Rpc\Code::INTERNAL, $message)); + + self::assertSame($headerValue, $md->value(Status::MESSAGE_HEADER)); + self::assertSame($message, parseStatus($md)->message); + } + + /** + * @return iterable + */ + public static function provideMessageCases(): iterable + { + yield 'ascii' => [ + 'Unknown method Ping for service echos.api.v1.EchoService: [{}] ~!', + 'Unknown method Ping for service echos.api.v1.EchoService: [{}] ~!', + ]; + + yield 'percent' => [ + '100%', + '100%25', + ]; + + yield 'range boundaries' => [ + " $%&~\x7f", + ' $%25&~%7F', + ]; + + yield 'special status message' => [ + "\t\ntest with whitespace\r\nand Unicode BMP ☺ and non-BMP 😈\t\n", + '%09%0Atest with whitespace%0D%0Aand Unicode BMP %E2%98%BA and non-BMP %F0%9F%98%88%09%0A', + ]; + } + + #[DataProvider('provideParseMessageCases')] + public function testParseMessage(string $headerValue, string $message): void + { + self::assertSame($message, parseStatus(new Metadata()->with(Status::MESSAGE_HEADER, $headerValue))->message); + } + + /** + * @return iterable + */ + public static function provideParseMessageCases(): iterable + { + yield 'lowercase hex' => [ + '%e2%98%ba', + '☺', + ]; + + yield 'invalid escape' => [ + '%zz', + '%zz', + ]; + + yield 'trailing percent' => [ + 'done 100%', + 'done 100%', + ]; + + yield 'plus is not a space' => [ + 'a+b', + 'a+b', + ]; + } + + public function testMissingMessage(): void + { + self::assertNull(parseStatus(new Metadata())->message); + } +} diff --git a/tests/NotImplementedTest.php b/tests/NotImplementedTest.php index a25b81d..bf7ce12 100644 --- a/tests/NotImplementedTest.php +++ b/tests/NotImplementedTest.php @@ -63,4 +63,13 @@ public function testMethodNotImplemented(): void $this->expectExceptionMessage('A grpc error with status code "UNIMPLEMENTED" and message "Unknown method Ping for service echos.api.v1.EchoService" occurred'); $client->invoke(new EchoRequest(), new Invoke('/echos.api.v1.EchoService/Ping', EchoResponse::class, RpcType::Unary)); } + + public function testMethodNotImplementedMessageIsPercentEncoded(): void + { + $client = new Client\Builder()->build(); + + // The client URI layer encodes "%" in the path before it reaches the server, so the server reports the method as "100%25". + $this->expectExceptionMessage('A grpc error with status code "UNIMPLEMENTED" and message "Unknown method 100%25 for service echos.api.v1.EchoService" occurred'); + $client->invoke(new EchoRequest(), new Invoke('/echos.api.v1.EchoService/100%', EchoResponse::class, RpcType::Unary)); + } } diff --git a/tests/UnaryTest.php b/tests/UnaryTest.php index 5b3dafc..fc02d94 100644 --- a/tests/UnaryTest.php +++ b/tests/UnaryTest.php @@ -119,6 +119,28 @@ public function testServerHandlerException(): void self::fail('Client::echo() above should be throw an exception'); } + + public function testSpecialStatusMessage(): void + { + $client = new EchoServiceClient( + new Client\Builder() + ->withUnaryInterceptors(new AuthorizationClientInterceptor('secret')) + ->build(), + ); + + $message = "\t\ntest with whitespace\r\nand Unicode BMP ☺ and non-BMP 😈\t\n"; + + try { + $client->echo(new EchoRequest($message), new Metadata()->with('server-status-message', '1')); + } catch (InvokeError $e) { + self::assertSame(Code::UNKNOWN, $e->statusCode); + self::assertSame($message, $e->statusMessage); + + return; + } + + self::fail('Client::echo() above should be throw an exception'); + } } final readonly class UnaryEchoServer implements EchoServiceServer @@ -138,6 +160,10 @@ public function echo( ]); } + if ($md->value('server-status-message') === '1') { + throw new InvokeError(Code::UNKNOWN, $request->sentence); + } + $sentence = $md->value('server-sentence') ?? $request->sentence; return new EchoResponse($sentence);