Closed stephenfrank closed 5 months ago
Are you sure you're running the latest version as we resolved a very similar issue a few weeks back: https://github.com/laravel/reverb/pull/87/files
Hi Joe, I'm using v1.0.0-beta9 which includes these updates.
I've sent a pull request where I believe there is an error: https://github.com/laravel/reverb/blob/main/src/Servers/Reverb/Http/Request.php#L36
if ($connection->bufferLength() < $contentLength[0] ?? 0)
should be
if ($request->getBody()->getSize() < $contentLength[0] ?? 0)
Content-Length should be compared to the length of the body only since the length of the buffer also includes the headers.
After this, the incomplete requests are rejected as null
. The behaviour seems to work for me because the connection is held open and the missing data is appended to the buffer (at least, that's how I think it works) after which the request gets through and is broadcast
15:07:41.6876 Request Content-Length> 362
15:07:41.6876 Request Body> 0
-
15:07:41.6878 Request Content-Length> 362
15:07:41.6878 Request Body> 362
@stephenfrank Your assumption is correct about how the buffer will continue to be filled until the end of body symbol is received.
Your logic makes sense to me, but I'm not sure how this is the first time it has been raised.
Assuming the values in your last messages are from log statements, could you also include the value of $connection->bufferLength()
?
I agree, seems like something that would have come up.
Here is the same logging with the buffers length:
1715094296.4642 | Connection Buffer> 948
1715094296.4642 | Request Content-Length> 362
1715094296.4642 | Request Body> 362
1715094297.982 | ->
1715094297.982 | Connection Buffer> 586
1715094297.982 | Request Content-Length> 362
1715094297.982 | Request Body> 0
1715094297.9822 | ->
1715094297.9822 | Connection Buffer> 948
1715094297.9822 | Request Content-Length> 362
1715094297.9822 | Request Body> 362
Running in Request.php
public static function from(string $message, Connection $connection, int $maxRequestSize): ?RequestInterface
{
$connection->appendToBuffer($message);
if ($connection->bufferLength() > $maxRequestSize) {
throw new OverflowException('Maximum HTTP buffer size of '.$maxRequestSize.'exceeded.');
}
if (static::isEndOfMessage($buffer = $connection->buffer())) {
$request = Message::parseRequest($buffer);
$t = microtime(true);
echo "$t | ->\n";
echo "$t | Connection Buffer> {$connection->bufferLength()}\n";
echo "$t | Request Content-Length> {$request->getHeader('Content-Length')[0]}\n";
echo "$t | Request Body> {$request->getBody()->getSize()}\n";
if (! $contentLength = $request->getHeader('Content-Length')) {
return $request;
}
if ($request->getBody()->getSize() < $contentLength[0] ?? 0) {
return null;
}
$connection->clearBuffer();
return $request;
}
return null;
}
Running on Docker / Macbook / ARM64:
php -v
PHP 8.2.9 (cli) (built: Aug 29 2023 10:48:25) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.9, Copyright (c) Zend Technologies
with Zend OPcache v8.2.9, Copyright (c), by Zend Technologies
with Xdebug v3.2.1, Copyright (c) 2002-2023, by Derick Rethans
Do the second and third logs comprise a single request?
Yes, sorry maybe I could explain better...
Request 1) $connection->on('data'
receives header and body in a single chunk
1715094296.4642 | Connection Buffer> 948
1715094296.4642 | Request Content-Length> 362
1715094296.4642 | Request Body> 362
Request 2) $connection->on('data'
is fired twice and receives header in first chunk and body in the second chunk
1715094297.982 | ->
1715094297.982 | Connection Buffer> 586
1715094297.982 | Request Content-Length> 362
1715094297.982 | Request Body> 0
1715094297.9822 | ->
1715094297.9822 | Connection Buffer> 948
1715094297.9822 | Request Content-Length> 362
1715094297.9822 | Request Body> 362
I'm still intrigued to know why your requests are getting split up like that, but I've tested your PR and all looks good so going to recommend this gets merged. Thanks for digging into this one 👍
Reverb Version
1.0.0-beta9
Laravel Version
11.6.0
PHP Version
8.2.9
Description
I have an issue where requests are intermittently dispatched without the request body.
I've tracked it down to the React PHP connection
$connection->on('data'
receiving the data in two chunks. eg. the headers in the first chunk and the body in the second chunk.Gives this output (edited for brevity)
I've hacked together a solution that solves this issue for me... essentially parsing the Content-Length header and waiting for the complete body before proceeding (it's just a proof of concept)
The ReactPHP HTTP server implementation has similar buffering to handle the full request body: https://github.com/reactphp/http/blob/1.x/src/Middleware/RequestBodyBufferMiddleware.php
Steps To Reproduce
I'm only running this on my local development environment so can't say for sure how to reproduce.