diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cceee5fc..d6656fc3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Fix regression introduced in #1129 (#1143) - Fix capturing of the request body in the `RequestIntegration` integration when the stream is empty (#1129) ### 2.5.0 (2020-09-14) diff --git a/composer.json b/composer.json index 3deb9567d..28ed5d1bf 100644 --- a/composer.json +++ b/composer.json @@ -24,7 +24,7 @@ "ext-json": "*", "ext-mbstring": "*", "guzzlehttp/promises": "^1.3", - "guzzlehttp/psr7": "^1.6", + "guzzlehttp/psr7": "^1.7", "jean85/pretty-package-versions": "^1.2", "php-http/async-client-implementation": "^1.0", "php-http/client-common": "^1.5|^2.0", diff --git a/src/Integration/RequestIntegration.php b/src/Integration/RequestIntegration.php index 6544be0f8..ab011a0ce 100644 --- a/src/Integration/RequestIntegration.php +++ b/src/Integration/RequestIntegration.php @@ -4,6 +4,7 @@ namespace Sentry\Integration; +use GuzzleHttp\Psr7\Utils; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UploadedFileInterface; use Sentry\Event; @@ -35,6 +36,17 @@ final class RequestIntegration implements IntegrationInterface */ private const REQUEST_BODY_MEDIUM_MAX_CONTENT_LENGTH = 10 ** 4; + /** + * This constant is a map of maximum allowed sizes for each value of the + * `max_request_body_size` option. + */ + private const MAX_REQUEST_BODY_SIZE_OPTION_TO_MAX_LENGTH_MAP = [ + 'none' => 0, + 'small' => self::REQUEST_BODY_SMALL_MAX_CONTENT_LENGTH, + 'medium' => self::REQUEST_BODY_MEDIUM_MAX_CONTENT_LENGTH, + 'always' => -1, + ]; + /** * @var Options|null The client options */ @@ -176,29 +188,23 @@ static function (string $key) use ($keysToRemove): bool { * the parsing fails then the raw data is returned. If there are submitted * fields or files, all of their information are parsed and returned. * - * @param Options $options The options of the client - * @param ServerRequestInterface $serverRequest The server request + * @param Options $options The options of the client + * @param ServerRequestInterface $request The server request * * @return mixed */ - private function captureRequestBody(Options $options, ServerRequestInterface $serverRequest) + private function captureRequestBody(Options $options, ServerRequestInterface $request) { $maxRequestBodySize = $options->getMaxRequestBodySize(); - $requestBody = $serverRequest->getBody(); - $requestBodySize = $requestBody->getSize(); - - if ( - !$requestBodySize || - 'none' === $maxRequestBodySize || - ('small' === $maxRequestBodySize && $requestBodySize > self::REQUEST_BODY_SMALL_MAX_CONTENT_LENGTH) || - ('medium' === $maxRequestBodySize && $requestBodySize > self::REQUEST_BODY_MEDIUM_MAX_CONTENT_LENGTH) - ) { + $requestBodySize = (int) $request->getHeaderLine('Content-Length'); + + if (!$this->isRequestBodySizeWithinReadBounds($requestBodySize, $maxRequestBodySize)) { return null; } - $requestData = $serverRequest->getParsedBody(); + $requestData = $request->getParsedBody(); $requestData = array_merge( - $this->parseUploadedFiles($serverRequest->getUploadedFiles()), + $this->parseUploadedFiles($request->getUploadedFiles()), \is_array($requestData) ? $requestData : [] ); @@ -206,15 +212,17 @@ private function captureRequestBody(Options $options, ServerRequestInterface $se return $requestData; } - if ('application/json' === $serverRequest->getHeaderLine('Content-Type')) { + $requestBody = Utils::copyToString($request->getBody(), self::MAX_REQUEST_BODY_SIZE_OPTION_TO_MAX_LENGTH_MAP[$maxRequestBodySize]); + + if ('application/json' === $request->getHeaderLine('Content-Type')) { try { - return JSON::decode($requestBody->getContents()); + return JSON::decode($requestBody); } catch (JsonException $exception) { // Fallback to returning the raw data from the request body } } - return $requestBody->getContents(); + return $requestBody; } /** @@ -245,4 +253,25 @@ private function parseUploadedFiles(array $uploadedFiles): array return $result; } + + private function isRequestBodySizeWithinReadBounds(int $requestBodySize, string $maxRequestBodySize): bool + { + if ($requestBodySize <= 0) { + return false; + } + + if ('none' === $maxRequestBodySize) { + return false; + } + + if ('small' === $maxRequestBodySize && $requestBodySize > self::REQUEST_BODY_SMALL_MAX_CONTENT_LENGTH) { + return false; + } + + if ('medium' === $maxRequestBodySize && $requestBodySize > self::REQUEST_BODY_MEDIUM_MAX_CONTENT_LENGTH) { + return false; + } + + return true; + } } diff --git a/tests/Integration/RequestIntegrationTest.php b/tests/Integration/RequestIntegrationTest.php index 6556fc3b1..e3eacc78f 100644 --- a/tests/Integration/RequestIntegrationTest.php +++ b/tests/Integration/RequestIntegrationTest.php @@ -7,10 +7,10 @@ use GuzzleHttp\Psr7\ServerRequest; use GuzzleHttp\Psr7\UploadedFile; use GuzzleHttp\Psr7\Uri; +use GuzzleHttp\Psr7\Utils; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Message\StreamInterface; use Sentry\ClientInterface; use Sentry\Event; use Sentry\Integration\RequestFetcherInterface; @@ -213,18 +213,18 @@ public function applyToEventDataProvider(): \Generator 'max_request_body_size' => 'none', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) - ->withParsedBody([ - 'foo' => 'foo value', - 'bar' => 'bar value', - ]) - ->withBody($this->getStreamMock(1)), + ->withHeader('Content-Length', '3') + ->withBody(Utils::streamFor('foo')), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], + 'Content-Length' => ['3'], ], ], + null, + null, ]; yield [ @@ -232,21 +232,16 @@ public function applyToEventDataProvider(): \Generator 'max_request_body_size' => 'small', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) - ->withParsedBody([ - 'foo' => 'foo value', - 'bar' => 'bar value', - ]) - ->withBody($this->getStreamMock(10 ** 3)), + ->withHeader('Content-Length', 10 ** 3) + ->withBody(Utils::streamFor('Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus at placerat est. Donec maximus odio augue, vitae bibendum nisi euismod nec. Nunc vel velit ligula. Ut non ultricies magna, non condimentum turpis. Donec pellentesque id nunc at facilisis. Sed fermentum ultricies nunc, id posuere ex ullamcorper quis. Sed varius tincidunt nulla, id varius nulla interdum sit amet. Pellentesque molestie sapien at mi tristique consequat. Nullam id eleifend arcu. Vivamus sed placerat neque. Ut sapien magna, elementum in euismod pretium, rhoncus vitae augue. Nam ullamcorper dui et tortor semper, eu feugiat elit faucibus. Curabitur vel auctor odio. Phasellus vestibulum ullamcorper dictum. Suspendisse fringilla, ipsum bibendum venenatis vulputate, nunc orci facilisis leo, commodo finibus mi arcu in turpis. Mauris ut ultrices est. Nam quis purus ut nulla interdum ornare. Proin in tellus egestas, commodo magna porta, consequat justo. Vivamus in convallis odio. Pellentesque porttitor, urna non gravida.')), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], + 'Content-Length' => ['1000'], ], - 'data' => [ - 'foo' => 'foo value', - 'bar' => 'bar value', - ], + 'data' => 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus at placerat est. Donec maximus odio augue, vitae bibendum nisi euismod nec. Nunc vel velit ligula. Ut non ultricies magna, non condimentum turpis. Donec pellentesque id nunc at facilisis. Sed fermentum ultricies nunc, id posuere ex ullamcorper quis. Sed varius tincidunt nulla, id varius nulla interdum sit amet. Pellentesque molestie sapien at mi tristique consequat. Nullam id eleifend arcu. Vivamus sed placerat neque. Ut sapien magna, elementum in euismod pretium, rhoncus vitae augue. Nam ullamcorper dui et tortor semper, eu feugiat elit faucibus. Curabitur vel auctor odio. Phasellus vestibulum ullamcorper dictum. Suspendisse fringilla, ipsum bibendum venenatis vulputate, nunc orci facilisis leo, commodo finibus mi arcu in turpis. Mauris ut ultrices est. Nam quis purus ut nulla interdum ornare. Proin in tellus egestas, commodo magna porta, consequat justo. Vivamus in convallis odio. Pellentesque porttitor, urna non gravid', ], ]; @@ -255,39 +250,41 @@ public function applyToEventDataProvider(): \Generator 'max_request_body_size' => 'small', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) + ->withHeader('Content-Length', (string) (10 ** 3)) ->withParsedBody([ 'foo' => 'foo value', 'bar' => 'bar value', - ]) - ->withBody($this->getStreamMock(10 ** 3 + 1)), + ]), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], + 'Content-Length' => ['1000'], + ], + 'data' => [ + 'foo' => 'foo value', + 'bar' => 'bar value', ], ], ]; yield [ [ - 'max_request_body_size' => 'medium', + 'max_request_body_size' => 'small', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) + ->withHeader('Content-Length', (string) (10 ** 3 + 1)) ->withParsedBody([ 'foo' => 'foo value', 'bar' => 'bar value', - ]) - ->withBody($this->getStreamMock(10 ** 4)), + ]), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], - ], - 'data' => [ - 'foo' => 'foo value', - 'bar' => 'bar value', + 'Content-Length' => ['1001'], ], ], ]; @@ -297,41 +294,41 @@ public function applyToEventDataProvider(): \Generator 'max_request_body_size' => 'medium', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) + ->withHeader('Content-Length', (string) (10 ** 4)) ->withParsedBody([ 'foo' => 'foo value', 'bar' => 'bar value', - ]) - ->withBody($this->getStreamMock(10 ** 4 + 1)), + ]), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], + 'Content-Length' => ['10000'], + ], + 'data' => [ + 'foo' => 'foo value', + 'bar' => 'bar value', ], ], ]; yield [ [ - 'max_request_body_size' => 'always', + 'max_request_body_size' => 'medium', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) - ->withUploadedFiles([ - 'foo' => new UploadedFile('foo content', 123, UPLOAD_ERR_OK, 'foo.ext', 'application/text'), - ]) - ->withBody($this->getStreamMock(123 + 321)), + ->withHeader('Content-Length', (string) (10 ** 4 + 1)) + ->withParsedBody([ + 'foo' => 'foo value', + 'bar' => 'bar value', + ]), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], - ], - 'data' => [ - 'foo' => [ - 'client_filename' => 'foo.ext', - 'client_media_type' => 'application/text', - 'size' => 123, - ], + 'Content-Length' => ['10001'], ], ], ]; @@ -341,18 +338,19 @@ public function applyToEventDataProvider(): \Generator 'max_request_body_size' => 'always', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) + ->withHeader('Content-Length', '444') ->withUploadedFiles([ 'foo' => [ new UploadedFile('foo content', 123, UPLOAD_ERR_OK, 'foo.ext', 'application/text'), new UploadedFile('bar content', 321, UPLOAD_ERR_OK, 'bar.ext', 'application/octet-stream'), ], - ]) - ->withBody($this->getStreamMock(123 + 321)), + ]), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], + 'Content-Length' => ['444'], ], 'data' => [ 'foo' => [ @@ -371,58 +369,21 @@ public function applyToEventDataProvider(): \Generator ], ]; - yield [ - [ - 'max_request_body_size' => 'always', - ], - (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) - ->withUploadedFiles([ - 'foo' => [ - 'bar' => [ - new UploadedFile('foo content', 123, UPLOAD_ERR_OK, 'foo.ext', 'application/text'), - new UploadedFile('bar content', 321, UPLOAD_ERR_OK, 'bar.ext', 'application/octet-stream'), - ], - ], - ]) - ->withBody($this->getStreamMock(123 + 321)), - [ - 'url' => 'http://www.example.com/foo', - 'method' => 'POST', - 'headers' => [ - 'Host' => ['www.example.com'], - ], - 'data' => [ - 'foo' => [ - 'bar' => [ - [ - 'client_filename' => 'foo.ext', - 'client_media_type' => 'application/text', - 'size' => 123, - ], - [ - 'client_filename' => 'bar.ext', - 'client_media_type' => 'application/octet-stream', - 'size' => 321, - ], - ], - ], - ], - ], - ]; - yield [ [ 'max_request_body_size' => 'always', ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) ->withHeader('Content-Type', 'application/json') - ->withBody($this->getStreamMock(13, '{"foo":"bar"}')), + ->withHeader('Content-Length', '13') + ->withBody(Utils::streamFor('{"foo":"bar"}')), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', 'headers' => [ 'Host' => ['www.example.com'], 'Content-Type' => ['application/json'], + 'Content-Length' => ['13'], ], 'data' => [ 'foo' => 'bar', @@ -436,7 +397,7 @@ public function applyToEventDataProvider(): \Generator ], (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) ->withHeader('Content-Type', 'application/json') - ->withBody($this->getStreamMock(1, '{')), + ->withBody(Utils::streamFor('{"foo":"bar"}')), [ 'url' => 'http://www.example.com/foo', 'method' => 'POST', @@ -444,60 +405,8 @@ public function applyToEventDataProvider(): \Generator 'Host' => ['www.example.com'], 'Content-Type' => ['application/json'], ], - 'data' => '{', ], ]; - - yield [ - [ - 'max_request_body_size' => 'always', - ], - (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) - ->withHeader('Content-Type', 'application/json') - ->withBody($this->getStreamMock(null)), - [ - 'url' => 'http://www.example.com/foo', - 'method' => 'POST', - 'headers' => [ - 'Host' => ['www.example.com'], - 'Content-Type' => ['application/json'], - ], - ], - ]; - - yield [ - [ - 'max_request_body_size' => 'always', - ], - (new ServerRequest('POST', new Uri('http://www.example.com/foo'))) - ->withHeader('Content-Type', 'application/json') - ->withBody($this->getStreamMock(0)), - [ - 'url' => 'http://www.example.com/foo', - 'method' => 'POST', - 'headers' => [ - 'Host' => ['www.example.com'], - 'Content-Type' => ['application/json'], - ], - ], - null, - null, - ]; - } - - private function getStreamMock(?int $size, string $content = ''): StreamInterface - { - /** @var MockObject|StreamInterface $stream */ - $stream = $this->createMock(StreamInterface::class); - $stream->expects($this->any()) - ->method('getSize') - ->willReturn($size); - - $stream->expects(null === $size ? $this->never() : $this->any()) - ->method('getContents') - ->willReturn($content); - - return $stream; } private function createRequestFetcher(ServerRequestInterface $request): RequestFetcherInterface