diff --git a/AGENTS.md b/AGENTS.md index 620a8f8..6eecaea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,9 @@ - Handle `preg_*` engine failures: when the result is used as data, test for `false` or `null` and throw a `\RuntimeException` including `preg_last_error_msg()`; boolean validation guards must compare strictly, such - as `=== 1`, so an engine failure can only ever fail closed. + as `=== 1`, so an engine failure can only ever fail closed. Diagnostic + escaping is the narrow exception: use a deterministic bytewise fallback rather + than throwing, so it cannot obscure the original exception. - Anchor validation patterns to the true end of input with the `D` modifier or `\z`; a bare `$` accepts a trailing newline. - Never embed raw control bytes in exception messages and other diagnostics; diff --git a/CHANGELOG.md b/CHANGELOG.md index d2d18a7..a62a29d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Drop support for PHP 7.2 and 7.3 * Require `guzzlehttp/guzzle` ^8.0, `guzzlehttp/promises` ^3.0, and `guzzlehttp/psr7` ^3.0 +* Escape unsafe command names and previous messages in generated command exceptions * Reject native PHP serialization of `ServiceClient` * Require custom implementations and subclasses of public command APIs to match native method signatures * Require service client response transformers to return `ResultInterface` values diff --git a/src/Exception/CommandException.php b/src/Exception/CommandException.php index 5dff139..4dfd390 100644 --- a/src/Exception/CommandException.php +++ b/src/Exception/CommandException.php @@ -8,6 +8,7 @@ use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\Exception\ResponseException; use GuzzleHttp\Exception\TransferException; +use GuzzleHttp\Psr7\DiagnosticValue; use Psr\Http\Client\NetworkExceptionInterface; use Psr\Http\Client\RequestExceptionInterface; use Psr\Http\Message\RequestInterface; @@ -54,8 +55,7 @@ public static function fromPrevious(CommandInterface $command, \Exception $prev) } // Prepare the message. - $message = 'There was an error executing the '.$command->getName() - .' command: '.$prev->getMessage(); + $message = \sprintf('There was an error executing the %s command: %s', DiagnosticValue::escape($command->getName()), DiagnosticValue::escape($prev->getMessage())); // Create the exception. return new $class($message, $command, $prev, $request, $response); diff --git a/tests/Exception/CommandExceptionTest.php b/tests/Exception/CommandExceptionTest.php index 2b33eca..c0bafb5 100644 --- a/tests/Exception/CommandExceptionTest.php +++ b/tests/Exception/CommandExceptionTest.php @@ -52,6 +52,23 @@ public function testFactoryReturnsExceptionIfAlreadyCommandException(): void $this->assertSame($previous, $exception); } + public function testFactoryEscapesUnsafeCommandAndPreviousMessages(): void + { + $commandName = "command\xC2\x80"; + $previousMessage = "failure\xFF"; + $command = $this->createMock(CommandInterface::class); + $command->method('getName')->willReturn($commandName); + $previous = new \RuntimeException($previousMessage); + + $exception = CommandException::fromPrevious($command, $previous); + + $this->assertSame('There was an error executing the command\\x80 command: failure\\xFF', $exception->getMessage()); + $this->assertSame($command, $exception->getCommand()); + $this->assertSame($commandName, $command->getName()); + $this->assertSame($previous, $exception->getPrevious()); + $this->assertSame($previousMessage, $previous->getMessage()); + } + public function testFactoryReturnsClientExceptionFor400LevelStatusCode(): void { $command = $this->createMock(CommandInterface::class);