From 33572beb164138d7e3696ce0f51f69c807076ce8 Mon Sep 17 00:00:00 2001 From: ernolf Date: Sat, 18 Jul 2026 21:55:00 +0200 Subject: [PATCH] fix: make middleware injection idempotent and header lookups case-insensitive - remove a possibly existing admin_audit_http_client stack entry before unshifting, so double decoration can no longer produce duplicate log entries; the integration test now asserts exactly one entry - rename the shadowed $handler variable in newClient() to $stack - find the Content-Length of the immediate-path check and the Host fallback in LogPathHelper case-insensitively Signed-off-by: ernolf --- lib/Http/Client/LoggingClientService.php | 10 ++++--- .../Middleware/HttpClientLoggerMiddleware.php | 9 +++++-- lib/Http/Client/Middleware/LogPathHelper.php | 21 +++++++++------ tests/integration/MiddlewareInjectionTest.php | 4 ++- .../HttpClientLoggerMiddlewareTest.php | 26 +++++++++++++++++++ .../Client/Middleware/LogPathHelperTest.php | 8 ++++++ 6 files changed, 64 insertions(+), 14 deletions(-) diff --git a/lib/Http/Client/LoggingClientService.php b/lib/Http/Client/LoggingClientService.php index 90b3bd0..723d128 100644 --- a/lib/Http/Client/LoggingClientService.php +++ b/lib/Http/Client/LoggingClientService.php @@ -40,9 +40,13 @@ public function newClient(?callable $handler = null): IClient { /** @var \GuzzleHttp\Client $guzzle */ $guzzle = $ref->getValue($client); - $handler = $guzzle->getConfig('handler'); - if ($handler instanceof \GuzzleHttp\HandlerStack) { - $handler->unshift( + $stack = $guzzle->getConfig('handler'); + if ($stack instanceof \GuzzleHttp\HandlerStack) { + // Idempotent: replace an already injected entry instead of + // stacking a second one (double decoration would silently + // duplicate every log entry). + $stack->remove('admin_audit_http_client'); + $stack->unshift( new HttpClientLoggerMiddleware( $this->logger, $this->resolveLogDir(), diff --git a/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php b/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php index 059a37b..f25a029 100644 --- a/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php +++ b/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php @@ -129,8 +129,13 @@ function (ResponseInterface $response) use ($request, $reqHeaders, $reqId) { // Content-Length: 0 — no body if (!$immediate) { - $compact = $this->compactHeaders($respHeaders); - $cl = $compact['content-length'] ?? $compact['Content-Length'] ?? null; + $cl = null; + foreach ($meta['responseHeaders'] as $k => $v) { + if (strtolower((string)$k) === 'content-length') { + $cl = is_array($v) ? ($v[0] ?? null) : $v; + break; + } + } if ($cl !== null && is_numeric($cl) && (int)$cl === 0) { $immediate = true; } diff --git a/lib/Http/Client/Middleware/LogPathHelper.php b/lib/Http/Client/Middleware/LogPathHelper.php index c04b6b1..543a11f 100644 --- a/lib/Http/Client/Middleware/LogPathHelper.php +++ b/lib/Http/Client/Middleware/LogPathHelper.php @@ -34,15 +34,20 @@ public static function getPathsFromMeta(array $meta, string $baseDir, ?string $r } } - if ($host === null && !empty($meta['requestHeaders']['Host'])) { - $hostHeader = $meta['requestHeaders']['Host']; - $hostHeader = is_array($hostHeader) ? ($hostHeader[0] ?? '') : (string)$hostHeader; - if ($hostHeader !== '') { - if (strpos($hostHeader, ':') !== false) { - [$host, $port] = explode(':', $hostHeader, 2) + [1 => null]; - } else { - $host = $hostHeader; + if ($host === null && !empty($meta['requestHeaders']) && is_array($meta['requestHeaders'])) { + foreach ($meta['requestHeaders'] as $name => $value) { + if (strtolower((string)$name) !== 'host') { + continue; } + $hostHeader = is_array($value) ? ($value[0] ?? '') : (string)$value; + if ($hostHeader !== '') { + if (strpos($hostHeader, ':') !== false) { + [$host, $port] = explode(':', $hostHeader, 2) + [1 => null]; + } else { + $host = $hostHeader; + } + } + break; } } diff --git a/tests/integration/MiddlewareInjectionTest.php b/tests/integration/MiddlewareInjectionTest.php index ffd58ff..decfb9d 100644 --- a/tests/integration/MiddlewareInjectionTest.php +++ b/tests/integration/MiddlewareInjectionTest.php @@ -44,7 +44,9 @@ public function testMiddlewareIsInjectedIntoHandlerStack(): void { $stackRef = new \ReflectionProperty(HandlerStack::class, 'stack'); $names = array_column($stackRef->getValue($handler), 1); - $this->assertContains('admin_audit_http_client', $names); + // This service decorates the already decorated container service, so + // without the idempotency guard the entry would appear twice here. + $this->assertCount(1, array_keys($names, 'admin_audit_http_client', true)); } public function testEnabledAppDecoratesClientService(): void { diff --git a/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php b/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php index 4adfc2d..d659839 100644 --- a/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php +++ b/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php @@ -125,6 +125,32 @@ public function testGeneratedRequestIdCarriesServerRequestIdPrefix(): void { $this->assertStringStartsWith('server-req-', (string)$seen); } + public function testMixedCaseContentLengthZeroLogsImmediately(): void { + $dir = sys_get_temp_dir() . '/aahc-cl-' . bin2hex(random_bytes(4)); + $mw = new HttpClientLoggerMiddleware(new NullLogger(), $dir); + + $handler = $mw(function (Request $request, array $options) { + return new FulfilledPromise(new Response(200, ['Content-length' => '0'], '')); + }); + + try { + $response = $handler(new Request('GET', 'https://example.com/ping'), [])->wait(); + + $this->assertNotInstanceOf(CountingStream::class, $response->getBody()); + + $lines = file($dir . '/example.com.json', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); + $this->assertNotFalse($lines); + $entry = json_decode($lines[0], true, 512, JSON_THROW_ON_ERROR); + $this->assertSame(200, $entry['status']); + $this->assertNull($entry['compressionStats']['decompressed_bytes']); + } finally { + foreach (glob($dir . '/*') ?: [] as $file) { + @unlink($file); + } + @rmdir($dir); + } + } + public function testWriteImmediateOmitsDecompressedBytesAndRatio(): void { $dir = sys_get_temp_dir() . '/aahc-wi-' . bin2hex(random_bytes(4)); $mw = new HttpClientLoggerMiddleware(new NullLogger(), $dir); diff --git a/tests/unit/Http/Client/Middleware/LogPathHelperTest.php b/tests/unit/Http/Client/Middleware/LogPathHelperTest.php index def6efc..3448bc2 100644 --- a/tests/unit/Http/Client/Middleware/LogPathHelperTest.php +++ b/tests/unit/Http/Client/Middleware/LogPathHelperTest.php @@ -55,6 +55,14 @@ public function testHostHeaderFallbackWithPort(): void { $this->assertSame($this->baseDir . '/foo.bar_8080.json', $json); } + public function testHostHeaderFallbackIsCaseInsensitive(): void { + [$json] = LogPathHelper::getPathsFromMeta( + ['requestHeaders' => ['host' => 'foo.bar:8080']], + $this->baseDir, + ); + $this->assertSame($this->baseDir . '/foo.bar_8080.json', $json); + } + public function testHostHeaderFallbackAcceptsArrayValues(): void { [$json] = LogPathHelper::getPathsFromMeta( ['requestHeaders' => ['Host' => ['foo.bar']]],