Skip to content

Commit 1b1ebd4

Browse files
pprkutSMillerDev
pprkut
authored and
SMillerDev
committed
Requests: Handle requests.failed hook correctly in case of redirects
1 parent cbc75b7 commit 1b1ebd4

5 files changed

Lines changed: 183 additions & 6 deletions

File tree

‎src/Exception.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ class Exception extends PHPException {
2929
*/
3030
protected $data;
3131

32+
/**
33+
* Whether the exception was already passed to the requests.failed hook or not
34+
*
35+
* @var boolean
36+
*/
37+
public $failed_hook_handled = FALSE;
38+
3239
/**
3340
* Create a new exception
3441
*

‎src/Exception/InvalidArgument.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@
1212
*/
1313
final class InvalidArgument extends InvalidArgumentException {
1414

15+
/**
16+
* Whether the exception was already passed to the requests.failed hook or not
17+
*
18+
* @var boolean
19+
*/
20+
public $failed_hook_handled = FALSE;
21+
1522
/**
1623
* Create a new invalid argument exception with a standardized text.
1724
*

‎src/Requests.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,10 @@ public static function request($url, $headers = [], $data = [], $type = self::GE
474474

475475
$parsed_response = self::parse_response($response, $url, $headers, $data, $options);
476476
} catch (Exception|InvalidArgument $e) {
477-
$options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]);
477+
if ($e->failed_hook_handled === FALSE) {
478+
$options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]);
479+
$e->failed_hook_handled = TRUE;
480+
}
478481
throw $e;
479482
}
480483

‎tests/Fixtures/TransportRedirectMock.php‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
<?php
2+
3+
namespace WpOrg\Requests\Tests\Fixtures;
4+
5+
use WpOrg\Requests\Transport;
6+
7+
final class TransportRedirectMock implements Transport {
8+
public $code = 302;
9+
public $chunked = false;
10+
public $body = '';
11+
public $raw_headers = '';
12+
13+
private $redirected = [];
14+
15+
public $redirected_transport = NULL;
16+
17+
private static $messages = [
18+
100 => '100 Continue',
19+
101 => '101 Switching Protocols',
20+
200 => '200 OK',
21+
201 => '201 Created',
22+
202 => '202 Accepted',
23+
203 => '203 Non-Authoritative Information',
24+
204 => '204 No Content',
25+
205 => '205 Reset Content',
26+
206 => '206 Partial Content',
27+
300 => '300 Multiple Choices',
28+
301 => '301 Moved Permanently',
29+
302 => '302 Found',
30+
303 => '303 See Other',
31+
304 => '304 Not Modified',
32+
305 => '305 Use Proxy',
33+
306 => '306 (Unused)',
34+
307 => '307 Temporary Redirect',
35+
400 => '400 Bad Request',
36+
401 => '401 Unauthorized',
37+
402 => '402 Payment Required',
38+
403 => '403 Forbidden',
39+
404 => '404 Not Found',
40+
405 => '405 Method Not Allowed',
41+
406 => '406 Not Acceptable',
42+
407 => '407 Proxy Authentication Required',
43+
408 => '408 Request Timeout',
44+
409 => '409 Conflict',
45+
410 => '410 Gone',
46+
411 => '411 Length Required',
47+
412 => '412 Precondition Failed',
48+
413 => '413 Request Entity Too Large',
49+
414 => '414 Request-URI Too Long',
50+
415 => '415 Unsupported Media Type',
51+
416 => '416 Requested Range Not Satisfiable',
52+
417 => '417 Expectation Failed',
53+
418 => '418 I\'m a teapot',
54+
428 => '428 Precondition Required',
55+
429 => '429 Too Many Requests',
56+
431 => '431 Request Header Fields Too Large',
57+
500 => '500 Internal Server Error',
58+
501 => '501 Not Implemented',
59+
502 => '502 Bad Gateway',
60+
503 => '503 Service Unavailable',
61+
504 => '504 Gateway Timeout',
62+
505 => '505 HTTP Version Not Supported',
63+
511 => '511 Network Authentication Required',
64+
];
65+
66+
public function request($url, $headers = [], $data = [], $options = []) {
67+
if (array_key_exists($url, $this->redirected))
68+
{
69+
return $this->redirected_transport->request($url, $headers, $data, $options);
70+
}
71+
72+
$redirect_url = "https://example.com/redirected?url=" . urlencode($url);
73+
74+
$status = isset(self::$messages[$this->code]) ? self::$messages[$this->code] : $this->code . ' unknown';
75+
$response = "HTTP/1.0 $status\r\n";
76+
$response .= "Content-Type: text/plain\r\n";
77+
if ($this->chunked) {
78+
$response .= "Transfer-Encoding: chunked\r\n";
79+
}
80+
$response .= "Location: $redirect_url\r\n";
81+
$response .= $this->raw_headers;
82+
$response .= "Connection: close\r\n\r\n";
83+
$response .= $this->body;
84+
85+
$this->redirected[$url] = TRUE;
86+
$this->redirected[$redirect_url] = TRUE;
87+
88+
return $response;
89+
}
90+
91+
public function request_multiple($requests, $options) {
92+
$responses = [];
93+
foreach ($requests as $id => $request) {
94+
$handler = new self();
95+
$handler->code = $request['options']['mock.code'];
96+
$handler->chunked = $request['options']['mock.chunked'];
97+
$handler->body = $request['options']['mock.body'];
98+
$handler->raw_headers = $request['options']['mock.raw_headers'];
99+
$responses[$id] = $handler->request($request['url'], $request['headers'], $request['data'], $request['options']);
100+
101+
if (!empty($options['mock.parse'])) {
102+
$request['options']['hooks']->dispatch('transport.internal.parse_response', [&$responses[$id], $request]);
103+
$request['options']['hooks']->dispatch('multiple.request.complete', [&$responses[$id], $id]);
104+
}
105+
}
106+
107+
return $responses;
108+
}
109+
110+
public static function test($capabilities = []) {
111+
return true;
112+
}
113+
}

‎tests/Requests/RequestsTest.php‎

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use WpOrg\Requests\Tests\Fixtures\TransportMock;
1717
use WpOrg\Requests\Tests\TestCase;
1818
use WpOrg\Requests\Tests\TypeProviderHelper;
19+
use WpOrg\Requests\Tests\Fixtures\TransportRedirectMock;
1920

2021
final class RequestsTest extends TestCase {
2122

@@ -159,7 +160,7 @@ public function testDefaultTransport() {
159160

160161
public function testTransportFailedTriggersRequestsFailedCallback() {
161162
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
162-
$mock->expects($this->atLeastOnce())->method('failed');
163+
$mock->expects($this->once())->method('failed');
163164
$hooks = new Hooks();
164165
$hooks->register('requests.failed', [$mock, 'failed']);
165166

@@ -177,7 +178,7 @@ public function testTransportFailedTriggersRequestsFailedCallback() {
177178

178179
public function testTransportInvalidArgumentTriggersRequestsFailedCallback() {
179180
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
180-
$mock->expects($this->atLeastOnce())->method('failed');
181+
$mock->expects($this->once())->method('failed');
181182
$hooks = new Hooks();
182183
$hooks->register('requests.failed', [$mock, 'failed']);
183184

@@ -301,7 +302,7 @@ public function testInvalidProtocolVersion() {
301302
*/
302303
public function testInvalidProtocolVersionTriggersRequestsFailedCallback() {
303304
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
304-
$mock->expects($this->atLeastOnce())->method('failed');
305+
$mock->expects($this->once())->method('failed');
305306
$hooks = new Hooks();
306307
$hooks->register('requests.failed', [$mock, 'failed']);
307308

@@ -339,7 +340,7 @@ public function testSingleCRLFSeparator() {
339340
*/
340341
public function testSingleCRLFSeparatorTriggersRequestsFailedCallback() {
341342
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
342-
$mock->expects($this->atLeastOnce())->method('failed');
343+
$mock->expects($this->once())->method('failed');
343344
$hooks = new Hooks();
344345
$hooks->register('requests.failed', [$mock, 'failed']);
345346

@@ -371,7 +372,7 @@ public function testInvalidStatus() {
371372

372373
public function testInvalidStatusTriggersRequestsFailedCallback() {
373374
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
374-
$mock->expects($this->atLeastOnce())->method('failed');
375+
$mock->expects($this->once())->method('failed');
375376
$hooks = new Hooks();
376377
$hooks->register('requests.failed', [$mock, 'failed']);
377378

@@ -400,6 +401,52 @@ public function test30xWithoutLocation() {
400401
$this->assertSame(0, $response->redirects);
401402
}
402403

404+
public function testRedirectToExceptionTriggersRequestsFailedCallbackOnce() {
405+
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
406+
$mock->expects($this->once())->method('failed');
407+
$hooks = new Hooks();
408+
$hooks->register('requests.failed', [$mock, 'failed']);
409+
410+
$transport = new TransportRedirectMock();
411+
$transport->redirected_transport = new TransportFailedMock();
412+
413+
$options = [
414+
'hooks' => $hooks,
415+
'transport' => $transport,
416+
];
417+
418+
$this->expectException(Exception::class);
419+
$this->expectExceptionMessage('Transport failed!');
420+
421+
$response = Requests::get('http://example.com/', [], $options);
422+
423+
$this->assertSame(302, $response->status_code);
424+
$this->assertSame(1, $response->redirects);
425+
}
426+
427+
public function testRedirectToInvalidArgumentTriggersRequestsFailedCallbackOnce() {
428+
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
429+
$mock->expects($this->once())->method('failed');
430+
$hooks = new Hooks();
431+
$hooks->register('requests.failed', [$mock, 'failed']);
432+
433+
$transport = new TransportRedirectMock();
434+
$transport->redirected_transport = new TransportInvalidArgumentMock();
435+
436+
$options = [
437+
'hooks' => $hooks,
438+
'transport' => $transport,
439+
];
440+
441+
$this->expectException(InvalidArgument::class);
442+
$this->expectExceptionMessage('Argument #1 ($url) must be of type string|Stringable');
443+
444+
$response = Requests::get('http://example.com/', [], $options);
445+
446+
$this->assertSame(302, $response->status_code);
447+
$this->assertSame(1, $response->redirects);
448+
}
449+
403450
public function testTimeoutException() {
404451
$options = ['timeout' => 0.5];
405452
$this->expectException(Exception::class);

0 commit comments

Comments
 (0)