From e170b996674922f48223e6c5baba26d9cc4b173d Mon Sep 17 00:00:00 2001 From: Kurt Thiemann Date: Sun, 9 Mar 2025 19:29:00 +0100 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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])