Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Proxy auth via http header #649

Merged
merged 7 commits into from

4 participants

@maximcherny

Proxy credentials are now passed via the "Proxy-Authorization: Basic ..." HTTP header - tested to work behind an authenticated proxy within a corporate network.

Passing the credentials via the actual proxy URL seems to have no effect when authentication is required.

@maximcherny

This essentially resolves #461

src/Composer/Util/StreamContextFactory.php
((37 lines not shown))
'request_fulluri' => true,
);
+
+ // Extract authentication credentials from the proxy url
+ $user = parse_url($proxy, PHP_URL_USER);
+ $pass = parse_url($proxy, PHP_URL_PASS);
+
+ if (isset($user)) {
+ $auth = $user;
+ if (isset($pass)) {
+ $auth .= ":{$pass}";
+ }
+ $auth = base64_encode($auth);
+
+ // Preserve headers if already set in default options
+ if (isset($defaultOptions['http']) && isset($defaultOptions['http']['header'])) {
@Seldaek Owner
Seldaek added a note

isset($defaultOptions['http']['header']) is enough, no need to check for intermediate keys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Composer/Util/StreamContextFactory.php
((58 lines not shown))
}
-
+
@Seldaek Owner
Seldaek added a note

Please remove trailing whitespace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Composer/Util/StreamContextFactory.php
((4 lines not shown))
-
- // http(s):// is not supported in proxy
- if ('http://' == substr($proxy, 0, 7)) {
- $proxy = 'tcp://' . rtrim(substr($proxy, 7), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':80');
- } else if ('https://' == substr($proxy, 0, 8)) {
- $proxy = 'ssl://' . rtrim(substr($proxy, 8), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':443');
+
+ $proxyURL = parse_url($proxy, PHP_URL_SCHEME) . "://";
+ $proxyURL .= parse_url($proxy, PHP_URL_HOST);
+
+ $proxyPort = parse_url($proxy, PHP_URL_PORT);
+
+ if (isset($proxyPort)) {
+ $proxyURL .= ":" . $proxyPort;
+ } else {
+ if ('http://' == substr($proxyURL, 0, 7)) {
@Seldaek Owner
Seldaek added a note

this could just be elseif + elseif instead of nesting it into the else block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Seldaek
Owner

Thanks, looks good apart from the few coding standards stuff I mentioned.

@maximcherny

No problem, I am pretty sure everything has been addressed

src/Composer/Util/StreamContextFactory.php
@@ -35,24 +35,51 @@ static public function getContext(array $defaultOptions = array(), array $defaul
if (isset($_SERVER['HTTP_PROXY']) || isset($_SERVER['http_proxy'])) {
// Some systems seem to rely on a lowercased version instead...
$proxy = isset($_SERVER['http_proxy']) ? $_SERVER['http_proxy'] : $_SERVER['HTTP_PROXY'];
-
- // http(s):// is not supported in proxy
- if ('http://' == substr($proxy, 0, 7)) {
- $proxy = 'tcp://' . rtrim(substr($proxy, 7), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':80');
- } else if ('https://' == substr($proxy, 0, 8)) {
- $proxy = 'ssl://' . rtrim(substr($proxy, 8), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':443');
+
+ $proxyURL = parse_url($proxy, PHP_URL_SCHEME) . "://";
+ $proxyURL .= parse_url($proxy, PHP_URL_HOST);
+
+ $proxyPort = parse_url($proxy, PHP_URL_PORT);
+
+ if (isset($proxyPort)) {
+ $proxyURL .= ":" . $proxyPort;
+ } else if ('http://' == substr($proxyURL, 0, 7)) {
@stof
stof added a note

elseif should be written without space according to the Symfony2 CS (which are used in Composer)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Composer/Util/StreamContextFactory.php
((35 lines not shown))
'request_fulluri' => true,
);
+
+ // Extract authentication credentials from the proxy url
+ $user = parse_url($proxy, PHP_URL_USER);
+ $pass = parse_url($proxy, PHP_URL_PASS);
+
+ if (isset($user)) {
+ $auth = $user;
+ if (isset($pass)) {
+ $auth .= ":{$pass}";
@stof
stof added a note

Why not $auth .= ':' . $pass ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
tests/Composer/Test/Util/StreamContextFactoryTest.php
@@ -96,7 +96,7 @@ public function testSSLProxy($expected, $proxy)
if (extension_loaded('openssl')) {
$context = StreamContextFactory::getContext();
$options = stream_context_get_options($context);
-
+
@stof
stof added a note

please remove the trailing whitespaces you added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Composer/Util/StreamContextFactory.php
((35 lines not shown))
'request_fulluri' => true,
);
+
@stof
stof added a note

you have trailing whitespaces in your (not really) empty lines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Composer/Util/StreamContextFactory.php
@@ -35,24 +35,51 @@ static public function getContext(array $defaultOptions = array(), array $defaul
if (isset($_SERVER['HTTP_PROXY']) || isset($_SERVER['http_proxy'])) {
// Some systems seem to rely on a lowercased version instead...
$proxy = isset($_SERVER['http_proxy']) ? $_SERVER['http_proxy'] : $_SERVER['HTTP_PROXY'];
-
- // http(s):// is not supported in proxy
- if ('http://' == substr($proxy, 0, 7)) {
- $proxy = 'tcp://' . rtrim(substr($proxy, 7), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':80');
- } else if ('https://' == substr($proxy, 0, 8)) {
- $proxy = 'ssl://' . rtrim(substr($proxy, 8), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':443');
+
+ $proxyURL = parse_url($proxy, PHP_URL_SCHEME) . "://";
+ $proxyURL .= parse_url($proxy, PHP_URL_HOST);
@Burgov
Burgov added a note

I may be wrong here, but wouldn't it be cheaper to call parse_url once without the second argument, and use the resulting array instead?

@Seldaek Owner
Seldaek added a note

That's correct, even though in the grand scale of things it won't make a noticeable difference, but +1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@maximcherny

Okay folks, I am hopeful this pleases you all

@stof stof commented on the diff
src/Composer/Util/StreamContextFactory.php
@@ -34,27 +34,53 @@ static public function getContext(array $defaultOptions = array(), array $defaul
// Handle system proxy
if (isset($_SERVER['HTTP_PROXY']) || isset($_SERVER['http_proxy'])) {
// Some systems seem to rely on a lowercased version instead...
- $proxy = isset($_SERVER['http_proxy']) ? $_SERVER['http_proxy'] : $_SERVER['HTTP_PROXY'];
+ $proxy = parse_url(isset($_SERVER['http_proxy']) ? $_SERVER['http_proxy'] : $_SERVER['HTTP_PROXY']);
+ } else {
+ $proxy = false;
+ }
@stof
stof added a note

I don't understand why you added the else here and moved the logic of the if in a second check

PHP manual tells me that "On seriously malformed URLs, parse_url() may return FALSE."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Seldaek Seldaek merged commit 6b3aa76 into composer:master
@Seldaek
Owner

Ok it's now merged, thanks!

@maximcherny

Nice, maybe also close #461?

@Seldaek
Owner

Done, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
44 src/Composer/Util/StreamContextFactory.php
@@ -34,27 +34,53 @@ static public function getContext(array $defaultOptions = array(), array $defaul
// Handle system proxy
if (isset($_SERVER['HTTP_PROXY']) || isset($_SERVER['http_proxy'])) {
// Some systems seem to rely on a lowercased version instead...
- $proxy = isset($_SERVER['http_proxy']) ? $_SERVER['http_proxy'] : $_SERVER['HTTP_PROXY'];
+ $proxy = parse_url(isset($_SERVER['http_proxy']) ? $_SERVER['http_proxy'] : $_SERVER['HTTP_PROXY']);
+ } else {
+ $proxy = false;
+ }
@stof
stof added a note

I don't understand why you added the else here and moved the logic of the if in a second check

PHP manual tells me that "On seriously malformed URLs, parse_url() may return FALSE."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- // http(s):// is not supported in proxy
- if ('http://' == substr($proxy, 0, 7)) {
- $proxy = 'tcp://' . rtrim(substr($proxy, 7), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':80');
- } else if ('https://' == substr($proxy, 0, 8)) {
- $proxy = 'ssl://' . rtrim(substr($proxy, 8), '/') . (parse_url($proxy, PHP_URL_PORT) ? '' : ':443');
+ if (false !== $proxy) {
+ $proxyURL = (isset($proxy['scheme']) ? $proxy['scheme'] : '') . '://';
+ $proxyURL .= isset($proxy['host']) ? $proxy['host'] : '';
+
+ if (isset($proxy['port'])) {
+ $proxyURL .= ":" . $proxy['port'];
+ } elseif ('http://' == substr($proxyURL, 0, 7)) {
+ $proxyURL .= ":80";
+ } elseif ('https://' == substr($proxyURL, 0, 8)) {
+ $proxyURL .= ":443";
}
- if (0 === strpos($proxy, 'ssl:') && !extension_loaded('openssl')) {
+ // http(s):// is not supported in proxy
+ $proxyURL = str_replace(array('http://', 'https://'), array('tcp://', 'ssl://'), $proxyURL);
+
+ if (0 === strpos($proxyURL, 'ssl:') && !extension_loaded('openssl')) {
throw new \RuntimeException('You must enable the openssl extension to use a proxy over https');
}
$options['http'] = array(
- 'proxy' => $proxy,
+ 'proxy' => $proxyURL,
'request_fulluri' => true,
);
+
+ if (isset($proxy['user'])) {
+ $auth = $proxy['user'];
+ if (isset($proxy['pass'])) {
+ $auth .= ':' . $proxy['pass'];
+ }
+ $auth = base64_encode($auth);
+
+ // Preserve headers if already set in default options
+ if (isset($defaultOptions['http']['header'])) {
+ $defaultOptions['http']['header'] .= "Proxy-Authorization: Basic {$auth}\r\n";
+ } else {
+ $options['http']['header'] = "Proxy-Authorization: Basic {$auth}\r\n";
+ }
+ }
}
$options = array_merge_recursive($options, $defaultOptions);
-
+
return stream_context_create($options, $defaultParams);
}
}
View
8 tests/Composer/Test/Util/StreamContextFactoryTest.php
@@ -63,12 +63,11 @@ public function testHttpProxy()
$context = StreamContextFactory::getContext(array('http' => array('method' => 'GET')));
$options = stream_context_get_options($context);
- $this->assertSame('http://proxyserver/', $_SERVER['HTTP_PROXY']);
-
$this->assertEquals(array('http' => array(
- 'proxy' => 'tcp://username:password@proxyserver.net:3128',
+ 'proxy' => 'tcp://proxyserver.net:3128',
'request_fulluri' => true,
'method' => 'GET',
+ 'header' => "Proxy-Authorization: Basic " . base64_encode('username:password') . "\r\n"
)), $options);
}
@@ -80,9 +79,10 @@ public function testHttpProxyWithoutPort()
$options = stream_context_get_options($context);
$this->assertEquals(array('http' => array(
- 'proxy' => 'tcp://username:password@proxyserver.net:80',
+ 'proxy' => 'tcp://proxyserver.net:80',
'request_fulluri' => true,
'method' => 'GET',
+ 'header' => "Proxy-Authorization: Basic " . base64_encode('username:password') . "\r\n"
)), $options);
}
Something went wrong with that request. Please try again.