From 688c120f9f33eaab5ad54647ff4fdde405cf983a Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 15 Mar 2017 18:49:03 -0700 Subject: [PATCH] Provide PhabricatorEnv::isSelfURI to test if a URI points at the current install Summary: Ref T5378. This repackages an existing check to see if a URI is a URI for the current install into a more reasonable form. In an upcoming change, I'll use this new check to test whether `http://example.whatever.com/T123` is a link to a task on the current install or not. Test Plan: This stuff has good test coverage already; added some more. Reviewers: chad Reviewed By: chad Maniphest Tasks: T5378 Differential Revision: https://secure.phabricator.com/D17502 --- ...fferentialRevisionIDCommitMessageField.php | 22 ++-------- ...DifferentialCommitMessageFieldTestCase.php | 1 + src/infrastructure/env/PhabricatorEnv.php | 43 ++++++++++++++----- .../env/__tests__/PhabricatorEnvTestCase.php | 35 +++++++++++++++ 4 files changed, 73 insertions(+), 28 deletions(-) diff --git a/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php b/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php index 8c85553e71..ac8ba4ebd4 100644 --- a/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php +++ b/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php @@ -50,24 +50,10 @@ final class DifferentialRevisionIDCommitMessageField $uri = new PhutilURI($uri_string); $path = $uri->getPath(); - $matches = null; - if (preg_match('#^/D(\d+)$#', $path, $matches)) { - $id = (int)$matches[1]; - - $prod_uri = new PhutilURI(PhabricatorEnv::getProductionURI('/D'.$id)); - - // Make sure the URI is the same as our URI. Basically, we want to ignore - // commits from other Phabricator installs. - if ($uri->getDomain() == $prod_uri->getDomain()) { - return $id; - } - - $allowed_uris = PhabricatorEnv::getAllowedURIs('/D'.$id); - - foreach ($allowed_uris as $allowed_uri) { - if ($uri_string == $allowed_uri) { - return $id; - } + if (PhabricatorEnv::isSelfURI($uri_string)) { + $matches = null; + if (preg_match('#^/D(\d+)$#', $path, $matches)) { + return (int)$matches[1]; } } diff --git a/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php b/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php index 295da8ca59..41fd2992f4 100644 --- a/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php +++ b/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php @@ -13,6 +13,7 @@ final class DifferentialCommitMessageFieldTestCase "D123\nSome-Custom-Field: The End" => 123, "{$base_uri}D123" => 123, "{$base_uri}D123\nSome-Custom-Field: The End" => 123, + 'https://www.other.com/D123' => null, ); $env = PhabricatorEnv::beginScopedEnv(); diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index b50c85c0f0..70ddb80630 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -406,21 +406,44 @@ final class PhabricatorEnv extends Phobject { return rtrim($production_domain, '/').$path; } - public static function getAllowedURIs($path) { - $uri = new PhutilURI($path); - if ($uri->getDomain()) { - return $path; + + public static function isSelfURI($raw_uri) { + $uri = new PhutilURI($raw_uri); + + $host = $uri->getDomain(); + if (!strlen($host)) { + return false; } - $allowed_uris = self::getEnvConfig('phabricator.allowed-uris'); - $return = array(); - foreach ($allowed_uris as $allowed_uri) { - $return[] = rtrim($allowed_uri, '/').$path; - } + $host = phutil_utf8_strtolower($host); - return $return; + $self_map = self::getSelfURIMap(); + return isset($self_map[$host]); } + private static function getSelfURIMap() { + $self_uris = array(); + $self_uris[] = self::getProductionURI('/'); + $self_uris[] = self::getURI('/'); + + $allowed_uris = self::getEnvConfig('phabricator.allowed-uris'); + foreach ($allowed_uris as $allowed_uri) { + $self_uris[] = $allowed_uri; + } + + $self_map = array(); + foreach ($self_uris as $self_uri) { + $host = id(new PhutilURI($self_uri))->getDomain(); + if (!strlen($host)) { + continue; + } + + $host = phutil_utf8_strtolower($host); + $self_map[$host] = $host; + } + + return $self_map; + } /** * Get the fully-qualified production URI for a static resource path. diff --git a/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php b/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php index d47258822b..f73299aa12 100644 --- a/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php +++ b/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php @@ -218,4 +218,39 @@ final class PhabricatorEnvTestCase extends PhabricatorTestCase { $this->assertFalse($caught instanceof Exception); } + public function testSelfURI() { + $base_uri = 'https://allowed.example.com/'; + + $allowed_uris = array( + 'https://old.example.com/', + ); + + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('phabricator.base-uri', $base_uri); + $env->overrideEnvConfig('phabricator.allowed-uris', $allowed_uris); + + $map = array( + 'https://allowed.example.com/' => true, + 'https://allowed.example.com' => true, + 'https://allowed.EXAMPLE.com' => true, + 'http://allowed.example.com/' => true, + 'https://allowed.example.com/path/to/resource.png' => true, + + 'https://old.example.com/' => true, + 'https://old.example.com' => true, + 'https://old.EXAMPLE.com' => true, + 'http://old.example.com/' => true, + 'https://old.example.com/path/to/resource.png' => true, + + 'https://other.example.com/' => false, + ); + + foreach ($map as $input => $expect) { + $this->assertEqual( + $expect, + PhabricatorEnv::isSelfURI($input), + pht('Is self URI? %s', $input)); + } + } + }