RST_STREAM if request body isn't fully awaited before sending response
kelunik opened this issue · 5 comments
kelunik commented
If a large request body isn't fully awaited before sending a response, amphp/http-server sends RST_STREAM frames for each request data frame.
kelunik commented
Uploading hangs now, I guess because the stream window isn't increased. The client connection is eventually closed, probably because no data from the client for a period of time. But I'm not sure which behavior we should really have here, because sending a large request body if the server doesn't expect one seems weird. Also the server sending a response while the upload is still in progress is questionable.
trowski commented
If the stream window isn't increased to at least the maximum body size, then that's definitely a bug.
Sending the response before the request is received certainly seems dubious, but I guess it should be supported. Apparently browsers tolerate it on XHR file uploads.
trowski commented
I believe this is now fixed with ec5438a. The server below worked regardless of file size uploaded. @kelunik Could you please verify?
<?php
require_once __DIR__.'/../vendor/autoload.php';
use Amp\Log\ConsoleFormatter;
use Amp\ByteStream\ResourceOutputStream;
use Amp\Http\Server\FormParser;
use Amp\Http\Server\Request;
use Amp\Http\Server\Response;
use Amp\Success;
use Amp\Socket\Server;
use Amp\Http\Server\HttpServer;
use Amp\Http\Server\RequestHandler;
use Amp\Http\Status;
use Monolog\Logger;
use Amp\Log\StreamHandler;
const HTML = <<<HTML
<html>
<body>
<form name="a" action="/" enctype="multipart/form-data" method="POST">
<input type="hidden" name="file_size" id="file_size" value=0>
<input type="file" name="file_upload" id="file_upload">
<input type=submit>
</form>
<script>
document.forms.a.addEventListener('submit', _ => {
document.forms.a.file_size.value = document.forms.a.file_upload.files[0].size;
})
</script>
</body>
</html>
HTML;
class UploadFile implements RequestHandler
{
public function handleRequest(Request $request): \Amp\Promise
{
if ($request->getMethod() === 'POST') {
return \Amp\call(function () use ($request) {
$request->getBody()->increaseSizeLimit(1024 * 1024 * 1024 * 1.5);
$parser = new FormParser\BufferingParser;
$form = yield $parser->parseForm($request);
\assert($form instanceof FormParser\Form);
$files = $form->getFiles();
$file = $form->getFile('file_upload');
return new Response(Status::BAD_REQUEST, ['content-type' => 'application/json'], \json_encode(['ok' => 'true', 'length' => \strlen($file->getContents())])); // for example
});
}
return new Success(new Response(Status::OK, ['content-type' => 'text/html; charset=utf-8'], HTML));
}
}
\Amp\Loop::run(function() {
$tlsContext = (new \Amp\Socket\ServerTlsContext())
->withDefaultCertificate(new \Amp\Socket\Certificate(__DIR__ . '/../test/server.pem'));
$bindContext = (new \Amp\Socket\BindContext())
->withTlsContext($tlsContext);
$sockets = [
Server::listen('0.0.0.0:1337', $bindContext),
Server::listen('[::]:1337', $bindContext),
];
$logHandler = new StreamHandler(new ResourceOutputStream(\STDOUT));
$logHandler->setFormatter(new ConsoleFormatter);
$logger = new Logger('server');
$logger->pushHandler($logHandler);
$server = new HttpServer($sockets, new UploadFile, $logger);
yield $server->start();
});kelunik commented
Currently it logs:
[2020-06-17T18:32:56.453129+00:00] server.critical: AssertionError: Tried to release a non-existent stream in /home/kelunik/GitHub/amphp/http-server/src/Driver/Http2Driver.php:567 Stack trace: #0 /home/kelunik/GitHub/amphp/http-server/src/Driver/Http2Driver.php(567): assert(false, 'Tried to releas...') #1 /home/kelunik/GitHub/amphp/http-server/src/Driver/Http2Driver.php(1142): Amp\Http\Server\Driver\Http2Driver->releaseStream(5) #2 /home/kelunik/GitHub/amphp/http-server/vendor/amphp/http/src/Http2/Http2Parser.php(306): Amp\Http\Server\Driver\Http2Driver->handleStreamEnd(5) #3 /home/kelunik/GitHub/amphp/http-server/vendor/amphp/http/src/Http2/Http2Parser.php(204): Amp\Http\Http2\Http2Parser->parseDataFrame('------WebKitFor...', 1025, 1, 5) #4 /home/kelunik/GitHub/amphp/http-server/src/Driver/Http2Driver.php(606): Amp\Http\Http2\Http2Parser->parse(NULL) #5 [internal function]: Amp\Http\Server\Driver\Http2Driver->parser(NULL) #6 /home/kelunik/GitHub/amphp/http-server/src/Driver/RemoteClient.php(385): Generator->send('\x00\x04\x01\x00\x01\x00\x00\x00\x05------...') #7 /home/kelunik/GitHub/amphp/http-server/src/Driver/RemoteClient.php(368): Amp\Http\Server\Driver\RemoteClient->parse('\x00\x04\x01\x00\x01\x00\x00\x00\x05------...') #8 /home/kelunik/GitHub/amphp/http-server/vendor/amphp/amp/lib/Loop/NativeDriver.php(192): Amp\Http\Server\Driver\RemoteClient->onReadable('y', Resource id #134, NULL) #9 /home/kelunik/GitHub/amphp/http-server/vendor/amphp/amp/lib/Loop/NativeDriver.php(97): Amp\Loop\NativeDriver->selectStreams(Array, Array, 0.324) #10 /home/kelunik/GitHub/amphp/http-server/vendor/amphp/amp/lib/Loop/Driver.php(134): Amp\Loop\NativeDriver->dispatch(true) #11 /home/kelunik/GitHub/amphp/http-server/vendor/amphp/amp/lib/Loop/Driver.php(72): Amp\Loop\Driver->tick() #12 /home/kelunik/GitHub/amphp/http-server/vendor/amphp/amp/lib/Loop.php(84): Amp\Loop\Driver->run() #13 /home/kelunik/GitHub/amphp/http-server/test.php(78): Amp\Loop::run(Object(Closure)) #14 {main} [] []