Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add IPv4 fallback on connection timeout, and adds COMPOSER_IPRESOLVE env var #11791

Merged
merged 3 commits into from
Jan 11, 2024
Merged

Add IPv4 fallback on connection timeout, and adds COMPOSER_IPRESOLVE env var #11791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d795e7d
Add IPv4 fallback on connection timeout, and adds COMPOSER_IPRESOLVE …
Seldaek Jan 10, 2024
fff3b86
Address feedback
Seldaek Jan 10, 2024
5a1708c
Add warning in diagnose command when COMPOSER_IPRESOLVE is set
Seldaek Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/03-cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,11 @@ defaults to 12 and must be between 1 and 50. If your proxy has issues with
concurrency maybe you want to lower this. Increasing it should generally not result
in performance gains.

### COMPOSER_IPRESOLVE

Set to `4` or `6` to force IPv4 or IPv6 DNS resolution. This only works when the
curl extension is used for downloads.

### HTTP_PROXY_REQUEST_FULLURI

If you use a proxy, but it does not support the request_fulluri flag, then you
Expand Down
5 changes: 5 additions & 0 deletions doc/articles/troubleshooting.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,11 @@ open stream: Operation timed out
We recommend you fix your IPv6 setup. If that is not possible, you can try the
following workarounds:

**Generic Workaround:**

Set the [`COMPOSER_IPRESOLVE=4`](../03-cli.md#composer-ipresolve) environment variable which will force curl to resolve
domains using IPv4. This only works when the curl extension is used for downloads.

**Workaround Linux:**

On linux, it seems that running this command helps to make ipv4 traffic have a
Expand Down
12 changes: 1 addition & 11 deletions phpstan/baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,16 +765,6 @@ parameters:
count: 1
path: ../src/Composer/Command/ShowCommand.php

-
message: "#^Only booleans are allowed in \\|\\|, array\\<string, Composer\\\\Package\\\\Link\\> given on the left side\\.$#"
count: 1
path: ../src/Composer/Command/ShowCommand.php

-
message: "#^Only booleans are allowed in \\|\\|, array\\<string, Composer\\\\Package\\\\Link\\> given on the right side\\.$#"
count: 1
path: ../src/Composer/Command/ShowCommand.php

-
message: "#^Parameter \\#1 \\$arrayTree of method Composer\\\\Command\\\\ShowCommand\\:\\:displayPackageTree\\(\\) expects array\\<int, array\\<string, array\\|string\\>\\>, array\\<int, array\\<string, array\\<int, array\\<string, array\\|string\\>\\>\\|string\\|null\\>\\> given\\.$#"
count: 2
Expand Down Expand Up @@ -4314,7 +4304,7 @@ parameters:
path: ../src/Composer/Util/Http/CurlDownloader.php

-
message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\<array\\{url\\: non\\-empty\\-string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: CurlHandle, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\<array\\{url\\: non\\-empty\\-string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: CurlHandle\\|resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\.$#"
message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\<array\\{url\\: non\\-empty\\-string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool, ipResolve\\: 4\\|6\\|null\\}, options\\: array, progress\\: array, curlHandle\\: CurlHandle, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\<array\\{url\\: non\\-empty\\-string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool, ipResolve\\: 4\\|6\\|null\\}, options\\: array, progress\\: array, curlHandle\\: CurlHandle\\|resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\.$#"
count: 1
path: ../src/Composer/Util/Http/CurlDownloader.php

Expand Down
7 changes: 6 additions & 1 deletion src/Composer/Command/DiagnoseCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ private function outputResult($result): void

if ($result) {
foreach ($result as $message) {
$io->write($message);
$io->write(trim($message));
}
}
}
Expand Down Expand Up @@ -776,6 +776,11 @@ private function checkPlatform()
$out($iniMessage, 'comment');
}

if (in_array(Platform::getEnv('COMPOSER_IPRESOLVE'), ['4', '6'], true)) {
$warnings['ipresolve'] = true;
$out('The COMPOSER_IPRESOLVE env var is set to ' . Platform::getEnv('COMPOSER_IPRESOLVE') .' which may result in network failures below.', 'comment');
}

return count($warnings) === 0 && count($errors) === 0 ? true : $output;
}

Expand Down
27 changes: 22 additions & 5 deletions src/Composer/Util/Http/CurlDownloader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* @internal
* @author Jordi Boggiano <j.boggiano@seld.be>
* @author Nicolas Grekas <p@tchwork.com>
* @phpstan-type Attributes array{retryAuthFailure: bool, redirects: int<0, max>, retries: int<0, max>, storeAuth: 'prompt'|bool}
* @phpstan-type Attributes array{retryAuthFailure: bool, redirects: int<0, max>, retries: int<0, max>, storeAuth: 'prompt'|bool, ipResolve: 4|6|null}
* @phpstan-type Job array{url: non-empty-string, origin: string, attributes: Attributes, options: mixed[], progress: mixed[], curlHandle: \CurlHandle, filename: string|null, headerHandle: resource, bodyHandle: resource, resolve: callable, reject: callable}
*/
class CurlDownloader
Expand Down Expand Up @@ -143,7 +143,7 @@ public function download(callable $resolve, callable $reject, string $origin, st
/**
* @param mixed[] $options
*
* @param array{retryAuthFailure?: bool, redirects?: int<0, max>, retries?: int<0, max>, storeAuth?: 'prompt'|bool} $attributes
* @param array{retryAuthFailure?: bool, redirects?: int<0, max>, retries?: int<0, max>, storeAuth?: 'prompt'|bool, ipResolve?: 4|6|null} $attributes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't make up your mind whether to use |null or ? and here you use both? ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's optional to pass it but it defaults to null so yeah this looks weird but works well enough

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ? after a key does not indicate nullability but the fact that this key is optional in the array.

* @param non-empty-string $url
*
* @return int internal job id
Expand All @@ -155,8 +155,15 @@ private function initDownload(callable $resolve, callable $reject, string $origi
'redirects' => 0,
'retries' => 0,
'storeAuth' => false,
'ipResolve' => null,
Seldaek marked this conversation as resolved.
Show resolved Hide resolved
], $attributes);

if ($attributes['ipResolve'] === null && Platform::getEnv('COMPOSER_IPRESOLVE') === '4') {
$attributes['ipResolve'] = 4;
} elseif ($attributes['ipResolve'] === null && Platform::getEnv('COMPOSER_IPRESOLVE') === '6') {
$attributes['ipResolve'] = 6;
}

$originalOptions = $options;

// check URL can be accessed (i.e. is not insecure), but allow insecure Packagist calls to $hashed providers as file integrity is verified with sha256
Expand Down Expand Up @@ -199,6 +206,12 @@ private function initDownload(callable $resolve, callable $reject, string $origi
curl_setopt($curlHandle, CURLOPT_ENCODING, ""); // let cURL set the Accept-Encoding header to what it supports
curl_setopt($curlHandle, CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS);

if ($attributes['ipResolve'] === 4) {
curl_setopt($curlHandle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

curl_setopt($curlHandle, CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS, 0);

When using the IPv4 protocol, is it possible to change the default 200ms wait for IPv6 to 0ms?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if resolution is forced to be ipv4, there won't be any happy eyeballs happening, so this timeout won't ever be used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that would be my expectation as well, so I don't think this is necessary.

} elseif ($attributes['ipResolve'] === 6) {
curl_setopt($curlHandle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V6);
}

if (function_exists('curl_share_init')) {
curl_setopt($curlHandle, CURLOPT_SHARE, $this->shareHandle);
}
Expand Down Expand Up @@ -352,8 +365,12 @@ public function tick(): void
|| (in_array($errno, [56 /* CURLE_RECV_ERROR */, 35 /* CURLE_SSL_CONNECT_ERROR */], true) && str_contains((string) $error, 'Connection reset by peer'))
) && $job['attributes']['retries'] < $this->maxRetries
) {
$attributes = ['retries' => $job['attributes']['retries'] + 1];
if ($errno === 7 && !isset($job['attributes']['ipResolve'])) { // CURLE_COULDNT_CONNECT, retry forcing IPv4 if no IP stack was selected
Seldaek marked this conversation as resolved.
Show resolved Hide resolved
$attributes['ipResolve'] = 4;
}
$this->io->writeError('Retrying ('.($job['attributes']['retries'] + 1).') ' . Url::sanitize($job['url']) . ' due to curl error '. $errno, true, IOInterface::DEBUG);
$this->restartJobWithDelay($job, $job['url'], ['retries' => $job['attributes']['retries'] + 1]);
$this->restartJobWithDelay($job, $job['url'], $attributes);
continue;
}

Expand Down Expand Up @@ -582,7 +599,7 @@ private function isAuthenticatedRetryNeeded(array $job, Response $response): arr
* @param Job $job
* @param non-empty-string $url
*
* @param array{retryAuthFailure?: bool, redirects?: int<0, max>, storeAuth?: 'prompt'|bool, retries?: int<1, max>} $attributes
* @param array{retryAuthFailure?: bool, redirects?: int<0, max>, storeAuth?: 'prompt'|bool, retries?: int<1, max>, ipResolve?: 4|6} $attributes
*/
private function restartJob(array $job, string $url, array $attributes = []): void
{
Expand All @@ -600,7 +617,7 @@ private function restartJob(array $job, string $url, array $attributes = []): vo
* @param Job $job
* @param non-empty-string $url
*
* @param array{retryAuthFailure?: bool, redirects?: int<0, max>, storeAuth?: 'prompt'|bool, retries: int<1, max>} $attributes
* @param array{retryAuthFailure?: bool, redirects?: int<0, max>, storeAuth?: 'prompt'|bool, retries: int<1, max>, ipResolve?: 4|6} $attributes
*/
private function restartJobWithDelay(array $job, string $url, array $attributes): void
{
Expand Down