diff --git a/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php b/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php --- a/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php +++ b/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php @@ -50,24 +50,10 @@ $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 --- a/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php +++ b/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php @@ -13,6 +13,7 @@ "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 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -406,21 +406,44 @@ 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; } + $host = phutil_utf8_strtolower($host); + + $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'); - $return = array(); foreach ($allowed_uris as $allowed_uri) { - $return[] = rtrim($allowed_uri, '/').$path; + $self_uris[] = $allowed_uri; } - return $return; - } + $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 --- a/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php +++ b/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php @@ -218,4 +218,39 @@ $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)); + } + } + }