From cccbc5e8cc0cf6e7c04de012a35e37b35beb2215 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Fri, 7 Feb 2025 14:27:37 +0100 Subject: [PATCH 01/11] allow UploadedFiles to use streams that are not based on files --- src/Psr7/Message/UploadedFile.php | 38 ++++++++++++++++++------ tests/UploadedFileTest.php | 48 ++++++++++++++++++++++++++++--- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/src/Psr7/Message/UploadedFile.php b/src/Psr7/Message/UploadedFile.php index e6984cc..22e9237 100644 --- a/src/Psr7/Message/UploadedFile.php +++ b/src/Psr7/Message/UploadedFile.php @@ -6,11 +6,11 @@ use Psr\Http\Message\StreamInterface; use Psr\Http\Message\UploadedFileInterface; use RuntimeException; +use Throwable; class UploadedFile implements UploadedFileInterface { protected bool $moved = false; - protected string $filePath; /** * @param StreamInterface $stream @@ -27,11 +27,6 @@ public function __construct( protected ?string $clientMediaType = null ) { - $file = $this->stream->getMetadata('uri'); - if ($file === null) { - throw new InvalidArgumentException('Stream uri not available'); - } - $this->filePath = $file; } /** @@ -50,9 +45,36 @@ public function moveTo(string $targetPath): void if ($this->moved) { throw new RuntimeException('Uploaded file already moved'); } - if (!@rename($this->filePath, $targetPath)) { - throw new RuntimeException('Uploaded file could not be moved'); + + $filePath = $this->stream->getMetadata('uri'); + if ($filePath !== null) { + if (!@rename($filePath, $targetPath)) { + throw new RuntimeException('Uploaded file could not be moved'); + } + $this->moved = true; + return; } + + if ($this->stream->tell() !== 0) { + if (!$this->stream->isSeekable()) { + throw new RuntimeException('Stream needs to be rewound, but is not seekable'); + } + $this->stream->rewind(); + } + + $target = fopen($targetPath, 'wb'); + if ($target === false) { + throw new RuntimeException('Target path could not be opened'); + } + + try { + while (!$this->stream->eof()) { + fwrite($target, $this->stream->read(4096)); + } + } finally { + fclose($target); + } + $this->moved = true; } diff --git a/tests/UploadedFileTest.php b/tests/UploadedFileTest.php index 11091b4..3037a1c 100644 --- a/tests/UploadedFileTest.php +++ b/tests/UploadedFileTest.php @@ -3,6 +3,7 @@ namespace Tests; use Aternos\CurlPsr\Psr17\Psr17Factory; +use Aternos\CurlPsr\Psr7\Stream\StringStream; use InvalidArgumentException; use PHPUnit\Framework\TestCase; use Psr\Http\Message\StreamInterface; @@ -66,11 +67,50 @@ public function testThrowWhenMovedToInvalidTarget(): void $uploadedFile->moveTo($target); } - public function testThrowIfStreamIsNotAFile(): void + public function testWriteStreamContentIfStreamIsNotAFile(): void { $stream = $this->factory->createStream('test'); - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Stream uri not available'); - $this->factory->createUploadedFile($stream, $stream->getSize(), UPLOAD_ERR_OK, "file.txt", "text/plain"); + $upload = $this->factory->createUploadedFile($stream, $stream->getSize(), UPLOAD_ERR_OK, "file.txt", "text/plain"); + + $this->target = tempnam(sys_get_temp_dir(), 'test'); + $upload->moveTo($this->target); + + $this->assertFileExists($this->target); + $this->assertEquals('test', file_get_contents($this->target)); + } + + public function testThrowIfWriteTargetCannotBeOpened(): void + { + $stream = $this->factory->createStream('test'); + $upload = $this->factory->createUploadedFile($stream, $stream->getSize(), UPLOAD_ERR_OK, "file.txt", "text/plain"); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Target path could not be opened'); + $upload->moveTo(sys_get_temp_dir() . "/non-existing-dir/file.txt"); + } + + public function testRewindStreamIfNecessary(): void + { + $stream = $this->factory->createStream('test'); + $stream->read(1); + $upload = $this->factory->createUploadedFile($stream, $stream->getSize(), UPLOAD_ERR_OK, "file.txt", "text/plain"); + + $this->target = tempnam(sys_get_temp_dir(), 'test'); + $upload->moveTo($this->target); + + $this->assertFileExists($this->target); + $this->assertEquals('test', file_get_contents($this->target)); + } + + public function testThrowIfStreamNeedsRewindButIsNotSeekable(): void + { + $stream = new StringStream('test', false); + $stream->read(1); + $upload = $this->factory->createUploadedFile($stream, $stream->getSize(), UPLOAD_ERR_OK, "file.txt", "text/plain"); + $this->target = tempnam(sys_get_temp_dir(), 'test'); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Stream needs to be rewound, but is not seekable'); + $upload->moveTo($this->target); } } From 2cb8277ffb99e2b590c49c8bc2a9c8e11997ab1b Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Fri, 7 Feb 2025 14:31:52 +0100 Subject: [PATCH 02/11] fix warning --- src/Psr7/Message/UploadedFile.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Psr7/Message/UploadedFile.php b/src/Psr7/Message/UploadedFile.php index 22e9237..1012143 100644 --- a/src/Psr7/Message/UploadedFile.php +++ b/src/Psr7/Message/UploadedFile.php @@ -62,7 +62,7 @@ public function moveTo(string $targetPath): void $this->stream->rewind(); } - $target = fopen($targetPath, 'wb'); + $target = @fopen($targetPath, 'wb'); if ($target === false) { throw new RuntimeException('Target path could not be opened'); } From cab1d5d0eb59a00179f26f5f175cbfeb6ae15eb7 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Tue, 18 Feb 2025 13:48:15 +0100 Subject: [PATCH 03/11] fix empty strings returned from request body streams being interpreted as eof --- src/Psr18/Client.php | 5 ++++- tests/HttpClientTest.php | 27 ++++++++++++++++++++++++++ tests/Stream/PredefinedChunkStream.php | 2 +- tests/Util/TestCurlHandle.php | 2 +- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/Psr18/Client.php b/src/Psr18/Client.php index 378f4fe..1f92629 100644 --- a/src/Psr18/Client.php +++ b/src/Psr18/Client.php @@ -85,7 +85,10 @@ protected function initRequest(RequestInterface $request, ResponseHeaderParser $ $ch->setopt(CURLOPT_UPLOAD, true); $ch->setopt(CURLOPT_READFUNCTION, function ($ch, $fd, $length) use ($requestBody) { - return $requestBody->read($length); + do { + $chunk = $requestBody->read($length); + } while ($chunk === "" && $requestBody->eof() === false); + return $chunk; }); $ch->setopt(CURLOPT_WRITEFUNCTION, /** @throws Throwable */ function ($ch, $data) { diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 83ec9e8..4d73ecb 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -144,6 +144,33 @@ public function testThrowRequestExceptionIfRequestBodyThrows(): void $this->client->sendRequest($request); } + public function testRequestBodyStreamCanReturnEmptyChunks(): void + { + $requestBody = new PredefinedChunkStream([ + "something", + "", + "else" + ]); + + $chunks = []; + $this->curlHandle->setOnAfterRead(function ($chunk) use (&$chunks) { + $chunks[] = $chunk; + }); + + $request = $this->requestFactory->createRequest("POST", "https://example.com") + ->withBody($requestBody); + + $this->client->sendRequest($request); + + // Last chunk is empty + $this->assertEmpty(array_pop($chunks)); + + // No empty chunks in between + foreach ($chunks as $chunk) { + $this->assertNotEmpty($chunk); + } + } + #[TestWith([CURLE_COULDNT_CONNECT, "CURLE_COULDNT_CONNECT"], "CURLE_COULDNT_CONNECT")] #[TestWith([CURLE_COULDNT_RESOLVE_HOST, "CURLE_COULDNT_RESOLVE_HOST"], "CURLE_COULDNT_RESOLVE_HOST")] #[TestWith([CURLE_COULDNT_RESOLVE_PROXY, "CURLE_COULDNT_RESOLVE_PROXY"], "CURLE_COULDNT_RESOLVE_PROXY")] diff --git a/tests/Stream/PredefinedChunkStream.php b/tests/Stream/PredefinedChunkStream.php index e43f00f..ee18ac8 100644 --- a/tests/Stream/PredefinedChunkStream.php +++ b/tests/Stream/PredefinedChunkStream.php @@ -126,7 +126,7 @@ public function isReadable(): bool */ public function read(int $length): string { - while (strlen($this->buffer) === 0 && count($this->chunks) > 0) { + if (strlen($this->buffer) === 0 && count($this->chunks) > 0) { $chunk = array_shift($this->chunks); if ($chunk instanceof Throwable) { throw $chunk; diff --git a/tests/Util/TestCurlHandle.php b/tests/Util/TestCurlHandle.php index de244b3..68245d1 100644 --- a/tests/Util/TestCurlHandle.php +++ b/tests/Util/TestCurlHandle.php @@ -71,7 +71,7 @@ public function doExec(): string|bool $chunk = $this->options[CURLOPT_READFUNCTION](null, $this->options[CURLOPT_INFILE] ?? null, 8192); $this->requestBodySink?->write($chunk); $uploaded += strlen($chunk); - $this->onAfterRead?->call($this); + $this->onAfterRead?->call($this, $chunk); $this->progressUpdate(0, 0, $this->getRequestHeader("Content-Length") ?? 0, $uploaded); } while (strlen($chunk) > 0); } From bd3718dea2496310f83291d92bd62282c1d2bfde Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Tue, 4 Mar 2025 16:49:41 +0100 Subject: [PATCH 04/11] allow setting request http protocol version to 1.1 --- src/Psr18/Client.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Psr18/Client.php b/src/Psr18/Client.php index 1f92629..2307c9f 100644 --- a/src/Psr18/Client.php +++ b/src/Psr18/Client.php @@ -74,6 +74,10 @@ protected function initRequest(RequestInterface $request, ResponseHeaderParser $ if ($request->getProtocolVersion() === "1.0") { $ch->setopt(CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_0); + } else if ($request->getProtocolVersion() === "1.1") { + $ch->setopt(CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1); + } else if ($request->getProtocolVersion() === "2.0") { + $ch->setopt(CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0); } $requestBody = $request->getBody(); From cee91ea3e7b9ab20574151b664841576bb053e41 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Tue, 4 Mar 2025 16:53:27 +0100 Subject: [PATCH 05/11] test setting all supported protocol versions --- tests/HttpClientTest.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 4d73ecb..1d52a69 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -105,13 +105,16 @@ public function testEmptyReasonPhrase(): void $this->assertEquals("GET", $this->curlHandle->getOption(CURLOPT_CUSTOMREQUEST)); } - public function testForceHttp10(): void + #[TestWith(["1.0", CURL_HTTP_VERSION_1_0])] + #[TestWith(["1.1", CURL_HTTP_VERSION_1_1])] + #[TestWith(["2.0", CURL_HTTP_VERSION_2_0])] + public function testHttpProtocolVersions(string $versionString, int $curlValue): void { $request = $this->requestFactory->createRequest("GET", "https://example.com") - ->withProtocolVersion("1.0"); + ->withProtocolVersion($versionString); $this->client->sendRequest($request); - $this->assertEquals(CURL_HTTP_VERSION_1_0, $this->curlHandle->getOption(CURLOPT_HTTP_VERSION)); + $this->assertEquals($curlValue, $this->curlHandle->getOption(CURLOPT_HTTP_VERSION)); } public function testRemoveEncodingAndLengthHeaderForEncodedResponse(): void From e170b996674922f48223e6c5baba26d9cc4b173d Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Sun, 9 Mar 2025 19:29:00 +0100 Subject: [PATCH 06/11] handle redirects directly instead of letting curl do it --- src/Exception/RequestRedirectedException.php | 10 + src/Exception/TooManyRedirectsException.php | 8 + src/Psr18/Client.php | 232 +++++++++++++++--- src/Psr18/ClientOptions.php | 16 ++ src/Psr18/UriResolver/UriResolver.php | 109 ++++++++ .../UriResolver/UriResolverInterface.php | 18 ++ src/Psr7/Message/Message.php | 4 +- src/Psr7/Stream/EmptyStream.php | 120 +++++++++ src/Psr7/Stream/StreamMetaDataTrait.php | 32 +++ src/Psr7/Stream/StringStream.php | 28 +-- tests/HttpClientTest.php | 8 +- tests/HttpClientTestCase.php | 2 +- tests/UriResolverTest.php | 48 ++++ 13 files changed, 566 insertions(+), 69 deletions(-) create mode 100644 src/Exception/RequestRedirectedException.php create mode 100644 src/Exception/TooManyRedirectsException.php create mode 100644 src/Psr18/ClientOptions.php create mode 100644 src/Psr18/UriResolver/UriResolver.php create mode 100644 src/Psr18/UriResolver/UriResolverInterface.php create mode 100644 src/Psr7/Stream/EmptyStream.php create mode 100644 src/Psr7/Stream/StreamMetaDataTrait.php create mode 100644 tests/UriResolverTest.php diff --git a/src/Exception/RequestRedirectedException.php b/src/Exception/RequestRedirectedException.php new file mode 100644 index 0000000..4232cfd --- /dev/null +++ b/src/Exception/RequestRedirectedException.php @@ -0,0 +1,10 @@ +responseFactory = $responseFactory ?? new Psr17Factory(); - $this->curlHandleFactory = $curlHandleFactory ?? new CurlHandleFactory(); + $factory = new Psr17Factory(); + $this->responseFactory = $responseFactory ?? $factory; + $this->uriFactory = $uriFactory ?? $factory; + $this->uriResolver = $uriResolver ?? new UriResolver($this->uriFactory); + $this->curlHandleFactory = new CurlHandleFactory(); + $this->options = new ClientOptions(); + } + + /** + * @param CurlHandleFactoryInterface $curlHandleFactory + * @return $this + * @internal Used for testing + */ + public function setCurlHandleFactory(CurlHandleFactoryInterface $curlHandleFactory): static + { + $this->curlHandleFactory = $curlHandleFactory; + return $this; } /** * @param RequestInterface $request * @param ResponseHeaderParser $headerParser + * @param ClientOptions $options * @return CurlHandleInterface */ - protected function initRequest(RequestInterface $request, ResponseHeaderParser $headerParser): CurlHandleInterface + protected function initRequest(RequestInterface $request, ResponseHeaderParser $headerParser, ClientOptions $options): CurlHandleInterface { $ch = $this->curlHandleFactory->createCurlHandle(); - foreach ($this->customCurlOptions as $option => $value) { + foreach ($options->curlOptions as $option => $value) { $ch->setopt($option, $value); } $ch->setopt(CURLOPT_URL, $request->getUri()); $ch->setopt(CURLOPT_REQUEST_TARGET, $request->getRequestTarget()); $ch->setopt(CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS); - $ch->setopt(CURLOPT_FOLLOWLOCATION, $this->maxRedirects > 0); - $ch->setopt(CURLOPT_MAXREDIRS, $this->maxRedirects); - $ch->setopt(CURLOPT_COOKIEFILE, $this->cookieFile); - $ch->setopt(CURLOPT_TIMEOUT, $this->timeout); + $ch->setopt(CURLOPT_FOLLOWLOCATION, false); + $ch->setopt(CURLOPT_COOKIEFILE, $options->cookieFile); + $ch->setopt(CURLOPT_TIMEOUT, $options->timeout); $ch->setopt(CURLOPT_ACCEPT_ENCODING, ""); if ($request->getProtocolVersion() === "1.0") { @@ -100,7 +120,7 @@ protected function initRequest(RequestInterface $request, ResponseHeaderParser $ return strlen($data); }); - $progressCallback = $this->createProgressCallback($request); + $progressCallback = $this->createProgressCallback($request, $options); if ($progressCallback !== null) { $ch->setopt(CURLOPT_NOPROGRESS, false); if ($this->shouldUseXferInfoFunction()) { @@ -131,11 +151,12 @@ protected function shouldUseXferInfoFunction(): bool * as to not expose the cURL handle to the user * * @param RequestInterface $request + * @param ClientOptions $options * @return Closure|null */ - protected function createProgressCallback(RequestInterface $request): ?Closure + protected function createProgressCallback(RequestInterface $request, ClientOptions $options): ?Closure { - $callback = $this->getProgressCallback(); + $callback = $options->progressCallback; if ($callback === null) { return null; } @@ -147,18 +168,35 @@ protected function createProgressCallback(RequestInterface $request): ?Closure */ public function sendRequest(RequestInterface $request): ResponseInterface { - foreach ($this->defaultHeaders as $name => $values) { + return $this->doSendRequest($request, clone $this->options); + } + + /** + * Actually send the request + * + * @param RequestInterface $request + * @param ClientOptions $options + * @param int $redirects + * @return ResponseInterface + * @throws NetworkException + * @throws RequestException + */ + protected function doSendRequest(RequestInterface $request, ClientOptions $options, int $redirects = 0): ResponseInterface + { + foreach ($options->defaultHeaders as $name => $values) { if (!$request->hasHeader($name)) { $request = $request->withHeader($name, $values); } } $headerParser = new ResponseHeaderParser(); - $ch = $this->initRequest($request, $headerParser); + $ch = $this->initRequest($request, $headerParser, $options); $fiber = new Fiber(function () use ($ch) { try { $ch->exec(); + } catch (RequestRedirectedException) { + // ignore } catch (Throwable $e) { $ch->close(); throw $e; @@ -188,6 +226,15 @@ public function sendRequest(RequestInterface $request): ResponseInterface $response = $headerParser->applyToResponse($response); + if ($this->isRedirect($response)) { + try { + $fiber->throw(new RequestRedirectedException()); + } catch (Throwable $e) { + throw new RequestException($request, "Could not close request before redirect", previous: $e); + } + return $this->handleRedirect($request, $response, $options, $redirects); + } + $size = null; if ($response->hasHeader("Content-Encoding")) { $response = $response @@ -203,6 +250,89 @@ public function sendRequest(RequestInterface $request): ResponseInterface return $response->withBody(new ResponseStream($ch, $fiber, $initial ?? "", $size)); } + /** + * @param ResponseInterface $response + * @return bool + */ + protected function isRedirect(ResponseInterface $response): bool + { + if ($response->getStatusCode() > 300 && $response->getStatusCode() < 400) { + return true; + } + + return $response->getStatusCode() === 300 && $response->hasHeader("Location"); + } + + /** + * Handle a redirect response + * + * @param RequestInterface $request + * @param ResponseInterface $response + * @param ClientOptions $options + * @param int $redirects + * @return ResponseInterface + * @throws NetworkException + * @throws RequestException + * @throws TooManyRedirectsException + */ + protected function handleRedirect(RequestInterface $request, ResponseInterface $response, ClientOptions $options, int $redirects): ResponseInterface + { + if ($redirects >= $options->maxRedirects) { + throw new TooManyRedirectsException($request, "Redirect limit of " . $options->maxRedirects . " reached"); + } + + $locationHeaders = $response->getHeader("Location"); + if (count($locationHeaders) === 0) { + throw new RequestException($request, "Redirect without location header"); + } + if (count($locationHeaders) > 1) { + throw new RequestException($request, "Multiple location headers in redirect"); + } + + try { + $relativeUri = $this->uriFactory->createUri($locationHeaders[0]); + } catch (Throwable $e) { + throw new RequestException($request, "Invalid location header in redirect", previous: $e); + } + + $location = $this->uriResolver->resolve($request->getUri(), $relativeUri); + $request = $request->withUri($location); + + if (in_array($response->getStatusCode(), $options->redirectToGetStatusCodes)) { + $request = $request->withMethod("GET") + ->withBody(new EmptyStream()) + ->withoutHeader("Content-Length"); + return $this->doSendRequest($request, $options, $redirects + 1); + } + + try { + $this->rewindBody($request); + } catch (Throwable $e) { + throw new RequestException($request, "Could not rewind body for redirect", previous: $e); + } + + return $this->doSendRequest($request, $options, $redirects + 1); + } + + /** + * @param RequestInterface $request + * @return void + * @throws RequestException + */ + protected function rewindBody(RequestInterface $request): void + { + $body = $request->getBody(); + $offset = $body->tell(); + if ($offset === 0) { + return; + } + + if (!$body->isSeekable()) { + throw new RequestException($request, "Request body is not seekable"); + } + $body->rewind(); + } + /** * If there was an error, close the handle and throw an exception * @@ -256,7 +386,7 @@ protected function setHeaders(RequestInterface $request, CurlHandleInterface $ch */ public function setTimeout(int $timeout): static { - $this->timeout = $timeout; + $this->options->timeout = $timeout; return $this; } @@ -265,7 +395,7 @@ public function setTimeout(int $timeout): static */ public function getTimeout(): int { - return $this->timeout; + return $this->options->timeout; } /** @@ -276,7 +406,7 @@ public function getTimeout(): int */ public function setMaxRedirects(int $maxRedirects): static { - $this->maxRedirects = $maxRedirects; + $this->options->maxRedirects = $maxRedirects; return $this; } @@ -285,7 +415,7 @@ public function setMaxRedirects(int $maxRedirects): static */ public function getMaxRedirects(): int { - return $this->maxRedirects; + return $this->options->maxRedirects; } /** @@ -296,7 +426,7 @@ public function getMaxRedirects(): int */ public function setCookieFile(string $cookieFile): static { - $this->cookieFile = $cookieFile; + $this->options->cookieFile = $cookieFile; return $this; } @@ -305,7 +435,7 @@ public function setCookieFile(string $cookieFile): static */ public function getCookieFile(): string { - return $this->cookieFile; + return $this->options->cookieFile; } /** @@ -316,7 +446,7 @@ public function getCookieFile(): string */ public function setProgressCallback(?Closure $progressCallback): static { - $this->progressCallback = $progressCallback; + $this->options->progressCallback = $progressCallback; return $this; } @@ -325,7 +455,7 @@ public function setProgressCallback(?Closure $progressCallback): static */ public function getProgressCallback(): ?Closure { - return $this->progressCallback; + return $this->options->progressCallback; } /** @@ -337,7 +467,7 @@ public function getProgressCallback(): ?Closure */ public function setCurlOption(int $option, mixed $value): static { - $this->customCurlOptions[$option] = $value; + $this->options->curlOptions[$option] = $value; return $this; } @@ -349,7 +479,15 @@ public function setCurlOption(int $option, mixed $value): static */ public function getCurlOption(int $option): mixed { - return $this->customCurlOptions[$option] ?? null; + return $this->options->curlOptions[$option] ?? null; + } + + /** + * @return array + */ + public function getCurlOptions(): array + { + return $this->options->curlOptions; } /** @@ -363,7 +501,7 @@ public function getCurlOption(int $option): mixed */ public function setDefaultHeaders(array $headers): static { - $this->defaultHeaders = $headers; + $this->options->defaultHeaders = $headers; return $this; } @@ -374,13 +512,13 @@ public function setDefaultHeaders(array $headers): static */ public function addDefaultHeader(string $name, string ...$values): static { - foreach ($this->defaultHeaders as $headerName => $headerValues) { + foreach ($this->options->defaultHeaders as $headerName => $headerValues) { if (strtolower($headerName) === strtolower($name)) { - $this->defaultHeaders[$headerName] = $values; + $this->options->defaultHeaders[$headerName] = $values; return $this; } } - $this->defaultHeaders[$name] = $values; + $this->options->defaultHeaders[$name] = $values; return $this; } @@ -389,6 +527,28 @@ public function addDefaultHeader(string $name, string ...$values): static */ public function getDefaultHeaders(): array { - return $this->defaultHeaders; + return $this->options->defaultHeaders; + } + + /** + * Set a list of status codes that should be redirected to using GET. + * By default, only 303 responses are redirected to using GET, + * but historically 301 and 302 have also used this behavior. + * + * @param int[] $redirectToGetStatusCodes + * @return $this + */ + public function setRedirectToGetStatusCodes(array $redirectToGetStatusCodes): static + { + $this->options->redirectToGetStatusCodes = $redirectToGetStatusCodes; + return $this; + } + + /** + * @return int[] + */ + public function getRedirectToGetStatusCodes(): array + { + return $this->options->redirectToGetStatusCodes; } } diff --git a/src/Psr18/ClientOptions.php b/src/Psr18/ClientOptions.php new file mode 100644 index 0000000..2605c3f --- /dev/null +++ b/src/Psr18/ClientOptions.php @@ -0,0 +1,16 @@ +strict && $relativeUri->getScheme() === $baseUri->getScheme()) { + $relativeUri = $relativeUri->withScheme(""); + } + + if ($relativeUri->getScheme() !== "") { + return $relativeUri; + } + + if ($relativeUri->getAuthority() !== "") { + return $relativeUri + ->withPath($this->removeDotSegments($relativeUri->getPath())) + ->withScheme($baseUri->getScheme()); + } + + $result = $this->uriFactory->createUri(); + if ($relativeUri->getPath() === "") { + $result = $result->withPath($baseUri->getPath()); + if ($relativeUri->getQuery() !== "") { + $result = $result->withQuery($relativeUri->getQuery()); + } else { + $result = $result->withQuery($baseUri->getQuery()); + } + } else { + $path = $this->mergePaths($baseUri->getPath(), $relativeUri->getPath()); + $result = $result->withPath($this->removeDotSegments($path)) + ->withQuery($relativeUri->getQuery()); + } + $result = $result->withUserInfo($baseUri->getUserInfo()) + ->withHost($baseUri->getHost()) + ->withPort($baseUri->getPort()); + + return $result->withScheme($baseUri->getScheme()) + ->withFragment($relativeUri->getFragment()); + } + + /** + * @see https://datatracker.ietf.org/doc/html/rfc3986#section-5.2.3 + * @param string $base + * @param string $relative + * @return string + */ + protected function mergePaths(string $base, string $relative): string + { + if (str_starts_with($relative, "/")) { + return $relative; + } + + $index = strrpos($base, "/"); + if ($index === false) { + return "/" . $relative; + } + + return substr($base, 0, $index + 1) . $relative; + } + + /** + * @see https://datatracker.ietf.org/doc/html/rfc3986#section-5.2.4 + * @param string $path + * @return string + */ + protected function removeDotSegments(string $path): string + { + $resultParts = []; + $parts = explode("/", $path); + + foreach ($parts as $part) { + if ($part === "..") { + array_pop($resultParts); + } elseif ($part !== ".") { + $resultParts[] = $part; + } + } + + $result = implode("/", $resultParts); + + if (str_starts_with($path, "/") && !str_starts_with($result, "/")) { + $result = "/" . $result; + } else if ($result !== "" && in_array(end($parts), [".", ".."])) { + $result .= "/"; + } + + return $result; + } +} diff --git a/src/Psr18/UriResolver/UriResolverInterface.php b/src/Psr18/UriResolver/UriResolverInterface.php new file mode 100644 index 0000000..2068c9e --- /dev/null +++ b/src/Psr18/UriResolver/UriResolverInterface.php @@ -0,0 +1,18 @@ +body === null) { - $this->body = new StringStream(""); + $this->body = new EmptyStream(); } return $this->body; diff --git a/src/Psr7/Stream/EmptyStream.php b/src/Psr7/Stream/EmptyStream.php new file mode 100644 index 0000000..6665736 --- /dev/null +++ b/src/Psr7/Stream/EmptyStream.php @@ -0,0 +1,120 @@ + false, + "eof" => $this->eof(), + "unread_bytes" => $this->getSize() - $this->tell(), + "mode" => $this->approximateMode(), + "seekable" => $this->isSeekable(), + default => null + }; + } + + return [ + "timed_out" => $this->getMetadata("timed_out"), + "blocked" => $this->getMetadata("blocked"), + "eof" => $this->getMetadata("eof"), + "unread_bytes" => $this->getMetadata("unread_bytes"), + "mode" => $this->getMetadata("mode"), + "seekable" => $this->getMetadata("seekable"), + ]; + } +} diff --git a/src/Psr7/Stream/StringStream.php b/src/Psr7/Stream/StringStream.php index 3a13428..45b21c2 100644 --- a/src/Psr7/Stream/StringStream.php +++ b/src/Psr7/Stream/StringStream.php @@ -7,6 +7,8 @@ class StringStream implements StreamInterface { + use StreamMetaDataTrait; + protected int $position = 0; /** @@ -179,30 +181,4 @@ protected function approximateMode(): ?string } return null; } - - /** - * @inheritDoc - */ - public function getMetadata(?string $key = null) - { - if ($key !== null) { - return match ($key) { - "timed_out", "blocked" => false, - "eof" => $this->eof(), - "unread_bytes" => $this->getSize() - $this->tell(), - "mode" => $this->approximateMode(), - "seekable" => $this->isSeekable(), - default => null - }; - } - - return [ - "timed_out" => $this->getMetadata("timed_out"), - "blocked" => $this->getMetadata("blocked"), - "eof" => $this->getMetadata("eof"), - "unread_bytes" => $this->getMetadata("unread_bytes"), - "mode" => $this->getMetadata("mode"), - "seekable" => $this->getMetadata("seekable"), - ]; - } } diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 1d52a69..9e379d1 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -287,12 +287,12 @@ public function testFallbackToProgressFunctionIfXferInfoFunctionIsMissing(): voi $this->curlHandle ->setResponseBody($responseBody); - $this->client = new class($this->requestFactory, $this->curlHandleFactory) extends \Aternos\CurlPsr\Psr18\Client { + $this->client = (new class($this->requestFactory) extends \Aternos\CurlPsr\Psr18\Client { protected function shouldUseXferInfoFunction(): bool { return false; } - }; + })->setCurlHandleFactory($this->curlHandleFactory); $progressCalled = false; $this->client->setProgressCallback(function () use (&$progressCalled) { @@ -336,8 +336,8 @@ public function testMaxRedirects(): void $request = $this->requestFactory->createRequest("GET", "https://example.com"); $this->client->sendRequest($request); - $this->assertTrue($this->curlHandle->getOption(CURLOPT_FOLLOWLOCATION)); - $this->assertEquals(10, $this->curlHandle->getOption(CURLOPT_MAXREDIRS)); + // Redirects are handled by the client, not by curl + $this->assertFalse($this->curlHandle->getOption(CURLOPT_FOLLOWLOCATION)); } public function testDisableRedirects(): void diff --git a/tests/HttpClientTestCase.php b/tests/HttpClientTestCase.php index 4c2a123..8802493 100644 --- a/tests/HttpClientTestCase.php +++ b/tests/HttpClientTestCase.php @@ -27,6 +27,6 @@ protected function setUp(): void $this->curlHandleFactory = new TestCurlHandleFactory(); $this->curlHandle = $this->curlHandleFactory->nextTestHandle(); - $this->client = new Client($psrFactory, $this->curlHandleFactory); + $this->client = (new Client($psrFactory))->setCurlHandleFactory($this->curlHandleFactory); } } diff --git a/tests/UriResolverTest.php b/tests/UriResolverTest.php new file mode 100644 index 0000000..a1fad62 --- /dev/null +++ b/tests/UriResolverTest.php @@ -0,0 +1,48 @@ +createUri(self::BASE_URI); + $relativeUri = $factory->createUri($relativeUri); + $resolvedUri = $resolver->resolve($baseUri, $relativeUri); + $this->assertEquals($expectedUri, (string)$resolvedUri); + } +} From 167c6718521fcd86d70520d8ec50336b7a892ae4 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Sun, 9 Mar 2025 19:43:08 +0100 Subject: [PATCH 07/11] test EmptyStream and UriResolver --- src/Psr18/UriResolver/UriResolver.php | 8 +------- src/Psr7/Stream/StreamMetaDataTrait.php | 17 ++++++++++++++++ src/Psr7/Stream/StringStream.php | 17 ---------------- tests/StreamTest.php | 26 +++++++++++++++++++++++++ tests/UriResolverTest.php | 5 +++-- 5 files changed, 47 insertions(+), 26 deletions(-) diff --git a/src/Psr18/UriResolver/UriResolver.php b/src/Psr18/UriResolver/UriResolver.php index 004401b..5856875 100644 --- a/src/Psr18/UriResolver/UriResolver.php +++ b/src/Psr18/UriResolver/UriResolver.php @@ -9,11 +9,9 @@ class UriResolver implements UriResolverInterface { /** * @param UriFactoryInterface $uriFactory - * @param bool $strict */ public function __construct( - protected UriFactoryInterface $uriFactory, - protected bool $strict = true + protected UriFactoryInterface $uriFactory ) { } @@ -23,10 +21,6 @@ public function __construct( */ public function resolve(UriInterface $baseUri, UriInterface $relativeUri): UriInterface { - if (!$this->strict && $relativeUri->getScheme() === $baseUri->getScheme()) { - $relativeUri = $relativeUri->withScheme(""); - } - if ($relativeUri->getScheme() !== "") { return $relativeUri; } diff --git a/src/Psr7/Stream/StreamMetaDataTrait.php b/src/Psr7/Stream/StreamMetaDataTrait.php index 0ff7759..3d26025 100644 --- a/src/Psr7/Stream/StreamMetaDataTrait.php +++ b/src/Psr7/Stream/StreamMetaDataTrait.php @@ -4,6 +4,23 @@ trait StreamMetaDataTrait { + /** + * @return string|null + */ + protected function approximateMode(): ?string + { + if ($this->isReadable() && $this->isWritable()) { + return "r+"; + } + if ($this->isReadable()) { + return "r"; + } + if ($this->isWritable()) { + return "w"; + } + return null; + } + /** * @inheritDoc */ diff --git a/src/Psr7/Stream/StringStream.php b/src/Psr7/Stream/StringStream.php index 45b21c2..6085bf8 100644 --- a/src/Psr7/Stream/StringStream.php +++ b/src/Psr7/Stream/StringStream.php @@ -164,21 +164,4 @@ public function getContents(): string } return substr($this->data, $this->position); } - - /** - * @return string|null - */ - protected function approximateMode(): ?string - { - if ($this->isReadable() && $this->isWritable()) { - return "r+"; - } - if ($this->isReadable()) { - return "r"; - } - if ($this->isWritable()) { - return "w"; - } - return null; - } } diff --git a/tests/StreamTest.php b/tests/StreamTest.php index 788e3b6..f2f99a6 100644 --- a/tests/StreamTest.php +++ b/tests/StreamTest.php @@ -2,6 +2,7 @@ namespace Tests; +use Aternos\CurlPsr\Psr7\Stream\EmptyStream; use Aternos\CurlPsr\Psr7\Stream\Stream; use PHPUnit\Framework\TestCase; use ReflectionObject; @@ -254,4 +255,29 @@ public function testStatFails(): void $this->assertNull($stream->getSize()); } + + public function testEmptyStream(): void + { + $stream = new EmptyStream(); + $this->assertEquals("", (string)$stream); + $this->assertNull($stream->detach()); + $this->assertEquals(0, $stream->getSize()); + $this->assertEquals(0, $stream->tell()); + $this->assertTrue($stream->eof()); + $this->assertTrue($stream->isSeekable()); + $this->assertTrue($stream->isReadable()); + $this->assertFalse($stream->isWritable()); + $this->assertIsArray($stream->getMetadata()); + $this->assertEquals(0, $stream->getMetadata("unread_bytes")); + $this->assertEquals("", $stream->read(10)); + $this->assertEquals("", $stream->getContents()); + + // Do nothing + $stream->seek(2); + $stream->rewind(); + $stream->close(); + + $this->expectException(RuntimeException::class); + $stream->write("test"); + } } diff --git a/tests/UriResolverTest.php b/tests/UriResolverTest.php index a1fad62..aaeeaf1 100644 --- a/tests/UriResolverTest.php +++ b/tests/UriResolverTest.php @@ -36,11 +36,12 @@ class UriResolverTest extends TestCase #[TestWith(["../../g", "http://a/g"])] #[TestWith(["../../../g", "http://a/g"])] #[TestWith(["../../../../g", "http://a/g"])] - public function testRelativeResolution(string $relativeUri, string $expectedUri) + #[TestWith(["abc", "http://a/abc", "http://a"])] + public function testRelativeResolution(string $relativeUri, string $expectedUri, string $baseUriString = self::BASE_URI): void { $factory = new Psr17Factory(); $resolver = new UriResolver($factory); - $baseUri = $factory->createUri(self::BASE_URI); + $baseUri = $factory->createUri($baseUriString); $relativeUri = $factory->createUri($relativeUri); $resolvedUri = $resolver->resolve($baseUri, $relativeUri); $this->assertEquals($expectedUri, (string)$resolvedUri); From 35084763cafe540c34fef98dbf192ad51eb88a11 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Sun, 9 Mar 2025 20:50:51 +0100 Subject: [PATCH 08/11] test redirects --- src/Psr18/Client.php | 10 +- tests/HttpClientTest.php | 221 ++++++++++++++++++++++++++++++++++ tests/Util/TestCurlHandle.php | 33 ++++- 3 files changed, 259 insertions(+), 5 deletions(-) diff --git a/src/Psr18/Client.php b/src/Psr18/Client.php index 4cc0276..c1e4b17 100644 --- a/src/Psr18/Client.php +++ b/src/Psr18/Client.php @@ -227,10 +227,12 @@ protected function doSendRequest(RequestInterface $request, ClientOptions $optio $response = $headerParser->applyToResponse($response); if ($this->isRedirect($response)) { - try { - $fiber->throw(new RequestRedirectedException()); - } catch (Throwable $e) { - throw new RequestException($request, "Could not close request before redirect", previous: $e); + if (!$fiber->isTerminated()) { + try { + $fiber->throw(new RequestRedirectedException()); + } catch (Throwable $e) { + throw new RequestException($request, "Could not close request before redirect", previous: $e); + } } return $this->handleRedirect($request, $response, $options, $redirects); } diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 9e379d1..3581c13 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -2,6 +2,10 @@ namespace Tests; +use Aternos\CurlPsr\Exception\RequestException; +use Aternos\CurlPsr\Exception\RequestRedirectedException; +use Aternos\CurlPsr\Psr7\Stream\StringStream; +use Exception; use PHPUnit\Framework\Attributes\TestWith; use Psr\Http\Client\NetworkExceptionInterface; use Psr\Http\Client\RequestExceptionInterface; @@ -241,6 +245,7 @@ public function testProgressCallbackHasTotalSizeIfKnown(): void $uTotal = $uploadTotal; } }); + $this->assertIsCallable($this->client->getProgressCallback()); $request = $this->requestFactory->createRequest("POST", "https://example.com") ->withBody($requestBody); @@ -367,6 +372,7 @@ public function testCustomCurlOption(): void { $this->client->setCurlOption(CURLOPT_BUFFERSIZE, 1024); $this->assertEquals(1024, $this->client->getCurlOption(CURLOPT_BUFFERSIZE)); + $this->assertCount(1, $this->client->getCurlOptions()); $request = $this->requestFactory->createRequest("GET", "https://example.com"); $this->client->sendRequest($request); @@ -415,4 +421,219 @@ public function testDefaultHeaderOverwrittenByRequestHeader(): void $this->assertContains("X-Test: Request", $this->curlHandle->getOption(CURLOPT_HTTPHEADER)); $this->assertNotContains("X-Test: Test", $this->curlHandle->getOption(CURLOPT_HTTPHEADER)); } + + public function testRedirect(): void + { + $body1 = $this->streamFactory->createStream(); + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]) + ->setRequestBodySink($body1); + + $body2 = $this->streamFactory->createStream(); + $target = $this->curlHandleFactory->nextTestHandle() + ->setRequestBodySink($body2); + + $request = $this->requestFactory->createRequest("POST", "https://example.com") + ->withBody($this->streamFactory->createStream("test1234")); + $this->client->sendRequest($request); + + $this->assertEquals("POST", $target->getOption(CURLOPT_CUSTOMREQUEST)); + $this->assertEquals("https://example.com/redirect", (string)$target->getOption(CURLOPT_URL)); + $this->assertEquals("test1234", (string) $body1); + $this->assertEquals("test1234", (string) $body2); + } + + public function testThrowOnRedirectIfBodyIsNotSeekable(): void + { + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]); + + $this->curlHandleFactory->nextTestHandle(); + $request = $this->requestFactory->createRequest("POST", "https://example.com") + ->withBody(new StringStream("test1234", false)); + + $this->expectException(RequestException::class); + $this->expectExceptionMessage("Could not rewind body for redirect"); + $this->client->sendRequest($request); + } + + public function testThrowOnMultipleLocationHeaders(): void + { + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect", + "Location: https://example.com/redirect1" + ]); + + $this->curlHandleFactory->nextTestHandle(); + $request = $this->requestFactory->createRequest("POST", "https://example.com"); + + $this->expectException(RequestException::class); + $this->expectExceptionMessage("Multiple location headers in redirect"); + $this->client->sendRequest($request); + } + + public function testRedirectLimit(): void + { + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect1" + ]); + + $this->curlHandleFactory->nextTestHandle() + ->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect2" + ]); + + $this->curlHandleFactory->nextTestHandle() + ->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect3" + ]); + + $request = $this->requestFactory->createRequest("POST", "https://example.com"); + + $this->client->setMaxRedirects(2); + + $this->expectException(RequestException::class); + $this->expectExceptionMessage("Redirect limit of 2 reached"); + $this->client->sendRequest($request); + } + + public function testRedirectToGetOn303(): void + { + $body1 = $this->streamFactory->createStream(); + $this->curlHandle->setInfo(["http_code" => 303]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]) + ->setRequestBodySink($body1); + + $body2 = $this->streamFactory->createStream(); + $target = $this->curlHandleFactory->nextTestHandle() + ->setRequestBodySink($body2); + + $request = $this->requestFactory->createRequest("POST", "https://example.com") + ->withBody($this->streamFactory->createStream("test1234")); + $this->client->sendRequest($request); + + $this->assertEquals("GET", $target->getOption(CURLOPT_CUSTOMREQUEST)); + $this->assertEquals("https://example.com/redirect", (string)$target->getOption(CURLOPT_URL)); + $this->assertEquals("test1234", (string) $body1); + $this->assertEquals("", (string) $body2); + } + + public function testRedirectToGetIfConfigured(): void + { + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]); + + $target = $this->curlHandleFactory->nextTestHandle(); + + $request = $this->requestFactory->createRequest("POST", "https://example.com") + ->withBody($this->streamFactory->createStream("test1234")); + $this->client->setRedirectToGetStatusCodes([302]); + $this->assertEquals([302], $this->client->getRedirectToGetStatusCodes()); + $this->client->sendRequest($request); + + $this->assertEquals("GET", $target->getOption(CURLOPT_CUSTOMREQUEST)); + $this->assertEquals("https://example.com/redirect", (string)$target->getOption(CURLOPT_URL)); + } + + public function testThrowOnRedirectWithoutLocation(): void + { + $this->curlHandle->setInfo(["http_code" => 303]); + + $this->expectException(RequestException::class); + $this->expectExceptionMessage("Redirect without location header"); + $request = $this->requestFactory->createRequest("GET", "https://example.com"); + $this->client->sendRequest($request); + } + + public function testThrowOnRedirectToInvalidTarget(): void + { + $this->curlHandle->setInfo(["http_code" => 303]) + ->setResponseHeaders([ + "Location: http://" + ]); + + $this->expectException(RequestException::class); + $this->expectExceptionMessage("Invalid location header in redirect"); + $request = $this->requestFactory->createRequest("GET", "https://example.com"); + $this->client->sendRequest($request); + } + + public function testRedirectOn300IfLocationIsSet(): void + { + $this->curlHandle->setInfo(["http_code" => 300]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]); + + $target = $this->curlHandleFactory->nextTestHandle(); + + $request = $this->requestFactory->createRequest("GET", "https://example.com"); + $this->client->sendRequest($request); + + $this->assertEquals("GET", $target->getOption(CURLOPT_CUSTOMREQUEST)); + $this->assertEquals("https://example.com/redirect", (string)$target->getOption(CURLOPT_URL)); + } + + public function testDoNotRedirectOn300IfLocationIsMissing(): void + { + $this->curlHandle->setInfo(["http_code" => 300]) + ->setResponseHeaders([ + 'Link: ; rel="alternate"' + ]); + + $target = $this->curlHandleFactory->nextTestHandle(); + + $request = $this->requestFactory->createRequest("GET", "https://example.com"); + $this->client->sendRequest($request); + + $this->assertNull($target->getOption(CURLOPT_CUSTOMREQUEST)); + } + + public function testRedirectCancelsInitialRequest(): void + { + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]) + ->setResponseBody($this->streamFactory->createStream(random_bytes(1024))); + + $this->curlHandleFactory->nextTestHandle(); + + $request = $this->requestFactory->createRequest("POST", "https://example.com"); + $this->client->sendRequest($request); + + $this->assertInstanceOf(RequestRedirectedException::class, $this->curlHandle->getExecError()); + } + + public function testThrowOnErrorWhileStoppingInitialRequest(): void + { + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]) + ->setResponseBody($this->streamFactory->createStream(random_bytes(1024))) + ->setOnWriteError(function () { + throw new Exception("Test"); + }); + + $this->curlHandleFactory->nextTestHandle(); + + $request = $this->requestFactory->createRequest("POST", "https://example.com"); + + $this->expectException(RequestException::class); + $this->expectExceptionMessage("Could not close request before redirect"); + $this->client->sendRequest($request); + } } diff --git a/tests/Util/TestCurlHandle.php b/tests/Util/TestCurlHandle.php index 68245d1..5ab4072 100644 --- a/tests/Util/TestCurlHandle.php +++ b/tests/Util/TestCurlHandle.php @@ -32,6 +32,8 @@ class TestCurlHandle implements CurlHandleInterface protected ?Closure $onAfterRead = null; protected ?Closure $onBeforeWrite = null; protected ?Closure $onAfterWrite = null; + protected ?Closure $onBeforeRequest = null; + protected ?Closure $onWriteError = null; protected ?Throwable $execError = null; /** @@ -48,6 +50,10 @@ public function setopt(int $option, mixed $value): bool */ public function exec(): string|bool { + $beforeRequest = $this->onBeforeRequest; + if ($beforeRequest !== null) { + $beforeRequest($this); + } try { return $this->doExec(); } catch (Throwable $e) { @@ -92,7 +98,12 @@ public function doExec(): string|bool if (isset($this->options[CURLOPT_WRITEFUNCTION]) && $this->responseBody !== null) { while (!$this->responseBody->eof()) { $this->onBeforeWrite?->call($this); - $this->options[CURLOPT_WRITEFUNCTION](null, $this->responseBody->read($this->responseChunkSize)); + try { + $this->options[CURLOPT_WRITEFUNCTION](null, $this->responseBody->read($this->responseChunkSize)); + } catch (Throwable $e) { + $this->onWriteError?->call($this, $e); + throw $e; + } $this->progressUpdate( $this->responseBody->getSize() ?? $this->getResponseHeader("Content-Length") ?? 0, $this->responseBody->tell(), 0, 0); @@ -444,4 +455,24 @@ public function getExecError(): ?Throwable return $this->execError; } + /** + * @param Closure|null $onBeforeRequest + * @return $this + */ + public function setOnBeforeRequest(?Closure $onBeforeRequest): static + { + $this->onBeforeRequest = $onBeforeRequest; + return $this; + } + + /** + * @param Closure|null $onWriteError + * @return $this + */ + public function setOnWriteError(?Closure $onWriteError): static + { + $this->onWriteError = $onWriteError; + return $this; + } + } From 70d8834f0c0fe2b4211e812538295427a2e15e69 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Sun, 9 Mar 2025 21:16:59 +0100 Subject: [PATCH 09/11] update readme --- README.md | 34 ++++++++++++++++++++++++++++++---- src/Psr18/Client.php | 20 +++++++++++++++++++- src/Psr18/ClientOptions.php | 1 + tests/HttpClientTest.php | 29 +++++++++++++++++------------ 4 files changed, 67 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index d8a6cb7..ea1ea05 100644 --- a/README.md +++ b/README.md @@ -21,11 +21,10 @@ implementations for [PSR-17 (HTTP Factories)](https://www.php-fig.org/psr/psr-17 $client = new \Aternos\CurlPsr\Psr18\Client(); ``` -When creating a client, you can optionally provide a PSR-17 `ResponseFactoryInterface` instance. By default, -the client will use the `Aternos\CurlPsr\Psr17\Psr17Factory` class included in this library. +When creating a client, you can optionally provide PSR-17 `ResponseFactoryInterface` and `UriFactoryInterface` instances. +By default, the client will use the `Aternos\CurlPsr\Psr17\Psr17Factory` class included in this library. -Additionally, you can pass an optional `CurlHandleFactoryInterface` instance as the second argument, -which is mainly used for testing purposes. +Additionally, you can pass an optional `UriResolverInterface` instance, which is used to resolve redirect targets. ### Configuring the client @@ -52,7 +51,34 @@ $client->setProgressCallback(function ( }); ``` +#### Custom cURL options + +You can set custom cURL options using the `setCurlOption` method. Note that some options cannot be set, since they are +used internally by the client. + +#### Redirects + +The client will follow redirects by default. You can set the maximum number of redirects to follow using the +`setMaxRedirects` method. It is also possible to disable redirects using `setFollowRedirects`. The difference between +setting the maximum number of redirects to 0 and disabling redirects is that the former will throw an exception if a +redirect is received, while the latter will simply return the redirect response. + +Only when status `303 See Other` is received, the client will automatically change the request method to `GET` and +remove the request body. Historically, this behavior was also sometimes present for `301` and `302`, so it is possible +to enable it for other status codes using the `setRedirectToGetStatusCodes` method. + +Status `300 Multiple Choices` will only be treated as a redirect if the `Location` header is present. +Otherwise, the response will be returned as is. + +To manage how redirect targets are resolved, or limit what locations the client can be redirected to, +you can pass an instance of `UriResolverInterface` to the client constructor. + +When a redirect response is received that does not prompt the client to change the request method to `GET` +and the body stream cannot be rewound, an exception is thrown. This is because the client cannot resend the request +with the same body stream. + #### Progress callback + The progress callback function works the same way as the `CURLOPT_PROGRESSFUNCTION` in cURL, except that it receives the PSR-7 request object instead of a cURL handle as the first argument. Please note that the request object passed to the callback is not necessarily same instance that was diff --git a/src/Psr18/Client.php b/src/Psr18/Client.php index c1e4b17..895eb4a 100644 --- a/src/Psr18/Client.php +++ b/src/Psr18/Client.php @@ -226,7 +226,7 @@ protected function doSendRequest(RequestInterface $request, ClientOptions $optio $response = $headerParser->applyToResponse($response); - if ($this->isRedirect($response)) { + if ($options->followRedirects && $this->isRedirect($response)) { if (!$fiber->isTerminated()) { try { $fiber->throw(new RequestRedirectedException()); @@ -553,4 +553,22 @@ public function getRedirectToGetStatusCodes(): array { return $this->options->redirectToGetStatusCodes; } + + /** + * @param bool $followRedirects + * @return $this + */ + public function setFollowRedirects(bool $followRedirects): static + { + $this->options->followRedirects = $followRedirects; + return $this; + } + + /** + * @return bool + */ + public function getFollowRedirects(): bool + { + return $this->options->followRedirects; + } } diff --git a/src/Psr18/ClientOptions.php b/src/Psr18/ClientOptions.php index 2605c3f..d5d1158 100644 --- a/src/Psr18/ClientOptions.php +++ b/src/Psr18/ClientOptions.php @@ -7,6 +7,7 @@ class ClientOptions { public int $timeout = 0; + public bool $followRedirects = true; public int $maxRedirects = 10; public string $cookieFile = ""; public ?Closure $progressCallback = null; diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 3581c13..b0539bf 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -345,18 +345,6 @@ public function testMaxRedirects(): void $this->assertFalse($this->curlHandle->getOption(CURLOPT_FOLLOWLOCATION)); } - public function testDisableRedirects(): void - { - $this->client->setMaxRedirects(0); - $this->assertEquals(0, $this->client->getMaxRedirects()); - - $request = $this->requestFactory->createRequest("GET", "https://example.com"); - $this->client->sendRequest($request); - - $this->assertFalse($this->curlHandle->getOption(CURLOPT_FOLLOWLOCATION)); - $this->assertEquals(0, $this->curlHandle->getOption(CURLOPT_MAXREDIRS)); - } - public function testCookieFile(): void { $this->client->setCookieFile("cookie.txt"); @@ -445,6 +433,23 @@ public function testRedirect(): void $this->assertEquals("test1234", (string) $body2); } + public function testDisableRedirects(): void + { + $this->curlHandle->setInfo(["http_code" => 302]) + ->setResponseHeaders([ + "Location: https://example.com/redirect" + ]); + + $request = $this->requestFactory->createRequest("GET", "https://example.com"); + $this->client->setFollowRedirects(false); + $this->assertEquals(false, $this->client->getFollowRedirects()); + + $response = $this->client->sendRequest($request); + + $this->assertEquals(302, $response->getStatusCode()); + $this->assertEquals("https://example.com/redirect", $response->getHeaderLine("Location")); + } + public function testThrowOnRedirectIfBodyIsNotSeekable(): void { $this->curlHandle->setInfo(["http_code" => 302]) From b867a2a0bbf29c46f1448f783ebf7270377219a8 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Mon, 10 Mar 2025 12:55:35 +0100 Subject: [PATCH 10/11] also remove content-type and content-encoding when redirecting to GET --- src/Psr18/Client.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Psr18/Client.php b/src/Psr18/Client.php index 895eb4a..8d5108f 100644 --- a/src/Psr18/Client.php +++ b/src/Psr18/Client.php @@ -303,7 +303,9 @@ protected function handleRedirect(RequestInterface $request, ResponseInterface $ if (in_array($response->getStatusCode(), $options->redirectToGetStatusCodes)) { $request = $request->withMethod("GET") ->withBody(new EmptyStream()) - ->withoutHeader("Content-Length"); + ->withoutHeader("Content-Length") + ->withoutHeader("Content-Type") + ->withoutHeader("Content-Encoding"); return $this->doSendRequest($request, $options, $redirects + 1); } From 93337634209759d184ac4653bfa5c4d5d9584491 Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Tue, 28 Oct 2025 17:09:29 +0100 Subject: [PATCH 11/11] set CURLOPT_NOBODY for HEAD requests --- src/Psr18/Client.php | 3 +++ tests/HttpClientTest.php | 1 + 2 files changed, 4 insertions(+) diff --git a/src/Psr18/Client.php b/src/Psr18/Client.php index 8d5108f..1f2ee1d 100644 --- a/src/Psr18/Client.php +++ b/src/Psr18/Client.php @@ -132,6 +132,9 @@ protected function initRequest(RequestInterface $request, ResponseHeaderParser $ $ch->setopt(CURLOPT_HEADERFUNCTION, $headerParser->getClosure()); $ch->setopt(CURLOPT_CUSTOMREQUEST, $request->getMethod()); + if ($request->getMethod() === "HEAD") { + $ch->setopt(CURLOPT_NOBODY, true); + } $this->setHeaders($request, $ch); diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index b0539bf..0fe6643 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -86,6 +86,7 @@ public function testHead(): void $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals("TEST", $response->getReasonPhrase()); $this->assertEquals("HEAD", $this->curlHandle->getOption(CURLOPT_CUSTOMREQUEST)); + $this->assertTrue($this->curlHandle->getOption(CURLOPT_NOBODY)); } public function testCustom(): void