From 9c3eb484e40130e8650a72d3a60a0ca4573cbb5d Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Wed, 29 Sep 2021 11:24:55 +0200 Subject: [PATCH 1/9] Requests: Add "requests.failed" hook This allows to inspect or alter the exception that is thrown to the user in case of transport errors, or in case of response parsing problems. --- docs/hooks.md | 7 +++++++ src/Requests.php | 18 +++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/docs/hooks.md b/docs/hooks.md index 10f04745b..3718c9b6d 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -63,6 +63,13 @@ Available Hooks To use this hook, pass a callback via `$options['complete']` when calling `WpOrg\Requests\Requests\request_multiple()`. +* **`requests.failed`** + + Alter/Inspect transport or response parsing exception before it is returned to the user. + + Parameters: `WpOrg\Requests\Exception|WpOrg\Requests\Exception\InvalidArgument &$exception`, `string $url`, `array $headers`, `array|string $data`, + `string $type`, `array $options` + * **`curl.before_request`** Set cURL options before the transport sets any (note that Requests may diff --git a/src/Requests.php b/src/Requests.php index 9390800b6..8c7794dcb 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -465,11 +465,23 @@ public static function request($url, $headers = [], $data = [], $type = self::GE $transport = self::get_transport($capabilities); } - $response = $transport->request($url, $headers, $data, $options); + try { + $response = $transport->request($url, $headers, $data, $options); + + $options['hooks']->dispatch('requests.before_parse', [&$response, $url, $headers, $data, $type, $options]); - $options['hooks']->dispatch('requests.before_parse', [&$response, $url, $headers, $data, $type, $options]); + $parsed_response = self::parse_response($response, $url, $headers, $data, $options); + } + catch (Exception $e) { + $options['hooks']->dispatch('requests.failed', [$e, $url, $headers, $data, $type, $options]); + throw $e; + } + catch (InvalidArgument $e) { + $options['hooks']->dispatch('requests.failed', [$e, $url, $headers, $data, $type, $options]); + throw $e; + } - return self::parse_response($response, $url, $headers, $data, $options); + return $parsed_response; } /** From 2b014043bc8a63c5893a3445ae35e97222b4569c Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Fri, 17 Dec 2021 20:20:19 +0100 Subject: [PATCH 2/9] Requests: Add tests for requests.failed hook --- tests/Fixtures/TransportFailedMock.php | 18 +++ .../Fixtures/TransportInvalidArgumentMock.php | 18 +++ tests/Requests/RequestsTest.php | 105 ++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 tests/Fixtures/TransportFailedMock.php create mode 100644 tests/Fixtures/TransportInvalidArgumentMock.php diff --git a/tests/Fixtures/TransportFailedMock.php b/tests/Fixtures/TransportFailedMock.php new file mode 100644 index 000000000..ec22c557a --- /dev/null +++ b/tests/Fixtures/TransportFailedMock.php @@ -0,0 +1,18 @@ +assertSame(200, $request->status_code); } + public function testTransportFailedTriggersRequestsFailedCallback() { + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock->expects($this->atLeastOnce())->method('failed'); + $hooks = new Hooks(); + $hooks->register('requests.failed', [$mock, 'failed']); + + $transport = new TransportFailedMock(); + + $options = [ + 'hooks' => $hooks, + 'transport' => $transport, + ]; + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Transport failed!'); + Requests::get('http://example.com/', [], $options); + } + + public function testTransportInvalidArgumentTriggersRequestsFailedCallback() { + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock->expects($this->atLeastOnce())->method('failed'); + $hooks = new Hooks(); + $hooks->register('requests.failed', [$mock, 'failed']); + + $transport = new TransportInvalidArgumentMock(); + + $options = [ + 'hooks' => $hooks, + 'transport' => $transport, + ]; + + $this->expectException(InvalidArgument::class); + $this->expectExceptionMessage('Argument #1 ($url) must be of type string|Stringable'); + Requests::get('http://example.com/', [], $options); + } + /** * Standard response header parsing */ @@ -252,6 +291,31 @@ public function testInvalidProtocolVersion() { Requests::get('http://example.com/', [], $options); } + /** + * Check that invalid protocols are not accepted + * + * We do not support HTTP/0.9. If this is really an issue for you, file a + * new issue, and update your server/proxy to support a proper protocol. + */ + public function testInvalidProtocolVersionTriggersRequestsFailedCallback() { + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock->expects($this->atLeastOnce())->method('failed'); + $hooks = new Hooks(); + $hooks->register('requests.failed', [$mock, 'failed']); + + $transport = new RawTransportMock(); + $transport->data = "HTTP/0.9 200 OK\r\n\r\n

Test"; + + $options = [ + 'hooks' => $hooks, + 'transport' => $transport, + ]; + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Response could not be parsed'); + Requests::get('http://example.com/', [], $options); + } + /** * HTTP/0.9 also appears to use a single CRLF instead of two. */ @@ -268,6 +332,28 @@ public function testSingleCRLFSeparator() { Requests::get('http://example.com/', [], $options); } + /** + * HTTP/0.9 also appears to use a single CRLF instead of two. + */ + public function testSingleCRLFSeparatorTriggersRequestsFailedCallback() { + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock->expects($this->atLeastOnce())->method('failed'); + $hooks = new Hooks(); + $hooks->register('requests.failed', [$mock, 'failed']); + + $transport = new RawTransportMock(); + $transport->data = "HTTP/0.9 200 OK\r\n

Test"; + + $options = [ + 'hooks' => $hooks, + 'transport' => $transport, + ]; + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Missing header/body separator'); + Requests::get('http://example.com/', [], $options); + } + public function testInvalidStatus() { $transport = new RawTransportMock(); $transport->data = "HTTP/1.1 OK\r\nTest: value\nAnother-Test: value\r\n\r\nTest"; @@ -281,6 +367,25 @@ public function testInvalidStatus() { Requests::get('http://example.com/', [], $options); } + public function testInvalidStatusTriggersRequestsFailedCallback() { + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock->expects($this->atLeastOnce())->method('failed'); + $hooks = new Hooks(); + $hooks->register('requests.failed', [$mock, 'failed']); + + $transport = new RawTransportMock(); + $transport->data = "HTTP/1.1 OK\r\nTest: value\nAnother-Test: value\r\n\r\nTest"; + + $options = [ + 'hooks' => $hooks, + 'transport' => $transport, + ]; + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Response could not be parsed'); + Requests::get('http://example.com/', [], $options); + } + public function test30xWithoutLocation() { $transport = new TransportMock(); $transport->code = 302; From 88b812e42597d5c9c2856abccc432f495ba1c1c9 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Fri, 17 Dec 2021 20:27:10 +0100 Subject: [PATCH 3/9] Requests: Fix style issues --- src/Requests.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Requests.php b/src/Requests.php index 8c7794dcb..8abcdaa97 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -471,12 +471,10 @@ public static function request($url, $headers = [], $data = [], $type = self::GE $options['hooks']->dispatch('requests.before_parse', [&$response, $url, $headers, $data, $type, $options]); $parsed_response = self::parse_response($response, $url, $headers, $data, $options); - } - catch (Exception $e) { + } catch (Exception $e) { $options['hooks']->dispatch('requests.failed', [$e, $url, $headers, $data, $type, $options]); throw $e; - } - catch (InvalidArgument $e) { + } catch (InvalidArgument $e) { $options['hooks']->dispatch('requests.failed', [$e, $url, $headers, $data, $type, $options]); throw $e; } From 7ace642fde39fc4b7de4a2a6916f0468d112091b Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Mon, 14 Mar 2022 09:49:04 +0100 Subject: [PATCH 4/9] Requests: Combine handling of Exception and InvalidArgument in one catch --- src/Requests.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Requests.php b/src/Requests.php index 8abcdaa97..27a608244 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -471,11 +471,8 @@ public static function request($url, $headers = [], $data = [], $type = self::GE $options['hooks']->dispatch('requests.before_parse', [&$response, $url, $headers, $data, $type, $options]); $parsed_response = self::parse_response($response, $url, $headers, $data, $options); - } catch (Exception $e) { - $options['hooks']->dispatch('requests.failed', [$e, $url, $headers, $data, $type, $options]); - throw $e; - } catch (InvalidArgument $e) { - $options['hooks']->dispatch('requests.failed', [$e, $url, $headers, $data, $type, $options]); + } catch (Exception|InvalidArgument $e) { + $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]); throw $e; } From 4c4d0b4a99bcf7e9797265be1700d692a3d702f6 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Mon, 14 Mar 2022 14:50:56 +0100 Subject: [PATCH 5/9] Requests: Handle requests.failed hook correctly in case of redirects --- src/Exception.php | 7 ++ src/Exception/InvalidArgument.php | 7 ++ src/Requests.php | 5 +- tests/Fixtures/TransportRedirectMock.php | 113 +++++++++++++++++++++++ tests/Requests/RequestsTest.php | 57 +++++++++++- 5 files changed, 183 insertions(+), 6 deletions(-) create mode 100644 tests/Fixtures/TransportRedirectMock.php diff --git a/src/Exception.php b/src/Exception.php index ef8d0ff43..bd0de3329 100644 --- a/src/Exception.php +++ b/src/Exception.php @@ -31,6 +31,13 @@ class Exception extends PHPException { */ protected $data; + /** + * Whether the exception was already passed to the requests.failed hook or not + * + * @var boolean + */ + public $failed_hook_handled = FALSE; + /** * Create a new exception * diff --git a/src/Exception/InvalidArgument.php b/src/Exception/InvalidArgument.php index 0d6d88095..565c94855 100644 --- a/src/Exception/InvalidArgument.php +++ b/src/Exception/InvalidArgument.php @@ -19,6 +19,13 @@ */ final class InvalidArgument extends InvalidArgumentException { + /** + * Whether the exception was already passed to the requests.failed hook or not + * + * @var boolean + */ + public $failed_hook_handled = FALSE; + /** * Create a new invalid argument exception with a standardized text. * diff --git a/src/Requests.php b/src/Requests.php index 27a608244..24dcd014f 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -472,7 +472,10 @@ public static function request($url, $headers = [], $data = [], $type = self::GE $parsed_response = self::parse_response($response, $url, $headers, $data, $options); } catch (Exception|InvalidArgument $e) { - $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]); + if ($e->failed_hook_handled === FALSE) { + $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]); + $e->failed_hook_handled = TRUE; + } throw $e; } diff --git a/tests/Fixtures/TransportRedirectMock.php b/tests/Fixtures/TransportRedirectMock.php new file mode 100644 index 000000000..6cc689e78 --- /dev/null +++ b/tests/Fixtures/TransportRedirectMock.php @@ -0,0 +1,113 @@ + '100 Continue', + 101 => '101 Switching Protocols', + 200 => '200 OK', + 201 => '201 Created', + 202 => '202 Accepted', + 203 => '203 Non-Authoritative Information', + 204 => '204 No Content', + 205 => '205 Reset Content', + 206 => '206 Partial Content', + 300 => '300 Multiple Choices', + 301 => '301 Moved Permanently', + 302 => '302 Found', + 303 => '303 See Other', + 304 => '304 Not Modified', + 305 => '305 Use Proxy', + 306 => '306 (Unused)', + 307 => '307 Temporary Redirect', + 400 => '400 Bad Request', + 401 => '401 Unauthorized', + 402 => '402 Payment Required', + 403 => '403 Forbidden', + 404 => '404 Not Found', + 405 => '405 Method Not Allowed', + 406 => '406 Not Acceptable', + 407 => '407 Proxy Authentication Required', + 408 => '408 Request Timeout', + 409 => '409 Conflict', + 410 => '410 Gone', + 411 => '411 Length Required', + 412 => '412 Precondition Failed', + 413 => '413 Request Entity Too Large', + 414 => '414 Request-URI Too Long', + 415 => '415 Unsupported Media Type', + 416 => '416 Requested Range Not Satisfiable', + 417 => '417 Expectation Failed', + 418 => '418 I\'m a teapot', + 428 => '428 Precondition Required', + 429 => '429 Too Many Requests', + 431 => '431 Request Header Fields Too Large', + 500 => '500 Internal Server Error', + 501 => '501 Not Implemented', + 502 => '502 Bad Gateway', + 503 => '503 Service Unavailable', + 504 => '504 Gateway Timeout', + 505 => '505 HTTP Version Not Supported', + 511 => '511 Network Authentication Required', + ]; + + public function request($url, $headers = [], $data = [], $options = []) { + if (array_key_exists($url, $this->redirected)) + { + return $this->redirected_transport->request($url, $headers, $data, $options); + } + + $redirect_url = "https://example.com/redirected?url=" . urlencode($url); + + $status = isset(self::$messages[$this->code]) ? self::$messages[$this->code] : $this->code . ' unknown'; + $response = "HTTP/1.0 $status\r\n"; + $response .= "Content-Type: text/plain\r\n"; + if ($this->chunked) { + $response .= "Transfer-Encoding: chunked\r\n"; + } + $response .= "Location: $redirect_url\r\n"; + $response .= $this->raw_headers; + $response .= "Connection: close\r\n\r\n"; + $response .= $this->body; + + $this->redirected[$url] = TRUE; + $this->redirected[$redirect_url] = TRUE; + + return $response; + } + + public function request_multiple($requests, $options) { + $responses = []; + foreach ($requests as $id => $request) { + $handler = new self(); + $handler->code = $request['options']['mock.code']; + $handler->chunked = $request['options']['mock.chunked']; + $handler->body = $request['options']['mock.body']; + $handler->raw_headers = $request['options']['mock.raw_headers']; + $responses[$id] = $handler->request($request['url'], $request['headers'], $request['data'], $request['options']); + + if (!empty($options['mock.parse'])) { + $request['options']['hooks']->dispatch('transport.internal.parse_response', [&$responses[$id], $request]); + $request['options']['hooks']->dispatch('multiple.request.complete', [&$responses[$id], $id]); + } + } + + return $responses; + } + + public static function test($capabilities = []) { + return true; + } +} diff --git a/tests/Requests/RequestsTest.php b/tests/Requests/RequestsTest.php index b62d8b3f4..bf80d5e3e 100644 --- a/tests/Requests/RequestsTest.php +++ b/tests/Requests/RequestsTest.php @@ -12,6 +12,7 @@ use WpOrg\Requests\Tests\Fixtures\TransportFailedMock; use WpOrg\Requests\Tests\Fixtures\TransportInvalidArgumentMock; use WpOrg\Requests\Tests\Fixtures\TransportMock; +use WpOrg\Requests\Tests\Fixtures\TransportRedirectMock; use WpOrg\Requests\Tests\TestCase; use WpOrg\Requests\Tests\TypeProviderHelper; @@ -157,7 +158,7 @@ public function testDefaultTransport() { public function testTransportFailedTriggersRequestsFailedCallback() { $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); - $mock->expects($this->atLeastOnce())->method('failed'); + $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -175,7 +176,7 @@ public function testTransportFailedTriggersRequestsFailedCallback() { public function testTransportInvalidArgumentTriggersRequestsFailedCallback() { $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); - $mock->expects($this->atLeastOnce())->method('failed'); + $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -299,7 +300,7 @@ public function testInvalidProtocolVersion() { */ public function testInvalidProtocolVersionTriggersRequestsFailedCallback() { $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); - $mock->expects($this->atLeastOnce())->method('failed'); + $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -337,7 +338,7 @@ public function testSingleCRLFSeparator() { */ public function testSingleCRLFSeparatorTriggersRequestsFailedCallback() { $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); - $mock->expects($this->atLeastOnce())->method('failed'); + $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -369,7 +370,7 @@ public function testInvalidStatus() { public function testInvalidStatusTriggersRequestsFailedCallback() { $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); - $mock->expects($this->atLeastOnce())->method('failed'); + $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -398,6 +399,52 @@ public function test30xWithoutLocation() { $this->assertSame(0, $response->redirects); } + public function testRedirectToExceptionTriggersRequestsFailedCallbackOnce() { + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock->expects($this->once())->method('failed'); + $hooks = new Hooks(); + $hooks->register('requests.failed', [$mock, 'failed']); + + $transport = new TransportRedirectMock(); + $transport->redirected_transport = new TransportFailedMock(); + + $options = [ + 'hooks' => $hooks, + 'transport' => $transport, + ]; + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Transport failed!'); + + $response = Requests::get('http://example.com/', [], $options); + + $this->assertSame(302, $response->status_code); + $this->assertSame(1, $response->redirects); + } + + public function testRedirectToInvalidArgumentTriggersRequestsFailedCallbackOnce() { + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock->expects($this->once())->method('failed'); + $hooks = new Hooks(); + $hooks->register('requests.failed', [$mock, 'failed']); + + $transport = new TransportRedirectMock(); + $transport->redirected_transport = new TransportInvalidArgumentMock(); + + $options = [ + 'hooks' => $hooks, + 'transport' => $transport, + ]; + + $this->expectException(InvalidArgument::class); + $this->expectExceptionMessage('Argument #1 ($url) must be of type string|Stringable'); + + $response = Requests::get('http://example.com/', [], $options); + + $this->assertSame(302, $response->status_code); + $this->assertSame(1, $response->redirects); + } + public function testTimeoutException() { $options = ['timeout' => 0.5]; $this->expectException(Exception::class); From dca20b21a09cf2dedafaa1c11020bddba1c2195c Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Tue, 15 Mar 2022 15:54:09 +0100 Subject: [PATCH 6/9] Revert "Requests: Combine handling of Exception and InvalidArgument in one catch" Multi-catch is PHP 7.1+, and we still need to support 5.6 This reverts commit 11faa0f93fd0da3b81e2818adf84e6062dfe9923. --- src/Requests.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Requests.php b/src/Requests.php index 24dcd014f..20058b204 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -471,11 +471,19 @@ public static function request($url, $headers = [], $data = [], $type = self::GE $options['hooks']->dispatch('requests.before_parse', [&$response, $url, $headers, $data, $type, $options]); $parsed_response = self::parse_response($response, $url, $headers, $data, $options); - } catch (Exception|InvalidArgument $e) { + } catch (Exception $e) { + if ($e->failed_hook_handled === FALSE) { + $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]); + $e->failed_hook_handled = TRUE; + } + + throw $e; + } catch (InvalidArgument $e) { if ($e->failed_hook_handled === FALSE) { $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]); $e->failed_hook_handled = TRUE; } + throw $e; } From 2dd33f3e6baf34dbb045e56e2055cd55c43241e2 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Tue, 15 Mar 2022 16:02:21 +0100 Subject: [PATCH 7/9] Requests: Fix style issues --- src/Exception.php | 2 +- src/Exception/InvalidArgument.php | 2 +- src/Requests.php | 8 ++++---- tests/Fixtures/TransportRedirectMock.php | 12 ++++++------ tests/Requests/RequestsTest.php | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Exception.php b/src/Exception.php index bd0de3329..8e7de2fde 100644 --- a/src/Exception.php +++ b/src/Exception.php @@ -36,7 +36,7 @@ class Exception extends PHPException { * * @var boolean */ - public $failed_hook_handled = FALSE; + public $failed_hook_handled = false; /** * Create a new exception diff --git a/src/Exception/InvalidArgument.php b/src/Exception/InvalidArgument.php index 565c94855..99cce5216 100644 --- a/src/Exception/InvalidArgument.php +++ b/src/Exception/InvalidArgument.php @@ -24,7 +24,7 @@ final class InvalidArgument extends InvalidArgumentException { * * @var boolean */ - public $failed_hook_handled = FALSE; + public $failed_hook_handled = false; /** * Create a new invalid argument exception with a standardized text. diff --git a/src/Requests.php b/src/Requests.php index 20058b204..5937b90cf 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -472,16 +472,16 @@ public static function request($url, $headers = [], $data = [], $type = self::GE $parsed_response = self::parse_response($response, $url, $headers, $data, $options); } catch (Exception $e) { - if ($e->failed_hook_handled === FALSE) { + if ($e->failed_hook_handled === false) { $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]); - $e->failed_hook_handled = TRUE; + $e->failed_hook_handled = true; } throw $e; } catch (InvalidArgument $e) { - if ($e->failed_hook_handled === FALSE) { + if ($e->failed_hook_handled === false) { $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]); - $e->failed_hook_handled = TRUE; + $e->failed_hook_handled = true; } throw $e; diff --git a/tests/Fixtures/TransportRedirectMock.php b/tests/Fixtures/TransportRedirectMock.php index 6cc689e78..2fead036c 100644 --- a/tests/Fixtures/TransportRedirectMock.php +++ b/tests/Fixtures/TransportRedirectMock.php @@ -12,7 +12,7 @@ final class TransportRedirectMock implements Transport { private $redirected = []; - public $redirected_transport = NULL; + public $redirected_transport = null; private static $messages = [ 100 => '100 Continue', @@ -64,12 +64,11 @@ final class TransportRedirectMock implements Transport { ]; public function request($url, $headers = [], $data = [], $options = []) { - if (array_key_exists($url, $this->redirected)) - { + if (array_key_exists($url, $this->redirected)) { return $this->redirected_transport->request($url, $headers, $data, $options); } - $redirect_url = "https://example.com/redirected?url=" . urlencode($url); + $redirect_url = 'https://example.com/redirected?url=' . urlencode($url); $status = isset(self::$messages[$this->code]) ? self::$messages[$this->code] : $this->code . ' unknown'; $response = "HTTP/1.0 $status\r\n"; @@ -77,13 +76,14 @@ public function request($url, $headers = [], $data = [], $options = []) { if ($this->chunked) { $response .= "Transfer-Encoding: chunked\r\n"; } + $response .= "Location: $redirect_url\r\n"; $response .= $this->raw_headers; $response .= "Connection: close\r\n\r\n"; $response .= $this->body; - $this->redirected[$url] = TRUE; - $this->redirected[$redirect_url] = TRUE; + $this->redirected[$url] = true; + $this->redirected[$redirect_url] = true; return $response; } diff --git a/tests/Requests/RequestsTest.php b/tests/Requests/RequestsTest.php index bf80d5e3e..58b6b3ab4 100644 --- a/tests/Requests/RequestsTest.php +++ b/tests/Requests/RequestsTest.php @@ -408,7 +408,7 @@ public function testRedirectToExceptionTriggersRequestsFailedCallbackOnce() { $transport = new TransportRedirectMock(); $transport->redirected_transport = new TransportFailedMock(); - $options = [ + $options = [ 'hooks' => $hooks, 'transport' => $transport, ]; @@ -431,7 +431,7 @@ public function testRedirectToInvalidArgumentTriggersRequestsFailedCallbackOnce( $transport = new TransportRedirectMock(); $transport->redirected_transport = new TransportInvalidArgumentMock(); - $options = [ + $options = [ 'hooks' => $hooks, 'transport' => $transport, ]; From 4d150be2ce22d40ba3d5393b561f70ce5ceff610 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Fri, 8 Sep 2023 10:08:05 +0200 Subject: [PATCH 8/9] Update tests for failed hook to work with phpunit 10 --- tests/Requests/RequestsTest.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Requests/RequestsTest.php b/tests/Requests/RequestsTest.php index 58b6b3ab4..f1f9dfdee 100644 --- a/tests/Requests/RequestsTest.php +++ b/tests/Requests/RequestsTest.php @@ -157,7 +157,7 @@ public function testDefaultTransport() { } public function testTransportFailedTriggersRequestsFailedCallback() { - $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock = $this->getMockedStdClassWithMethods(['failed']); $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -175,7 +175,7 @@ public function testTransportFailedTriggersRequestsFailedCallback() { } public function testTransportInvalidArgumentTriggersRequestsFailedCallback() { - $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock = $this->getMockedStdClassWithMethods(['failed']); $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -299,7 +299,7 @@ public function testInvalidProtocolVersion() { * new issue, and update your server/proxy to support a proper protocol. */ public function testInvalidProtocolVersionTriggersRequestsFailedCallback() { - $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock = $this->getMockedStdClassWithMethods(['failed']); $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -337,7 +337,7 @@ public function testSingleCRLFSeparator() { * HTTP/0.9 also appears to use a single CRLF instead of two. */ public function testSingleCRLFSeparatorTriggersRequestsFailedCallback() { - $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock = $this->getMockedStdClassWithMethods(['failed']); $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -369,7 +369,7 @@ public function testInvalidStatus() { } public function testInvalidStatusTriggersRequestsFailedCallback() { - $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock = $this->getMockedStdClassWithMethods(['failed']); $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -400,7 +400,7 @@ public function test30xWithoutLocation() { } public function testRedirectToExceptionTriggersRequestsFailedCallbackOnce() { - $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock = $this->getMockedStdClassWithMethods(['failed']); $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); @@ -423,7 +423,7 @@ public function testRedirectToExceptionTriggersRequestsFailedCallbackOnce() { } public function testRedirectToInvalidArgumentTriggersRequestsFailedCallbackOnce() { - $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock(); + $mock = $this->getMockedStdClassWithMethods(['failed']); $mock->expects($this->once())->method('failed'); $hooks = new Hooks(); $hooks->register('requests.failed', [$mock, 'failed']); From 7cec224dc7ab2745283fa92773436b359847ec76 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Wed, 28 Feb 2024 10:48:45 +0100 Subject: [PATCH 9/9] Use HTTP status helper class in TransportRedirectMock --- tests/Fixtures/TransportRedirectMock.php | 54 ++---------------------- 1 file changed, 3 insertions(+), 51 deletions(-) diff --git a/tests/Fixtures/TransportRedirectMock.php b/tests/Fixtures/TransportRedirectMock.php index 2fead036c..e6efa8b5e 100644 --- a/tests/Fixtures/TransportRedirectMock.php +++ b/tests/Fixtures/TransportRedirectMock.php @@ -3,6 +3,7 @@ namespace WpOrg\Requests\Tests\Fixtures; use WpOrg\Requests\Transport; +use WpOrg\Requests\Utility\HttpStatus; final class TransportRedirectMock implements Transport { public $code = 302; @@ -14,55 +15,6 @@ final class TransportRedirectMock implements Transport { public $redirected_transport = null; - private static $messages = [ - 100 => '100 Continue', - 101 => '101 Switching Protocols', - 200 => '200 OK', - 201 => '201 Created', - 202 => '202 Accepted', - 203 => '203 Non-Authoritative Information', - 204 => '204 No Content', - 205 => '205 Reset Content', - 206 => '206 Partial Content', - 300 => '300 Multiple Choices', - 301 => '301 Moved Permanently', - 302 => '302 Found', - 303 => '303 See Other', - 304 => '304 Not Modified', - 305 => '305 Use Proxy', - 306 => '306 (Unused)', - 307 => '307 Temporary Redirect', - 400 => '400 Bad Request', - 401 => '401 Unauthorized', - 402 => '402 Payment Required', - 403 => '403 Forbidden', - 404 => '404 Not Found', - 405 => '405 Method Not Allowed', - 406 => '406 Not Acceptable', - 407 => '407 Proxy Authentication Required', - 408 => '408 Request Timeout', - 409 => '409 Conflict', - 410 => '410 Gone', - 411 => '411 Length Required', - 412 => '412 Precondition Failed', - 413 => '413 Request Entity Too Large', - 414 => '414 Request-URI Too Long', - 415 => '415 Unsupported Media Type', - 416 => '416 Requested Range Not Satisfiable', - 417 => '417 Expectation Failed', - 418 => '418 I\'m a teapot', - 428 => '428 Precondition Required', - 429 => '429 Too Many Requests', - 431 => '431 Request Header Fields Too Large', - 500 => '500 Internal Server Error', - 501 => '501 Not Implemented', - 502 => '502 Bad Gateway', - 503 => '503 Service Unavailable', - 504 => '504 Gateway Timeout', - 505 => '505 HTTP Version Not Supported', - 511 => '511 Network Authentication Required', - ]; - public function request($url, $headers = [], $data = [], $options = []) { if (array_key_exists($url, $this->redirected)) { return $this->redirected_transport->request($url, $headers, $data, $options); @@ -70,8 +22,8 @@ public function request($url, $headers = [], $data = [], $options = []) { $redirect_url = 'https://example.com/redirected?url=' . urlencode($url); - $status = isset(self::$messages[$this->code]) ? self::$messages[$this->code] : $this->code . ' unknown'; - $response = "HTTP/1.0 $status\r\n"; + $text = HttpStatus::is_valid_code($this->code) ? HttpStatus::get_text($this->code) : 'unknown'; + $response = "HTTP/1.0 {$this->code} $text\r\n"; $response .= "Content-Type: text/plain\r\n"; if ($this->chunked) { $response .= "Transfer-Encoding: chunked\r\n";