feat: limit OTLP HTTP response body to 4 MiB#1935
Conversation
Prevents unbounded memory usage when a misconfigured or malicious collector sends an oversized response body. - Add ResponseBodySizeLimit::MAX_BYTES constant (4 MiB) - Add PsrUtils::readBodyWithSizeLimit() and decode() - Update PsrTransport::handleResponse() to use the size-capped reader - Add TransportResponseException for typed HTTP error handling - Add ResponseBodySizeLimitTest (10 test cases) Fixes #1932
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1935 +/- ##
============================================
- Coverage 68.17% 66.10% -2.08%
+ Complexity 3021 2999 -22
============================================
Files 450 451 +1
Lines 8820 8771 -49
============================================
- Hits 6013 5798 -215
- Misses 2807 2973 +166
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| { | ||
| $body = $payload; | ||
|
|
||
| if ($this->compression === 'gzip') { |
There was a problem hiding this comment.
| if ($this->compression === 'gzip') { | |
| if ($this->compression === 'gzip' && extension_loaded('zlib')) { |
Same for the other if ($this->compression === 'gzip') below; can we merge these two ifs?
Alternatively check in constructor.
| string $endpoint, | ||
| string $contentType, | ||
| array $headers = [], | ||
| string $compression = 'none' |
There was a problem hiding this comment.
| string $compression = 'none' | |
| ?string $compression = null, |
| ->withBody($stream) | ||
| ->withHeader('Content-Type', $this->contentType); | ||
|
|
||
| for ($retries = 0;; $retries++) { |
There was a problem hiding this comment.
Is the removal of retries intended?
| continue; | ||
| } | ||
| // (3) Read at most $maxRead bytes. | ||
| $body = $response->getBody()->read($maxRead); |
There was a problem hiding this comment.
Has to call read in a loop.
Fewer than $length bytes may be returned if underlying stream call returns fewer bytes.
| if ($value === '') { | ||
| return $value; | ||
| } | ||
| $body = self::readBodyWithSizeLimit($response); |
There was a problem hiding this comment.
Should decompress the response body stream while reading it instead of buffering its content and then decompressing the entire body at once.
| if ($result === false) { | ||
| throw new LogicException(); | ||
| } | ||
| return $decoded; |
There was a problem hiding this comment.
The limit must apply to the decompressed body, see OTLP/HTTP Response:
The client MUST limit the size of the response body when parsing it, including after decompression, to mitigate possible excessive memory usage caused by a misconfigured or malicious server.
| final class PsrTransport implements TransportInterface | ||
| final class PsrTransport |
There was a problem hiding this comment.
This change seems unintended?
| private readonly array $compression, | ||
| private readonly int $retryDelay, | ||
| private readonly int $maxRetries, | ||
| ClientInterface $client, |
There was a problem hiding this comment.
Why has the constructor property promotion been removed here? 🤔
| { | ||
| private int $statusCode; | ||
| private string $responseBody; | ||
|
|
||
| public function __construct(int $statusCode, string $responseBody, string $message = '') | ||
| { | ||
| parent::__construct($message !== '' ? $message : sprintf('HTTP %d', $statusCode)); | ||
| $this->statusCode = $statusCode; | ||
| $this->responseBody = $responseBody; | ||
| } |
There was a problem hiding this comment.
| { | |
| private int $statusCode; | |
| private string $responseBody; | |
| public function __construct(int $statusCode, string $responseBody, string $message = '') | |
| { | |
| parent::__construct($message !== '' ? $message : sprintf('HTTP %d', $statusCode)); | |
| $this->statusCode = $statusCode; | |
| $this->responseBody = $responseBody; | |
| } | |
| { | |
| public function __construct( | |
| private int $statusCode, | |
| private string $responseBody, | |
| string $message = '', | |
| ) { | |
| parent::__construct($message ?: sprintf('HTTP %d', $statusCode)); | |
| } |
- Restore PsrTransport to original structure (implements TransportInterface, constructor property promotion, retry logic, closed state, shutdown/forceFlush) - Change PsrUtils::decode() to accept ResponseInterface instead of string, applying the 4 MiB size limit to the **decompressed** body per OTLP spec - Add readWithLimit() that loops on stream read() for partial returns - Remove unused TransportResponseException (no longer referenced) - Update PsrUtilsTest to match new decode() signature - Update ResponseBodySizeLimitTest for new behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 participants
Prevents unbounded memory usage when a misconfigured or malicious collector sends an oversized response body.
Fixes #1932