1
0
Fork 0
mirror of https://github.com/composer/composer synced 2025-05-09 00:22:53 +00:00

Refactor proxy handling to require https_proxy (#11915)

Composer has always allowed a single http_proxy (or CGI_HTTP_PROXY)
environment variable to be used for both HTTP and HTTPS requests. But
many other tools and libraries require scheme-specific values.

The landscape is already complicated by the use of and need for upper
and lower case values, so to bring matters inline with current practice
https_proxy is now required for HTTPS requests.

The new proxy handler incorporates a transition mechanism, which allows
http_proxy to be used for all requests when https_proxy is not set and
provides a `needsTransitionWarning` method for the main application.

Moving to scheme-specific environment variables means that a user may
set a single proxy for either HTTP or HTTPS requests. To accomodate this
situation during the transition period, an https_proxy value can be set
to an empty string which will prevent http_proxy being used for HTTPS
requests.
This commit is contained in:
John Stevenson 2024-04-17 13:34:26 +01:00 committed by GitHub
parent 92f641ac3d
commit 3cc490d4c4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 717 additions and 734 deletions

View file

@ -15,8 +15,16 @@ namespace Composer\Test\Util\Http;
use Composer\Util\Http\ProxyManager;
use Composer\Test\TestCase;
/**
* @phpstan-import-type contextOptions from \Composer\Util\Http\RequestProxy
*/
class ProxyManagerTest extends TestCase
{
// isTransitional can be removed after the transition period
/** @var bool */
private $isTransitional = true;
protected function setUp(): void
{
unset(
@ -26,7 +34,8 @@ class ProxyManagerTest extends TestCase
$_SERVER['https_proxy'],
$_SERVER['NO_PROXY'],
$_SERVER['no_proxy'],
$_SERVER['CGI_HTTP_PROXY']
$_SERVER['CGI_HTTP_PROXY'],
$_SERVER['cgi_http_proxy']
);
ProxyManager::reset();
}
@ -41,7 +50,8 @@ class ProxyManagerTest extends TestCase
$_SERVER['https_proxy'],
$_SERVER['NO_PROXY'],
$_SERVER['no_proxy'],
$_SERVER['CGI_HTTP_PROXY']
$_SERVER['CGI_HTTP_PROXY'],
$_SERVER['cgi_http_proxy']
);
ProxyManager::reset();
}
@ -49,54 +59,137 @@ class ProxyManagerTest extends TestCase
public function testInstantiation(): void
{
$originalInstance = ProxyManager::getInstance();
$this->assertInstanceOf('Composer\Util\Http\ProxyManager', $originalInstance);
$sameInstance = ProxyManager::getInstance();
$this->assertTrue($originalInstance === $sameInstance);
self::assertTrue($originalInstance === $sameInstance);
ProxyManager::reset();
$newInstance = ProxyManager::getInstance();
$this->assertFalse($sameInstance === $newInstance);
self::assertFalse($sameInstance === $newInstance);
}
public function testGetProxyForRequestThrowsOnBadProxyUrl(): void
{
$_SERVER['http_proxy'] = 'localhost';
$proxyManager = ProxyManager::getInstance();
self::expectException('Composer\Downloader\TransportException');
$proxyManager->getProxyForRequest('http://example.com');
}
/**
* @dataProvider dataRequest
* @dataProvider dataCaseOverrides
*
* @param array<string, mixed> $server
* @param mixed[] $expectedOptions
* @param non-empty-string $url
* @param array<string, string> $server
* @param non-empty-string $url
*/
public function testGetProxyForRequest(array $server, string $url, string $expectedUrl, array $expectedOptions, bool $expectedSecure, string $expectedMessage): void
public function testLowercaseOverridesUppercase(array $server, string $url, string $expectedUrl): void
{
$_SERVER = array_merge($_SERVER, $server);
$proxyManager = ProxyManager::getInstance();
$proxy = $proxyManager->getProxyForRequest($url);
$this->assertInstanceOf('Composer\Util\Http\RequestProxy', $proxy);
$this->assertSame($expectedUrl, $proxy->getUrl());
$this->assertSame($expectedOptions, $proxy->getContextOptions());
$this->assertSame($expectedSecure, $proxy->isSecure());
$message = $proxy->getFormattedUrl();
if ($expectedMessage) {
$condition = stripos($message, $expectedMessage) !== false;
} else {
$condition = $expectedMessage === $message;
}
$this->assertTrue($condition, 'lastProxy check');
self::assertSame($expectedUrl, $proxy->getStatus());
}
/**
* @return list<array{0: array<string, string>, 1: string, 2: string}>
*/
public static function dataCaseOverrides(): array
{
// server, url, expectedUrl
return [
[['HTTP_PROXY' => 'http://upper.com', 'http_proxy' => 'http://lower.com'], 'http://repo.org', 'http://lower.com:80'],
[['CGI_HTTP_PROXY' => 'http://upper.com', 'cgi_http_proxy' => 'http://lower.com'], 'http://repo.org', 'http://lower.com:80'],
[['HTTPS_PROXY' => 'http://upper.com', 'https_proxy' => 'http://lower.com'], 'https://repo.org', 'http://lower.com:80'],
];
}
/**
* @dataProvider dataCGIProxy
*
* @param array<string, string> $server
*/
public function testCGIProxyIsOnlyUsedWhenNoHttpProxy(array $server, string $expectedUrl): void
{
$_SERVER = array_merge($_SERVER, $server);
$proxyManager = ProxyManager::getInstance();
$proxy = $proxyManager->getProxyForRequest('http://repo.org');
self::assertSame($expectedUrl, $proxy->getStatus());
}
/**
* @return list<array{0: array<string, string>, 1: string}>
*/
public static function dataCGIProxy(): array
{
// server, expectedUrl
return [
[['CGI_HTTP_PROXY' => 'http://cgi.com:80'], 'http://cgi.com:80'],
[['http_proxy' => 'http://http.com:80', 'CGI_HTTP_PROXY' => 'http://cgi.com:80'], 'http://http.com:80'],
];
}
public function testNoHttpProxyDoesNotUseHttpsProxy(): void
{
$_SERVER['https_proxy'] = 'https://proxy.com:443';
$proxyManager = ProxyManager::getInstance();
$proxy = $proxyManager->getProxyForRequest('http://repo.org');
self::assertSame('', $proxy->getStatus());
}
public function testNoHttpsProxyDoesNotUseHttpProxy(): void
{
$_SERVER['http_proxy'] = 'http://proxy.com:80';
// This can be removed after the transition period.
// An empty https_proxy value prevents using any http_proxy
if ($this->isTransitional) {
$_SERVER['https_proxy'] = '';
}
$proxyManager = ProxyManager::getInstance();
$proxy = $proxyManager->getProxyForRequest('https://repo.org');
self::assertSame('', $proxy->getStatus());
}
/**
* This test can be removed after the transition period
*/
public function testTransitional(): void
{
$_SERVER['http_proxy'] = 'http://proxy.com:80';
$proxyManager = ProxyManager::getInstance();
$proxy = $proxyManager->getProxyForRequest('https://repo.org');
self::assertSame('http://proxy.com:80', $proxy->getStatus());
self::assertTrue($proxyManager->needsTransitionWarning());
}
/**
* @dataProvider dataRequest
*
* @param array<string, string> $server
* @param non-empty-string $url
* @param ?contextOptions $options
*/
public function testGetProxyForRequest(array $server, string $url, ?array $options, string $status, bool $excluded): void
{
$_SERVER = array_merge($_SERVER, $server);
$proxyManager = ProxyManager::getInstance();
$proxy = $proxyManager->getProxyForRequest($url);
self::assertSame($options, $proxy->getContextOptions());
self::assertSame($status, $proxy->getStatus());
self::assertSame($excluded, $proxy->isExcludedByNoProxy());
}
/**
* Tests context options. curl options are tested in RequestProxyTest.php
*
* @return list<array{0: array<string, string>, 1: string, 2: ?contextOptions, 3: string, 4: bool}>
*/
public static function dataRequest(): array
{
$server = [
@ -105,68 +198,26 @@ class ProxyManagerTest extends TestCase
'no_proxy' => 'other.repo.org',
];
// server, url, expectedUrl, expectedOptions, expectedSecure, expectedMessage
// server, url, options, status, excluded
return [
[[], 'http://repo.org', '', [], false, ''],
[$server, 'http://repo.org', 'http://user:p%40ss@proxy.com:80',
[[], 'http://repo.org', null, '', false],
[$server, 'http://repo.org',
['http' => [
'proxy' => 'tcp://proxy.com:80',
'header' => 'Proxy-Authorization: Basic dXNlcjpwQHNz',
'request_fulluri' => true,
]],
'http://***:***@proxy.com:80',
false,
'http://user:***@proxy.com:80',
],
[
$server, 'https://repo.org', 'https://proxy.com:443',
[$server, 'https://repo.org',
['http' => [
'proxy' => 'ssl://proxy.com:443',
]],
true,
'https://proxy.com:443',
false,
],
[$server, 'https://other.repo.org', '', [], false, 'no_proxy'],
];
}
/**
* @dataProvider dataStatus
*
* @param array<string, mixed> $server
*/
public function testGetStatus(array $server, bool $expectedStatus, ?string $expectedMessage): void
{
$_SERVER = array_merge($_SERVER, $server);
$proxyManager = ProxyManager::getInstance();
$status = $proxyManager->isProxying();
$message = $proxyManager->getFormattedProxy();
$this->assertSame($expectedStatus, $status);
if ($expectedMessage !== null) {
$condition = stripos($message, $expectedMessage) !== false;
} else {
$condition = $expectedMessage === $message;
}
$this->assertTrue($condition, 'message check');
}
public static function dataStatus(): array
{
// server, expectedStatus, expectedMessage
return [
[[], false, null],
[['http_proxy' => 'localhost'], false, 'malformed'],
[
['http_proxy' => 'http://user:p%40ss@proxy.com:80'],
true,
'http=http://user:***@proxy.com:80',
],
[
['http_proxy' => 'proxy.com:80', 'https_proxy' => 'proxy.com:80'],
true,
'http=proxy.com:80, https=proxy.com:80',
],
[$server, 'https://other.repo.org', null, 'excluded by no_proxy', true],
];
}
}