diff --git a/src/Api/AwaitableWebpage.php b/src/Api/AwaitableWebpage.php index d35e76ba..5af45d25 100644 --- a/src/Api/AwaitableWebpage.php +++ b/src/Api/AwaitableWebpage.php @@ -42,6 +42,13 @@ public function __construct( 'keys', 'drag', 'append', + // A repeated navigation restarts the page load the previous attempt was waiting + // for, so on a slow runner none of them ever finishes within an attempt. Playwright + // already waits for the load state itself. + 'navigate', + 'refresh', + 'back', + 'forward', ], ) { // diff --git a/src/Drivers/LaravelHttpServer.php b/src/Drivers/LaravelHttpServer.php index f2af1a21..fcd42754 100644 --- a/src/Drivers/LaravelHttpServer.php +++ b/src/Drivers/LaravelHttpServer.php @@ -374,11 +374,15 @@ private function handleRequest(AmpRequest $request): Response } } - return new Response( - $response->getStatusCode(), - $response->headers->all(), // @phpstan-ignore-line - $content, - ); + // Headers before the body: the constructor sets the body first and then replaces every + // header with the given ones, which drops the Content-Length the body had just set. Without + // one, a 204 or 304 is framed chunked and its terminator reaches Chromium as the start of the + // next response on the socket. + $ampResponse = new Response($response->getStatusCode()); + $ampResponse->setHeaders($response->headers->all()); // @phpstan-ignore-line + $ampResponse->setBody($content); + + return $ampResponse; } /** @@ -488,7 +492,9 @@ private function parseMultipartBody(string $body, string $boundary): array $files = []; foreach (explode("--{$boundary}", $body) as $part) { - $part = mb_ltrim($part, "\r\n"); + // Byte for byte, not mb_ltrim(): that replaces every byte which is not valid UTF-8, and a + // file part is binary. + $part = (string) preg_replace('/^[\r\n]+/', '', $part); if ($part === '' || str_starts_with($part, '--')) { continue; } diff --git a/src/Execution.php b/src/Execution.php index 394ca307..3a482a78 100644 --- a/src/Execution.php +++ b/src/Execution.php @@ -126,9 +126,14 @@ public function waitForExpectation(callable $callback): mixed $start = microtime(true); $end = $start + ($timeout / 1_000); + // Each attempt bounds the Playwright calls it makes, so a page that loads or paints slowly + // gets a longer attempt when the configured timeout is longer, instead of every attempt + // failing at the same one second on a busy CI runner. + $attempt = max(1_000, intdiv($timeout, 5)); + while (microtime(true) < $end) { try { - return Playwright::usingTimeout(1_000, $callback); + return Playwright::usingTimeout($attempt, $callback); } catch (ExpectationFailedException) { // } diff --git a/src/Playwright/Client.php b/src/Playwright/Client.php index 863b0936..3dca3e56 100644 --- a/src/Playwright/Client.php +++ b/src/Playwright/Client.php @@ -107,6 +107,15 @@ public function execute(string $guid, string $method, array $params = [], array /** @var array{id: string|null, params: array{add: string|null}, error: array{error: array{message: string|null}}} $response */ $response = json_decode($responseJson, true); + // A response to another request is the late answer of one whose consumer stopped + // reading before it arrived: a navigation once it reached its waitUntil state, a + // querySelector once the handle was created. Its result and its error belong to that + // request. Taken as this one's, the error fails an assertion that passed, and the + // result reaches whichever call reads the next value, an evaluate() included. + if (isset($response['id']) && $response['id'] !== $requestId) { + continue; + } + if (isset($response['error']['error']['message'])) { $message = $response['error']['error']['message']; diff --git a/tests/Browser/Visit/BodilessResponseTest.php b/tests/Browser/Visit/BodilessResponseTest.php new file mode 100644 index 00000000..77b9b0b7 --- /dev/null +++ b/tests/Browser/Visit/BodilessResponseTest.php @@ -0,0 +1,69 @@ +http(); + assert($http instanceof LaravelHttpServer); + + $socket = connect("127.0.0.1:{$http->port}"); + $socket->write("GET {$path} HTTP/1.1\r\nHost: 127.0.0.1\r\nConnection: keep-alive\r\n\r\n"); + + $raw = ''; + + try { + while (($chunk = $socket->read(new TimeoutCancellation(0.4))) !== null) { + $raw .= $chunk; + } + } catch (CancelledException) { + // Silence is the end of the response: the connection stays open on keep-alive. + } + + $socket->close(); + + return $raw; +} + +it('writes nothing after the headers of a response that has no body', function (int $status): void { + Route::get('/', fn (): string => 'home'); + Route::get('/bodiless', fn (): Response => new Response('', $status)); + + // Starts the in-process server; the response under test is then read off the wire. + visit('/')->assertSee('home'); + + $raw = bodilessResponseWireTo('/bodiless'); + + expect($raw)->toStartWith("HTTP/1.1 {$status} ") + ->and($raw)->toEndWith("\r\n\r\n") + // The bytes Chromium reads as the start of the next response on the socket. + ->and($raw)->not->toContain("0\r\n\r\n") + ->and(mb_strtolower($raw))->not->toContain('transfer-encoding: chunked'); +})->with([204, 304]); + +it('writes a response that has a body with its length', function (): void { + Route::get('/', fn (): string => 'home'); + Route::get('/body', fn (): string => 'hello'); + + visit('/')->assertSee('home'); + + $raw = bodilessResponseWireTo('/body'); + + expect($raw)->toStartWith('HTTP/1.1 200 ') + ->and(mb_strtolower($raw))->toContain("content-length: 5\r\n") + ->and($raw)->toEndWith("\r\n\r\nhello"); +}); diff --git a/tests/Browser/Webpage/UploadTest.php b/tests/Browser/Webpage/UploadTest.php index 82994b20..a3bb1732 100644 --- a/tests/Browser/Webpage/UploadTest.php +++ b/tests/Browser/Webpage/UploadTest.php @@ -39,3 +39,27 @@ unlink($tempFile); }); + +it('keeps the bytes of an uploaded file that are not valid UTF-8', function (): void { + Route::get('/', fn (): string => ' +
+ + +
+ '); + Route::post('/upload', fn (Request $request): string => 'received '.bin2hex( + (string) file_get_contents($request->file('avatar')->getPathname()), + )); + + // The first bytes of a PNG, which no UTF-8 decoder accepts. + $bytes = "\x89PNG\r\n\x1a\n\x00\x00\x00\x0dIHDR\xff\xfe\x80"; + $tempFile = tempnam(sys_get_temp_dir(), 'test'); + file_put_contents($tempFile, $bytes); + + visit('/') + ->attach('#avatar', $tempFile) + ->click('Upload') + ->assertSee('received '.bin2hex($bytes)); + + unlink($tempFile); +}); diff --git a/tests/Unit/Playwright/ClientTest.php b/tests/Unit/Playwright/ClientTest.php new file mode 100644 index 00000000..2c06a7ca --- /dev/null +++ b/tests/Unit/Playwright/ClientTest.php @@ -0,0 +1,195 @@ +> $messages + */ +function clientAnsweringWith(array $messages): Client +{ + $connection = new class($messages) implements IteratorAggregate, WebsocketConnection + { + private string $requestId = ''; + + /** + * @param array> $messages + */ + public function __construct(private array $messages) + { + // + } + + public function sendText(string $data): void + { + /** @var array{id: string} $request */ + $request = json_decode($data, true); + + $this->requestId = $request['id']; + } + + public function receive(?Cancellation $cancellation = null): WebsocketMessage + { + $message = array_shift($this->messages); + + if ($message === null) { + throw new LogicException('The request was answered, yet the client keeps reading.'); + } + + return WebsocketMessage::fromText(str_replace('{id}', $this->requestId, (string) json_encode($message))); + } + + public function getIterator(): Iterator + { + yield from []; + } + + public function getHandshakeResponse(): Response + { + throw new LogicException('Not part of the scripted exchange.'); + } + + public function getId(): int + { + return 0; + } + + public function getLocalAddress(): SocketAddress + { + throw new LogicException('Not part of the scripted exchange.'); + } + + public function getRemoteAddress(): SocketAddress + { + throw new LogicException('Not part of the scripted exchange.'); + } + + public function getTlsInfo(): ?TlsInfo + { + return null; + } + + public function getCloseInfo(): WebsocketCloseInfo + { + throw new LogicException('Not part of the scripted exchange.'); + } + + public function isCompressionEnabled(): bool + { + return false; + } + + public function sendBinary(string $data): void + { + // + } + + public function streamText(ReadableStream $stream): void + { + // + } + + public function streamBinary(ReadableStream $stream): void + { + // + } + + public function ping(): void + { + // + } + + public function getCount(WebsocketCount $type): int + { + return 0; + } + + public function getTimestamp(WebsocketTimestamp $type): float + { + return 0.0; + } + + public function isClosed(): bool + { + return false; + } + + public function close(int $code = WebsocketCloseCode::NORMAL_CLOSE, string $reason = ''): void + { + // + } + + public function onClose(Closure $onClose): void + { + // + } + }; + + $client = new Client; + + new ReflectionProperty(Client::class, 'websocketConnection')->setValue($client, $connection); + + return $client; +} + +it('passes over the late error of an earlier request', function (): void { + $client = clientAnsweringWith([ + ['id' => 'earlier', 'error' => ['error' => ['message' => 'Timeout 1000ms exceeded.']]], + ['id' => '{id}', 'result' => ['value' => ['b' => true]]], + ]); + + $messages = iterator_to_array($client->execute('frame@1', 'isVisible', ['selector' => '#x']), false); + + expect($messages)->toHaveCount(1) + ->and($messages[0]['result']['value'])->toBe(['b' => true]); +}); + +it('passes over the late result of an earlier request', function (): void { + $client = clientAnsweringWith([ + ['id' => 'earlier', 'result' => ['value' => ['b' => false]]], + ['id' => '{id}', 'result' => ['value' => ['a' => []]]], + ]); + + $messages = iterator_to_array($client->execute('frame@1', 'evaluateExpression', ['expression' => 'window.errors']), false); + + expect($messages)->toHaveCount(1) + ->and($messages[0]['result']['value'])->toBe(['a' => []]); +}); + +it('throws the error of its own request', function (): void { + $client = clientAnsweringWith([ + ['id' => '{id}', 'error' => ['error' => ['message' => 'Timeout 1000ms exceeded.']]], + ]); + + expect(fn (): array => iterator_to_array($client->execute('frame@1', 'isVisible', ['selector' => '#x']), false)) + ->toThrow(ExpectationFailedException::class, 'Timeout 1000ms exceeded.'); +}); + +it('hands on the events that arrive before its response', function (): void { + $client = clientAnsweringWith([ + ['method' => '__create__', 'params' => ['type' => 'ElementHandle', 'guid' => 'handle@1']], + ['id' => '{id}', 'result' => ['element' => ['guid' => 'handle@1']]], + ]); + + $messages = iterator_to_array($client->execute('frame@1', 'querySelector', ['selector' => '#x']), false); + + expect($messages)->toHaveCount(2) + ->and($messages[0]['method'])->toBe('__create__') + ->and($messages[1]['result']['element']['guid'])->toBe('handle@1'); +});