Skip to content

Commit 2eea4a5

Browse files
GrotaxGrotax
committed
Implement HTTP redirect handling with loop protection and relative URL support; add tests for response behavior
1 parent c51de5f commit 2eea4a5

File tree

3 files changed

+335
-4
lines changed

3 files changed

+335
-4
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
- HTTP redirect handling (301, 302, 303, 307, 308) with loop protection and relative URL support (#29)
12+
1013
### Changed
1114

1215
### Fixed

‎src/FeedIo/Adapter/Http/Client.php‎

Lines changed: 106 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
class Client implements ClientInterface
1717
{
18+
private const MAX_REDIRECTS = 10;
19+
1820
public function __construct(private readonly PsrClientInterface $client)
1921
{
2022
}
@@ -41,11 +43,23 @@ public function getResponse(string $url, ?DateTime $modifiedSince = null): Respo
4143
* @param string $method
4244
* @param string $url
4345
* @param DateTime|null $modifiedSince
46+
* @param int $redirectCount
4447
* @return ResponseInterface
4548
* @throws ClientExceptionInterface
4649
*/
47-
protected function request(string $method, string $url, ?DateTime $modifiedSince = null): ResponseInterface
48-
{
50+
protected function request(
51+
string $method,
52+
string $url,
53+
?DateTime $modifiedSince = null,
54+
int $redirectCount = 0
55+
): ResponseInterface {
56+
if ($redirectCount >= self::MAX_REDIRECTS) {
57+
throw new ServerErrorException(
58+
new \Nyholm\Psr7\Response(508, [], 'Too many redirects'),
59+
0
60+
);
61+
}
62+
4963
$headers = [];
5064

5165
if ($modifiedSince) {
@@ -64,14 +78,102 @@ protected function request(string $method, string $url, ?DateTime $modifiedSince
6478
return new Response($psrResponse, $duration);
6579
case 301:
6680
case 302:
67-
case 303:
6881
case 307:
6982
case 308:
70-
return $this->request($method, $psrResponse->getHeaderLine('Location'), $modifiedSince);
83+
return $this->handleRedirect(
84+
$method,
85+
$url,
86+
$psrResponse,
87+
$modifiedSince,
88+
$redirectCount,
89+
$duration
90+
);
91+
case 303:
92+
// 303 See Other always requires GET
93+
return $this->handleRedirect(
94+
'GET',
95+
$url,
96+
$psrResponse,
97+
$modifiedSince,
98+
$redirectCount,
99+
$duration
100+
);
71101
case 404:
72102
throw new NotFoundException('not found', $duration);
73103
default:
74104
throw new ServerErrorException($psrResponse, $duration);
75105
}
76106
}
107+
108+
/**
109+
* Handle HTTP redirect responses
110+
*
111+
* @param string $method
112+
* @param string $currentUrl
113+
* @param \Psr\Http\Message\ResponseInterface $psrResponse
114+
* @param DateTime|null $modifiedSince
115+
* @param int $redirectCount
116+
* @param float $duration
117+
* @return ResponseInterface
118+
* @throws ClientExceptionInterface
119+
*/
120+
protected function handleRedirect(
121+
string $method,
122+
string $currentUrl,
123+
\Psr\Http\Message\ResponseInterface $psrResponse,
124+
?DateTime $modifiedSince,
125+
int $redirectCount,
126+
float $duration
127+
): ResponseInterface {
128+
$location = $psrResponse->getHeaderLine('Location');
129+
130+
if (empty($location)) {
131+
throw new ServerErrorException($psrResponse, $duration);
132+
}
133+
134+
// Handle relative URLs
135+
$redirectUrl = $this->resolveRedirectUrl($currentUrl, $location);
136+
137+
return $this->request($method, $redirectUrl, $modifiedSince, $redirectCount + 1);
138+
}
139+
140+
/**
141+
* Resolve potentially relative redirect URL to absolute URL
142+
*
143+
* @param string $currentUrl
144+
* @param string $location
145+
* @return string
146+
*/
147+
protected function resolveRedirectUrl(string $currentUrl, string $location): string
148+
{
149+
// If location is already absolute, return it
150+
if (preg_match('/^https?:\/\//i', $location)) {
151+
return $location;
152+
}
153+
154+
// Parse current URL
155+
$parts = parse_url($currentUrl);
156+
if (!$parts) {
157+
return $location;
158+
}
159+
160+
$scheme = $parts['scheme'] ?? 'http';
161+
$host = $parts['host'] ?? '';
162+
163+
// Handle absolute path (starts with /)
164+
if (str_starts_with($location, '/')) {
165+
$port = isset($parts['port']) ? ':' . $parts['port'] : '';
166+
return "{$scheme}://{$host}{$port}{$location}";
167+
}
168+
169+
// Handle relative path
170+
$path = $parts['path'] ?? '/';
171+
$basePath = dirname($path);
172+
if ($basePath === '.') {
173+
$basePath = '/';
174+
}
175+
176+
$port = isset($parts['port']) ? ':' . $parts['port'] : '';
177+
return "{$scheme}://{$host}{$port}{$basePath}/{$location}";
178+
}
77179
}

‎tests/FeedIo/Adapter/Http/ClientTest.php‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace FeedIo\Adapter\Http;
6+
7+
use DateTime;
8+
use FeedIo\Adapter\NotFoundException;
9+
use FeedIo\Adapter\ServerErrorException;
10+
use Nyholm\Psr7\Response as PsrResponse;
11+
use PHPUnit\Framework\TestCase;
12+
use Psr\Http\Client\ClientInterface as PsrClientInterface;
13+
use Psr\Http\Message\RequestInterface;
14+
15+
class ClientTest extends TestCase
16+
{
17+
private PsrClientInterface $psrClient;
18+
private Client $client;
19+
20+
protected function setUp(): void
21+
{
22+
$this->psrClient = $this->createMock(PsrClientInterface::class);
23+
$this->client = new Client($this->psrClient);
24+
}
25+
26+
public function testGetResponseWithSuccess(): void
27+
{
28+
$psrResponse = new PsrResponse(200, [], 'feed content');
29+
30+
$this->psrClient
31+
->expects($this->once())
32+
->method('sendRequest')
33+
->willReturn($psrResponse);
34+
35+
$response = $this->client->getResponse('https://example.com/feed.xml');
36+
37+
$this->assertEquals(200, $response->getStatusCode());
38+
$this->assertEquals('feed content', $response->getBody());
39+
}
40+
41+
public function testGetResponseWith304NotModified(): void
42+
{
43+
$modifiedSince = new DateTime('2025-01-01');
44+
45+
// HEAD request returns 304
46+
$headResponse = new PsrResponse(304);
47+
48+
$this->psrClient
49+
->expects($this->once())
50+
->method('sendRequest')
51+
->with($this->callback(function (RequestInterface $request) use ($modifiedSince) {
52+
return $request->getMethod() === 'HEAD'
53+
&& $request->hasHeader('If-Modified-Since')
54+
&& $request->getHeaderLine('If-Modified-Since') === $modifiedSince->format(DateTime::RFC2822);
55+
}))
56+
->willReturn($headResponse);
57+
58+
$response = $this->client->getResponse('https://example.com/feed.xml', $modifiedSince);
59+
60+
$this->assertEquals(304, $response->getStatusCode());
61+
}
62+
63+
public function testGetResponseWithModifiedSinceButFeedChanged(): void
64+
{
65+
$modifiedSince = new DateTime('2025-01-01');
66+
67+
// HEAD request returns 200 (modified)
68+
$headResponse = new PsrResponse(200);
69+
// GET request also returns 200 with content
70+
$getResponse = new PsrResponse(200, [], 'new feed content');
71+
72+
$this->psrClient
73+
->expects($this->exactly(2))
74+
->method('sendRequest')
75+
->willReturnOnConsecutiveCalls($headResponse, $getResponse);
76+
77+
$response = $this->client->getResponse('https://example.com/feed.xml', $modifiedSince);
78+
79+
$this->assertEquals(200, $response->getStatusCode());
80+
$this->assertEquals('new feed content', $response->getBody());
81+
}
82+
83+
public function testGetResponseThrowsNotFoundOn404(): void
84+
{
85+
$psrResponse = new PsrResponse(404);
86+
87+
$this->psrClient
88+
->expects($this->once())
89+
->method('sendRequest')
90+
->willReturn($psrResponse);
91+
92+
$this->expectException(NotFoundException::class);
93+
$this->expectExceptionMessage('not found');
94+
95+
$this->client->getResponse('https://example.com/feed.xml');
96+
}
97+
98+
public function testGetResponseThrowsServerErrorOnServerError(): void
99+
{
100+
$psrResponse = new PsrResponse(500);
101+
102+
$this->psrClient
103+
->expects($this->once())
104+
->method('sendRequest')
105+
->willReturn($psrResponse);
106+
107+
$this->expectException(ServerErrorException::class);
108+
109+
$this->client->getResponse('https://example.com/feed.xml');
110+
}
111+
112+
/**
113+
* @dataProvider redirectStatusCodeProvider
114+
*/
115+
public function testGetResponseFollowsRedirects(int $statusCode): void
116+
{
117+
$redirectResponse = new PsrResponse($statusCode, ['Location' => 'https://example.com/new-feed.xml']);
118+
$finalResponse = new PsrResponse(200, [], 'redirected feed content');
119+
120+
$this->psrClient
121+
->expects($this->exactly(2))
122+
->method('sendRequest')
123+
->willReturnOnConsecutiveCalls($redirectResponse, $finalResponse);
124+
125+
$response = $this->client->getResponse('https://example.com/old-feed.xml');
126+
127+
$this->assertEquals(200, $response->getStatusCode());
128+
$this->assertEquals('redirected feed content', $response->getBody());
129+
}
130+
131+
public static function redirectStatusCodeProvider(): array
132+
{
133+
return [
134+
'301 Moved Permanently' => [301],
135+
'302 Found' => [302],
136+
'303 See Other' => [303],
137+
'307 Temporary Redirect' => [307],
138+
'308 Permanent Redirect' => [308],
139+
];
140+
}
141+
142+
public function testGetResponseFollowsMultipleRedirects(): void
143+
{
144+
$redirect1 = new PsrResponse(301, ['Location' => 'https://example.com/redirect2.xml']);
145+
$redirect2 = new PsrResponse(302, ['Location' => 'https://example.com/final.xml']);
146+
$finalResponse = new PsrResponse(200, [], 'final content');
147+
148+
$this->psrClient
149+
->expects($this->exactly(3))
150+
->method('sendRequest')
151+
->willReturnOnConsecutiveCalls($redirect1, $redirect2, $finalResponse);
152+
153+
$response = $this->client->getResponse('https://example.com/start.xml');
154+
155+
$this->assertEquals(200, $response->getStatusCode());
156+
$this->assertEquals('final content', $response->getBody());
157+
}
158+
159+
public function testGetResponsePreservesModifiedSinceThroughRedirects(): void
160+
{
161+
$modifiedSince = new DateTime('2025-01-01');
162+
163+
// HEAD request with If-Modified-Since returns redirect
164+
$headRedirect = new PsrResponse(301, ['Location' => 'https://example.com/new-location.xml']);
165+
// HEAD request to new location returns 200
166+
$headResponse = new PsrResponse(200);
167+
// GET request to new location
168+
$getResponse = new PsrResponse(200, [], 'content');
169+
170+
$this->psrClient
171+
->expects($this->exactly(3))
172+
->method('sendRequest')
173+
->willReturnOnConsecutiveCalls($headRedirect, $headResponse, $getResponse);
174+
175+
$response = $this->client->getResponse('https://example.com/old-location.xml', $modifiedSince);
176+
177+
$this->assertEquals(200, $response->getStatusCode());
178+
}
179+
180+
public function testGetResponseWithEmptyLocationHeaderThrowsException(): void
181+
{
182+
$redirectResponse = new PsrResponse(301, ['Location' => '']);
183+
184+
$this->psrClient
185+
->expects($this->once())
186+
->method('sendRequest')
187+
->willReturn($redirectResponse);
188+
189+
$this->expectException(ServerErrorException::class);
190+
191+
$this->client->getResponse('https://example.com/feed.xml');
192+
}
193+
194+
public function testResponseDurationIsTracked(): void
195+
{
196+
$psrResponse = new PsrResponse(200, [], 'content');
197+
198+
$this->psrClient
199+
->expects($this->once())
200+
->method('sendRequest')
201+
->willReturn($psrResponse);
202+
203+
$response = $this->client->getResponse('https://example.com/feed.xml');
204+
205+
$this->assertIsFloat($response->getDuration());
206+
$this->assertGreaterThanOrEqual(0, $response->getDuration());
207+
}
208+
209+
public function testResponseDurationIsTrackedOnError(): void
210+
{
211+
$psrResponse = new PsrResponse(404);
212+
213+
$this->psrClient
214+
->expects($this->once())
215+
->method('sendRequest')
216+
->willReturn($psrResponse);
217+
218+
try {
219+
$this->client->getResponse('https://example.com/feed.xml');
220+
$this->fail('Expected NotFoundException to be thrown');
221+
} catch (NotFoundException $e) {
222+
$this->assertIsFloat($e->getDuration());
223+
$this->assertGreaterThanOrEqual(0, $e->getDuration());
224+
}
225+
}
226+
}

0 commit comments

Comments
 (0)