From 58db362050f1d180b33791edee30dc19f574ac0c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Jun 2026 20:09:31 +0000 Subject: [PATCH] Surface raw body in BringApiException when error envelope is unparsable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring returns several 4xx shapes beyond the documented errors/messages JSON envelope — plain text, HTML pages, JSON variants from upstream gateways. In those cases the parser silently dropped the body and the caller saw only "HTTP 400 (no parsable error body)", with no clue what Bring was actually complaining about. Now the fallback synthetic error carries a redacted, single-line, 500-char-capped snippet of the body so the real rejection reason reaches the operator. Credential-shaped values (api_key, apikey, x-mybring-api-key, authorization, password, secret, token) are redacted in both JSON-property and form-value shapes. The parseable-envelope path is unchanged: getMessage() still embeds only the first parsed error, preserving the existing guarantee that Bring's potentially credential-echoing response bodies don't leak via getMessage(). --- src/v4/Exception/BringApiException.php | 53 +++++++++++++++++++++++++- tests/v4/Http/TransportTest.php | 39 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/src/v4/Exception/BringApiException.php b/src/v4/Exception/BringApiException.php index 24f9623..07a5813 100644 --- a/src/v4/Exception/BringApiException.php +++ b/src/v4/Exception/BringApiException.php @@ -102,9 +102,60 @@ public static function fromResponse(ResponseInterface $response, ?\Throwable $pr } if ($errors === []) { - $errors[] = new BringApiError('', sprintf('HTTP %d (no parsable error body)', $status), null); + // No recognised error envelope. Surface a snippet of the raw body + // so the actual Bring rejection reason isn't lost — without this, + // unfamiliar 4xx shapes (plain text, HTML, JSON envelopes other + // than errors/messages) collapse to "no parsable error body" and + // the operator has nothing to act on. The body is redacted and + // truncated; the parseable-envelope path is unchanged so the + // "Bring sometimes echoes credentials" guarantee on getMessage() + // still holds for the case it was written for. + $snippet = self::redactBodySnippet($body); + $errors[] = new BringApiError( + '', + $snippet === '' + ? sprintf('HTTP %d (empty error body)', $status) + : sprintf('HTTP %d: %s', $status, $snippet), + null, + ); } return new self($status, $errors, $response, $previous); } + + /** + * Trim, single-line, truncate and redact obvious credential-shaped values + * from a response body so it's safe to embed in an exception message. + */ + private static function redactBodySnippet(string $body): string + { + $body = trim($body); + if ($body === '') { + return ''; + } + + // Collapse whitespace so multi-line HTML/JSON doesn't blow up logs. + $body = (string) preg_replace('/\s+/u', ' ', $body); + + // Redact common credential shapes: JSON "key":"value", form key=value, + // and Authorization-style header values. Match key names case-insensitively. + $sensitive = '(?:api[_-]?key|apikey|x-mybring-api-key|authorization|password|secret|token)'; + $body = (string) preg_replace( + '/"(' . $sensitive . ')"\s*:\s*"[^"]*"/i', + '"$1":"[REDACTED]"', + $body, + ); + $body = (string) preg_replace( + '/\b(' . $sensitive . ')\s*=\s*[^&\s"\']+/i', + '$1=[REDACTED]', + $body, + ); + + $limit = 500; + if (mb_strlen($body) > $limit) { + $body = mb_substr($body, 0, $limit) . '…'; + } + + return $body; + } } diff --git a/tests/v4/Http/TransportTest.php b/tests/v4/Http/TransportTest.php index 4173b5d..e52af47 100644 --- a/tests/v4/Http/TransportTest.php +++ b/tests/v4/Http/TransportTest.php @@ -101,6 +101,45 @@ public function testApiExceptionMessageEmbedsFirstErrorButNotRawBody(): void } } + public function testUnparsableErrorBodyIsSurfacedInMessage(): void + { + // Bring occasionally returns a 4xx with a body that doesn't fit the + // errors/messages envelope (plain text, HTML, a different JSON shape). + // We must still surface enough of it that the operator can act on it + // instead of collapsing to "no parsable error body". + $body = 'Recipient postal code 1642 is not serviced by BUSINESS_PARCEL'; + $client = new RecordingClient([new Response(400, ['Content-Type' => 'text/plain'], $body)]); + $transport = new Transport($client, $this->factory, $this->factory, $this->factory, new NullAuthorization()); + + try { + $transport->send(new PostalCodeEndpoint('0150')); + self::fail('expected BringApiException'); + } catch (BringApiException $e) { + self::assertStringContainsString('HTTP 400', $e->getMessage()); + self::assertStringContainsString('postal code 1642', $e->getMessage()); + self::assertStringNotContainsString('no parsable error body', $e->getMessage()); + } + } + + public function testUnparsableErrorBodyRedactsCredentialShapedValues(): void + { + // If the unparsable body happens to echo credential-shaped values, + // they must be redacted before being embedded in getMessage(). + $body = '{"detail":"unauthorized","api_key":"secretKey","Authorization":"Basic abc"}'; + $client = new RecordingClient([new Response(401, ['Content-Type' => 'application/json'], $body)]); + $transport = new Transport($client, $this->factory, $this->factory, $this->factory, new NullAuthorization()); + + try { + $transport->send(new PostalCodeEndpoint('0150')); + self::fail('expected BringApiException'); + } catch (BringApiException $e) { + self::assertStringContainsString('HTTP 401', $e->getMessage()); + self::assertStringContainsString('unauthorized', $e->getMessage()); + self::assertStringNotContainsString('secretKey', $e->getMessage()); + self::assertStringNotContainsString('Basic abc', $e->getMessage()); + } + } + public function testTransportExceptionWrapsPsr18Failure(): void { $networkFail = new class ('boom') extends \RuntimeException implements ClientExceptionInterface {