From c762050b7ce86e0ec60d36f00ec4c3258c05980d Mon Sep 17 00:00:00 2001 From: vrana Date: Mon, 18 Jun 2012 15:11:47 -0700 Subject: [PATCH] Get rid of file_get_contents($uri) Summary: It requires `allow_url_fopen` which we don't check in setup and our installation is about to disable it. Test Plan: Login with OAuth. /oauth/facebook/diagnose/ Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D2787 --- .../PhabricatorOAuthDiagnosticsController.php | 47 +++++++------------ .../PhabricatorOAuthLoginController.php | 33 +++---------- .../PhabricatorOAuthProviderDisqus.php | 2 +- .../PhabricatorOAuthProviderFacebook.php | 2 +- .../PhabricatorOAuthProviderGitHub.php | 2 +- .../PhabricatorOAuthProviderPhabricator.php | 2 +- .../files/storage/PhabricatorFile.php | 9 +--- ...icatorUserOAuthSettingsPanelController.php | 2 +- src/infrastructure/PhabricatorSetup.php | 4 +- .../handler/PhabricatorIRCMacroHandler.php | 2 +- 10 files changed, 34 insertions(+), 71 deletions(-) diff --git a/src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php b/src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php index 5fada6c989..ec9add8a23 100644 --- a/src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php +++ b/src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php @@ -86,21 +86,9 @@ final class PhabricatorOAuthDiagnosticsController 'Application secret is set.'); } - $timeout = stream_context_create( - array( - 'http' => array( - 'ignore_errors' => true, - 'timeout' => 5, - ), - )); - $timeout_strict = stream_context_create( - array( - 'http' => array( - 'timeout' => 5, - ), - )); + $timeout = 5; - $internet = @file_get_contents("http://google.com/", false, $timeout); + $internet = HTTPSFuture::loadContent("http://google.com/", $timeout); if ($internet === false) { $results['internet'] = array( $res_no, @@ -116,7 +104,7 @@ final class PhabricatorOAuthDiagnosticsController $test_uris = $provider->getTestURIs(); foreach ($test_uris as $uri) { - $success = @file_get_contents($uri, false, $timeout); + $success = HTTPSFuture::loadContent($uri, $timeout); if ($success === false) { $results[$uri] = array( $res_no, @@ -140,22 +128,23 @@ final class PhabricatorOAuthDiagnosticsController 'grant_type' => 'client_credentials', )); - $token_value = @file_get_contents($test_uri, false, $timeout); - $token_strict = @file_get_contents($test_uri, false, $timeout_strict); - if ($token_value === false) { + $future = new HTTPSFuture($test_uri); + $future->setTimeout($timeout); + try { + list($body) = $future->resolvex(); $results['App Login'] = array( - $res_no, - null, - "Unable to perform an application login with your Application ID ". - "and Application Secret. You may have mistyped or misconfigured ". - "them; {$name} may have revoked your authorization; or {$name} may ". - "be having technical problems."); - } else { - if ($token_strict) { + $res_ok, + '(A Valid Token)', + "Raw application login to {$name} works."); + } catch (Exception $ex) { + if ($ex instanceof HTTPFutureResponseStatusCURL) { $results['App Login'] = array( - $res_ok, - '(A Valid Token)', - "Raw application login to {$name} works."); + $res_no, + null, + "Unable to perform an application login with your Application ID ". + "and Application Secret. You may have mistyped or misconfigured ". + "them; {$name} may have revoked your authorization; or {$name} ". + "may be having technical problems."); } else { $data = json_decode($token_value, true); if (!is_array($data)) { diff --git a/src/applications/auth/controller/PhabricatorOAuthLoginController.php b/src/applications/auth/controller/PhabricatorOAuthLoginController.php index 219a9f3200..8591c63c1d 100644 --- a/src/applications/auth/controller/PhabricatorOAuthLoginController.php +++ b/src/applications/auth/controller/PhabricatorOAuthLoginController.php @@ -63,7 +63,7 @@ final class PhabricatorOAuthLoginController $userinfo_uri = (string)$userinfo_uri; try { - $user_data = @file_get_contents($userinfo_uri); + $user_data = HTTPSFuture::loadContent($userinfo_uri); if ($user_data === false) { throw new PhabricatorOAuthProviderException( "Request to '{$userinfo_uri}' failed!"); @@ -262,34 +262,13 @@ final class PhabricatorOAuthLoginController 'code' => $code, ) + $provider->getExtraTokenParameters(); - $post_data = http_build_query($query_data, '', '&'); - $post_length = strlen($post_data); - - $stream_context = stream_context_create( - array( - 'http' => array( - 'method' => 'POST', - 'header' => - "Content-Type: application/x-www-form-urlencoded\r\n". - "Content-Length: {$post_length}\r\n", - 'content' => $post_data, - ), - )); - - $stream = fopen($auth_uri, 'r', false, $stream_context); - - $response = false; - $meta = null; - if ($stream) { - $meta = stream_get_meta_data($stream); - $response = stream_get_contents($stream); - fclose($stream); - } - - if ($response === false) { + $future = new HTTPSFuture($auth_uri, $query_data); + $future->setMethod('POST'); + try { + list($response) = $future->resolvex(); + } catch (Exception $ex) { return $this->buildErrorResponse(new PhabricatorOAuthFailureView()); } - $data = $provider->decodeTokenResponse($response); $token = idx($data, 'access_token'); diff --git a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderDisqus.php b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderDisqus.php index ad07d3cad0..126d463c43 100644 --- a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderDisqus.php +++ b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderDisqus.php @@ -124,7 +124,7 @@ final class PhabricatorOAuthProviderDisqus extends PhabricatorOAuthProvider { if ($avatar) { $uri = idx($avatar, 'permalink'); if ($uri) { - return @file_get_contents($uri); + return HTTPSFuture::loadContent($uri); } } return null; diff --git a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php index a8d16bd351..ec396d7145 100644 --- a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php +++ b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php @@ -111,7 +111,7 @@ final class PhabricatorOAuthProviderFacebook extends PhabricatorOAuthProvider { public function retrieveUserProfileImage() { $uri = 'https://graph.facebook.com/me/picture?access_token='; - return @file_get_contents($uri.$this->getAccessToken()); + return HTTPSFuture::loadContent($uri.$this->getAccessToken()); } public function retrieveUserAccountURI() { diff --git a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderGitHub.php b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderGitHub.php index ee8fb7e08f..c91b30a44f 100644 --- a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderGitHub.php +++ b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderGitHub.php @@ -105,7 +105,7 @@ final class PhabricatorOAuthProviderGitHub extends PhabricatorOAuthProvider { public function retrieveUserProfileImage() { $uri = idx($this->userData, 'avatar_url'); if ($uri) { - return @file_get_contents($uri); + return HTTPSFuture::loadContent($uri); } return null; } diff --git a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderPhabricator.php b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderPhabricator.php index f299bd9e47..68096386cc 100644 --- a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderPhabricator.php +++ b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderPhabricator.php @@ -126,7 +126,7 @@ extends PhabricatorOAuthProvider { public function retrieveUserProfileImage() { $uri = $this->userData['image']; - return @file_get_contents($uri); + return HTTPSFuture::loadContent($uri); } public function retrieveUserAccountURI() { diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 08b091e05b..c0be163cbd 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -203,14 +203,9 @@ final class PhabricatorFile extends PhabricatorFileDAO { return null; } - $timeout = stream_context_create( - array( - 'http' => array( - 'timeout' => 5, - ), - )); + $timeout = 5; - $file_data = @file_get_contents($uri, false, $timeout); + $file_data = HTTPSFuture::loadContent($uri, $timeout); if ($file_data === false) { return null; } diff --git a/src/applications/people/controller/settings/panels/PhabricatorUserOAuthSettingsPanelController.php b/src/applications/people/controller/settings/panels/PhabricatorUserOAuthSettingsPanelController.php index 2c00bc441b..8f8dbf8fd4 100644 --- a/src/applications/people/controller/settings/panels/PhabricatorUserOAuthSettingsPanelController.php +++ b/src/applications/people/controller/settings/panels/PhabricatorUserOAuthSettingsPanelController.php @@ -206,7 +206,7 @@ final class PhabricatorUserOAuthSettingsPanelController $token = $oauth_info->getToken(); try { $userinfo_uri->setQueryParam('access_token', $token); - $user_data = @file_get_contents($userinfo_uri); + $user_data = HTTPSFuture::loadContent($userinfo_uri); $provider->setUserData($user_data); $provider->setAccessToken($token); $image = $provider->retrieveUserProfileImage(); diff --git a/src/infrastructure/PhabricatorSetup.php b/src/infrastructure/PhabricatorSetup.php index e42c96b471..e3fe9d618b 100644 --- a/src/infrastructure/PhabricatorSetup.php +++ b/src/infrastructure/PhabricatorSetup.php @@ -155,8 +155,8 @@ final class PhabricatorSetup { 'iconv', // There is a chance we might not need this, but some configurations (like - // Amazon SES) will require it. Just mark it 'required' since it's widely - // available and relatively core. + // OAuth or Amazon SES) will require it. Just mark it 'required' since + // it's widely available and relatively core. 'curl', ); foreach ($extensions as $extension) { diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php b/src/infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php index f5008689f0..63ff2879b8 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php +++ b/src/infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php @@ -119,7 +119,7 @@ final class PhabricatorIRCMacroHandler extends PhabricatorIRCHandler { } public function rasterize($macro, $size, $aspect) { - $image = @file_get_contents($macro['uri']); + $image = HTTPSFuture::loadContent($macro['uri']); if (!$image) { return false; }