-
Notifications
You must be signed in to change notification settings - Fork 2k
refactor(HTTP): split setCURLOptions into category helper methods to reduce complexity #10339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -526,19 +526,46 @@ protected function setResponseHeaders(array $headers = []) | |||||||||
| */ | ||||||||||
| protected function setCURLOptions(array $curlOptions = [], array $config = []) | ||||||||||
| { | ||||||||||
| // Auth Headers | ||||||||||
| if (! empty($config['auth'])) { | ||||||||||
| $curlOptions[CURLOPT_USERPWD] = $config['auth'][0] . ':' . $config['auth'][1]; | ||||||||||
| $curlOptions = $this->applyAuthOptions($curlOptions, $config); | ||||||||||
| $curlOptions = $this->applySslOptions($curlOptions, $config); | ||||||||||
| $curlOptions = $this->applyProxyOptions($curlOptions, $config); | ||||||||||
| $curlOptions = $this->applyDebugOptions($curlOptions, $config); | ||||||||||
| $curlOptions = $this->applyRedirectOptions($curlOptions, $config); | ||||||||||
| $curlOptions = $this->applyConnectionOptions($curlOptions, $config); | ||||||||||
| $curlOptions = $this->applyPayloadOptions($curlOptions, $config); | ||||||||||
|
|
||||||||||
| return $this->applyMiscOptions($curlOptions, $config); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (! empty($config['auth'][2]) && strtolower($config['auth'][2]) === 'digest') { | ||||||||||
| $curlOptions[CURLOPT_HTTPAUTH] = CURLAUTH_DIGEST; | ||||||||||
| } else { | ||||||||||
| $curlOptions[CURLOPT_HTTPAUTH] = CURLAUTH_BASIC; | ||||||||||
| } | ||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applyAuthOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // Auth Headers | ||||||||||
| if (isset($config['auth']) && $config['auth'] !== []) { | ||||||||||
| $curlOptions[CURLOPT_USERPWD] = $config['auth'][0] . ':' . $config['auth'][1]; | ||||||||||
| $curlOptions[CURLOPT_HTTPAUTH] = (isset($config['auth'][2]) && $config['auth'][2] !== '' && strtolower($config['auth'][2]) === 'digest') | ||||||||||
| ? CURLAUTH_DIGEST | ||||||||||
| : CURLAUTH_BASIC; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return $curlOptions; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applySslOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // Certificate | ||||||||||
| if (! empty($config['cert'])) { | ||||||||||
| if (isset($config['cert']) && $config['cert'] !== '' && $config['cert'] !== []) { | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $cert = $config['cert']; | ||||||||||
|
|
||||||||||
| if (is_array($cert)) { | ||||||||||
|
|
@@ -571,30 +598,51 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return $curlOptions; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applyProxyOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // Proxy | ||||||||||
| if (isset($config['proxy'])) { | ||||||||||
| $curlOptions[CURLOPT_HTTPPROXYTUNNEL] = true; | ||||||||||
| $curlOptions[CURLOPT_PROXY] = $config['proxy']; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return $curlOptions; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applyDebugOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // Debug | ||||||||||
| if ($config['debug']) { | ||||||||||
| if (isset($config['debug']) && $config['debug'] !== false && $config['debug'] !== '') { | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $curlOptions[CURLOPT_VERBOSE] = 1; | ||||||||||
| $curlOptions[CURLOPT_STDERR] = is_string($config['debug']) ? fopen($config['debug'], 'a+b') : fopen('php://stderr', 'wb'); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Decode Content | ||||||||||
| if (! empty($config['decode_content'])) { | ||||||||||
| $accept = $this->getHeaderLine('Accept-Encoding'); | ||||||||||
|
|
||||||||||
| if ($accept !== '') { | ||||||||||
| $curlOptions[CURLOPT_ENCODING] = $accept; | ||||||||||
| } else { | ||||||||||
| $curlOptions[CURLOPT_ENCODING] = ''; | ||||||||||
| $curlOptions[CURLOPT_HTTPHEADER] = 'Accept-Encoding'; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| return $curlOptions; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applyRedirectOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // Allow Redirects | ||||||||||
| if (array_key_exists('allow_redirects', $config)) { | ||||||||||
| $settings = $this->redirectDefaults; | ||||||||||
|
|
@@ -623,6 +671,17 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return $curlOptions; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applyConnectionOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // DNS Cache Timeout | ||||||||||
| if (isset($config['dns_cache_timeout']) && is_numeric($config['dns_cache_timeout']) && $config['dns_cache_timeout'] >= -1) { | ||||||||||
| $curlOptions[CURLOPT_DNS_CACHE_TIMEOUT] = (int) $config['dns_cache_timeout']; | ||||||||||
|
|
@@ -634,13 +693,45 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) | |||||||||
| : true; | ||||||||||
|
|
||||||||||
| // Timeout | ||||||||||
| $curlOptions[CURLOPT_TIMEOUT_MS] = (float) $config['timeout'] * 1000; | ||||||||||
| $curlOptions[CURLOPT_TIMEOUT_MS] = (float) ($config['timeout'] ?? 0) * 1000; | ||||||||||
|
|
||||||||||
| // Connection Timeout | ||||||||||
| $curlOptions[CURLOPT_CONNECTTIMEOUT_MS] = (float) $config['connect_timeout'] * 1000; | ||||||||||
| $curlOptions[CURLOPT_CONNECTTIMEOUT_MS] = (float) ($config['connect_timeout'] ?? 150) * 1000; | ||||||||||
|
|
||||||||||
| // Resolve IP | ||||||||||
| if (array_key_exists('force_ip_resolve', $config)) { | ||||||||||
| $curlOptions[CURLOPT_IPRESOLVE] = match ($config['force_ip_resolve']) { | ||||||||||
| 'v4' => CURL_IPRESOLVE_V4, | ||||||||||
| 'v6' => CURL_IPRESOLVE_V6, | ||||||||||
| default => CURL_IPRESOLVE_WHATEVER, | ||||||||||
| }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return $curlOptions; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applyPayloadOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // Decode Content | ||||||||||
| if (isset($config['decode_content']) && $config['decode_content'] !== false) { | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $accept = $this->getHeaderLine('Accept-Encoding'); | ||||||||||
|
|
||||||||||
| if ($accept !== '') { | ||||||||||
| $curlOptions[CURLOPT_ENCODING] = $accept; | ||||||||||
| } else { | ||||||||||
| $curlOptions[CURLOPT_ENCODING] = ''; | ||||||||||
| $curlOptions[CURLOPT_HTTPHEADER] = 'Accept-Encoding'; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Post Data - application/x-www-form-urlencoded | ||||||||||
| if (! empty($config['form_params']) && is_array($config['form_params'])) { | ||||||||||
| if (isset($config['form_params']) && is_array($config['form_params']) && $config['form_params'] !== []) { | ||||||||||
| $postFields = http_build_query($config['form_params']); | ||||||||||
| $curlOptions[CURLOPT_POSTFIELDS] = $postFields; | ||||||||||
|
|
||||||||||
|
|
@@ -651,14 +742,11 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) | |||||||||
| } | ||||||||||
|
|
||||||||||
| // Post Data - multipart/form-data | ||||||||||
| if (! empty($config['multipart']) && is_array($config['multipart'])) { | ||||||||||
| if (isset($config['multipart']) && is_array($config['multipart']) && $config['multipart'] !== []) { | ||||||||||
| // setting the POSTFIELDS option automatically sets multipart | ||||||||||
| $curlOptions[CURLOPT_POSTFIELDS] = $config['multipart']; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // HTTP Errors | ||||||||||
| $curlOptions[CURLOPT_FAILONERROR] = array_key_exists('http_errors', $config) ? (bool) $config['http_errors'] : true; | ||||||||||
|
|
||||||||||
| // JSON | ||||||||||
| if (isset($config['json'])) { | ||||||||||
| // Will be set as the body in `applyBody()` | ||||||||||
|
|
@@ -668,30 +756,37 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) | |||||||||
| $this->setHeader('Content-Length', (string) strlen($json)); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Resolve IP | ||||||||||
| if (array_key_exists('force_ip_resolve', $config)) { | ||||||||||
| $curlOptions[CURLOPT_IPRESOLVE] = match ($config['force_ip_resolve']) { | ||||||||||
| 'v4' => CURL_IPRESOLVE_V4, | ||||||||||
| 'v6' => CURL_IPRESOLVE_V6, | ||||||||||
| default => CURL_IPRESOLVE_WHATEVER, | ||||||||||
| }; | ||||||||||
| } | ||||||||||
| return $curlOptions; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @param array<int, mixed> $curlOptions | ||||||||||
| * @param array<string, mixed> $config | ||||||||||
| * | ||||||||||
| * @return array<int, mixed> | ||||||||||
| */ | ||||||||||
| private function applyMiscOptions(array $curlOptions, array $config): array | ||||||||||
| { | ||||||||||
| // HTTP Errors | ||||||||||
| $curlOptions[CURLOPT_FAILONERROR] = array_key_exists('http_errors', $config) ? (bool) $config['http_errors'] : true; | ||||||||||
|
|
||||||||||
| // version | ||||||||||
| if (! empty($config['version'])) { | ||||||||||
| if (isset($config['version']) && $config['version'] !== '') { | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $version = sprintf('%.1F', $config['version']); | ||||||||||
| if ($version === '1.0') { | ||||||||||
| $curlOptions[CURLOPT_HTTP_VERSION] = CURL_HTTP_VERSION_1_0; | ||||||||||
| } elseif ($version === '1.1') { | ||||||||||
| $curlOptions[CURLOPT_HTTP_VERSION] = CURL_HTTP_VERSION_1_1; | ||||||||||
| } elseif ($version === '2.0') { | ||||||||||
| $curlOptions[CURLOPT_HTTP_VERSION] = CURL_HTTP_VERSION_2_0; | ||||||||||
| } elseif ($version === '3.0') { | ||||||||||
| if (! defined('CURL_HTTP_VERSION_3')) { | ||||||||||
| define('CURL_HTTP_VERSION_3', 30); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| $curlOptions[CURLOPT_HTTP_VERSION] = CURL_HTTP_VERSION_3; | ||||||||||
| if (! defined('CURL_HTTP_VERSION_3')) { | ||||||||||
| define('CURL_HTTP_VERSION_3', 30); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| $versions = [ | ||||||||||
| '1.0' => CURL_HTTP_VERSION_1_0, | ||||||||||
| '1.1' => CURL_HTTP_VERSION_1_1, | ||||||||||
| '2.0' => CURL_HTTP_VERSION_2_0, | ||||||||||
| '3.0' => CURL_HTTP_VERSION_3, | ||||||||||
| ]; | ||||||||||
|
|
||||||||||
| if (isset($versions[$version])) { | ||||||||||
| $curlOptions[CURLOPT_HTTP_VERSION] = $versions[$version]; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,8 +39,6 @@ class CURLRequestTest extends CIUnitTestCase | |
|
|
||
| protected function setUp(): void | ||
| { | ||
| parent::setUp(); | ||
|
|
||
|
Comment on lines
-42
to
-43
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a mistake? Why are you removing this? |
||
| $this->resetServices(); | ||
| Services::injectMock('superglobals', new Superglobals()); | ||
| $this->request = $this->getRequest(); | ||
|
|
@@ -1566,4 +1564,127 @@ public function testProxyAndContinueResponses(): void | |
|
|
||
| $this->assertSame($testBody, $response->getBody()); | ||
| } | ||
|
|
||
| public function testApplyAuthOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applyAuthOptions'); | ||
|
|
||
| $options = $invoker([], ['auth' => ['user', 'pass', 'digest']]); | ||
| $this->assertSame('user:pass', $options[CURLOPT_USERPWD]); | ||
| $this->assertSame(CURLAUTH_DIGEST, $options[CURLOPT_HTTPAUTH]); | ||
|
|
||
| $options2 = $invoker([], ['auth' => ['user', 'pass', 'basic']]); | ||
| $this->assertSame(CURLAUTH_BASIC, $options2[CURLOPT_HTTPAUTH]); | ||
| } | ||
|
|
||
| public function testApplySslOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applySslOptions'); | ||
|
|
||
| $options = $invoker([], ['cert' => __FILE__]); | ||
| $this->assertSame(__FILE__, $options[CURLOPT_SSLCERT]); | ||
|
|
||
| $options2 = $invoker([], ['verify' => false]); | ||
| $this->assertFalse($options2[CURLOPT_SSL_VERIFYPEER]); | ||
| $this->assertSame(0, $options2[CURLOPT_SSL_VERIFYHOST]); | ||
| } | ||
|
|
||
| public function testApplyProxyOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applyProxyOptions'); | ||
|
|
||
| $options = $invoker([], ['proxy' => 'http://proxy.example.com']); | ||
| $this->assertTrue($options[CURLOPT_HTTPPROXYTUNNEL]); | ||
| $this->assertSame('http://proxy.example.com', $options[CURLOPT_PROXY]); | ||
| } | ||
|
|
||
| public function testApplyDebugOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applyDebugOptions'); | ||
|
|
||
| $options = $invoker([], ['debug' => true]); | ||
| $this->assertSame(1, $options[CURLOPT_VERBOSE]); | ||
| $this->assertIsResource($options[CURLOPT_STDERR]); | ||
| } | ||
|
|
||
| public function testApplyRedirectOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applyRedirectOptions'); | ||
|
|
||
| $options = $invoker([], ['allow_redirects' => false]); | ||
| $this->assertSame(0, $options[CURLOPT_FOLLOWLOCATION]); | ||
|
|
||
| $options2 = $invoker([], ['allow_redirects' => true]); | ||
| $this->assertSame(1, $options2[CURLOPT_FOLLOWLOCATION]); | ||
| $this->assertSame(5, $options2[CURLOPT_MAXREDIRS]); | ||
| } | ||
|
|
||
| public function testApplyConnectionOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applyConnectionOptions'); | ||
|
|
||
| $options = $invoker([], [ | ||
| 'dns_cache_timeout' => 120, | ||
| 'fresh_connect' => false, | ||
| 'timeout' => 10, | ||
| 'connect_timeout' => 5, | ||
| 'force_ip_resolve' => 'v4', | ||
| ]); | ||
|
|
||
| $this->assertSame(120, $options[CURLOPT_DNS_CACHE_TIMEOUT]); | ||
| $this->assertFalse($options[CURLOPT_FRESH_CONNECT]); | ||
| $this->assertEqualsWithDelta(10000.0, $options[CURLOPT_TIMEOUT_MS], PHP_FLOAT_EPSILON); | ||
| $this->assertEqualsWithDelta(5000.0, $options[CURLOPT_CONNECTTIMEOUT_MS], PHP_FLOAT_EPSILON); | ||
| $this->assertSame(CURL_IPRESOLVE_V4, $options[CURLOPT_IPRESOLVE]); | ||
| } | ||
|
|
||
| public function testApplyPayloadOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applyPayloadOptions'); | ||
|
|
||
| $options = $invoker([], ['form_params' => ['foo' => 'bar']]); | ||
| $this->assertSame('foo=bar', $options[CURLOPT_POSTFIELDS]); | ||
|
|
||
| $options2 = $invoker([], ['multipart' => ['file' => 'data']]); | ||
| $this->assertSame(['file' => 'data'], $options2[CURLOPT_POSTFIELDS]); | ||
| } | ||
|
|
||
| public function testApplyMiscOptionsDirect(): void | ||
| { | ||
| $invoker = self::getPrivateMethodInvoker($this->request, 'applyMiscOptions'); | ||
|
|
||
| $options = $invoker([], [ | ||
| 'http_errors' => false, | ||
| 'version' => '2.0', | ||
| 'cookie' => 'cookies.txt', | ||
| 'user_agent' => 'TestAgent', | ||
| ]); | ||
|
|
||
| $this->assertFalse($options[CURLOPT_FAILONERROR]); | ||
| $this->assertSame(CURL_HTTP_VERSION_2_0, $options[CURLOPT_HTTP_VERSION]); | ||
| $this->assertSame('cookies.txt', $options[CURLOPT_COOKIEJAR]); | ||
| $this->assertSame('cookies.txt', $options[CURLOPT_COOKIEFILE]); | ||
| $this->assertSame('TestAgent', $options[CURLOPT_USERAGENT]); | ||
| } | ||
|
|
||
| public function testCURLOptionsPreservesIntegerKeys(): void | ||
| { | ||
| // cURL options use integer constants as keys. This test ensures they are not re-indexed. | ||
| $request = $this->getRequest(); | ||
| $method = self::getPrivateMethodInvoker($request, 'setCURLOptions'); | ||
|
|
||
| $initialOptions = [ | ||
| CURLOPT_RETURNTRANSFER => true, | ||
| ]; | ||
|
|
||
| $config = [ | ||
| 'auth' => ['user', 'pass'], | ||
| ]; | ||
|
|
||
| $options = $method($initialOptions, $config); | ||
|
|
||
| // Verify keys are preserved and not re-indexed to 0, 1... | ||
| $this->assertArrayHasKey(CURLOPT_RETURNTRANSFER, $options); | ||
| $this->assertArrayHasKey(CURLOPT_USERPWD, $options); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.