Skip to content

Commit 29202ff

Browse files
pprkutpprkut
committed
Requests: Handle requests.failed hook correctly in case of redirects
1 parent 11faa0f commit 29202ff

File tree

5 files changed

+184
-6
lines changed

5 files changed

+184
-6
lines changed

‎src/Exception.php

+7
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

+7
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

+5-1
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,11 @@ 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+
{
479+
$options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]);
480+
$e->failed_hook_handled = TRUE;
481+
}
478482
throw $e;
479483
}
480484

‎tests/Fixtures/TransportRedirectMock.php

+113
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/RequestsTest.php

+52-5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use WpOrg\Requests\Tests\Fixtures\TransportFailedMock;
2121
use WpOrg\Requests\Tests\Fixtures\TransportInvalidArgumentMock;
2222
use WpOrg\Requests\Tests\Fixtures\TransportMock;
23+
use WpOrg\Requests\Tests\Fixtures\TransportRedirectMock;
2324

2425
final class RequestsTest extends TestCase {
2526

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

178179
public function testTransportFailedTriggersRequestsFailedCallback() {
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

@@ -195,7 +196,7 @@ public function testTransportFailedTriggersRequestsFailedCallback() {
195196

196197
public function testTransportInvalidArgumentTriggersRequestsFailedCallback() {
197198
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
198-
$mock->expects($this->atLeastOnce())->method('failed');
199+
$mock->expects($this->once())->method('failed');
199200
$hooks = new Hooks();
200201
$hooks->register('requests.failed', [$mock, 'failed']);
201202

@@ -319,7 +320,7 @@ public function testInvalidProtocolVersion() {
319320
*/
320321
public function testInvalidProtocolVersionTriggersRequestsFailedCallback() {
321322
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
322-
$mock->expects($this->atLeastOnce())->method('failed');
323+
$mock->expects($this->once())->method('failed');
323324
$hooks = new Hooks();
324325
$hooks->register('requests.failed', [$mock, 'failed']);
325326

@@ -357,7 +358,7 @@ public function testSingleCRLFSeparator() {
357358
*/
358359
public function testSingleCRLFSeparatorTriggersRequestsFailedCallback() {
359360
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
360-
$mock->expects($this->atLeastOnce())->method('failed');
361+
$mock->expects($this->once())->method('failed');
361362
$hooks = new Hooks();
362363
$hooks->register('requests.failed', [$mock, 'failed']);
363364

@@ -389,7 +390,7 @@ public function testInvalidStatus() {
389390

390391
public function testInvalidStatusTriggersRequestsFailedCallback() {
391392
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
392-
$mock->expects($this->atLeastOnce())->method('failed');
393+
$mock->expects($this->once())->method('failed');
393394
$hooks = new Hooks();
394395
$hooks->register('requests.failed', [$mock, 'failed']);
395396

@@ -418,6 +419,52 @@ public function test30xWithoutLocation() {
418419
$this->assertSame(0, $response->redirects);
419420
}
420421

422+
public function testRedirectToExceptionTriggersRequestsFailedCallbackOnce() {
423+
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
424+
$mock->expects($this->once())->method('failed');
425+
$hooks = new Hooks();
426+
$hooks->register('requests.failed', [$mock, 'failed']);
427+
428+
$transport = new TransportRedirectMock();
429+
$transport->redirected_transport = new TransportFailedMock();
430+
431+
$options = [
432+
'hooks' => $hooks,
433+
'transport' => $transport,
434+
];
435+
436+
$this->expectException(Exception::class);
437+
$this->expectExceptionMessage('Transport failed!');
438+
439+
$response = Requests::get('http://example.com/', [], $options);
440+
441+
$this->assertSame(302, $response->status_code);
442+
$this->assertSame(1, $response->redirects);
443+
}
444+
445+
public function testRedirectToInvalidArgumentTriggersRequestsFailedCallbackOnce() {
446+
$mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
447+
$mock->expects($this->once())->method('failed');
448+
$hooks = new Hooks();
449+
$hooks->register('requests.failed', [$mock, 'failed']);
450+
451+
$transport = new TransportRedirectMock();
452+
$transport->redirected_transport = new TransportInvalidArgumentMock();
453+
454+
$options = [
455+
'hooks' => $hooks,
456+
'transport' => $transport,
457+
];
458+
459+
$this->expectException(InvalidArgument::class);
460+
$this->expectExceptionMessage('Argument #1 ($url) must be of type string|Stringable');
461+
462+
$response = Requests::get('http://example.com/', [], $options);
463+
464+
$this->assertSame(302, $response->status_code);
465+
$this->assertSame(1, $response->redirects);
466+
}
467+
421468
public function testTimeoutException() {
422469
$options = ['timeout' => 0.5];
423470
$this->expectException(Exception::class);

0 commit comments

Comments
 (0)