diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -12668,7 +12668,7 @@ 'PhutilRemarkupDelRule' => 'PhutilRemarkupRule', 'PhutilRemarkupDocumentLinkRule' => 'PhutilRemarkupRule', 'PhutilRemarkupEngine' => 'PhutilMarkupEngine', - 'PhutilRemarkupEngineTestCase' => 'PhutilTestCase', + 'PhutilRemarkupEngineTestCase' => 'PhabricatorTestCase', 'PhutilRemarkupEscapeRemarkupRule' => 'PhutilRemarkupRule', 'PhutilRemarkupEvalRule' => 'PhutilRemarkupRule', 'PhutilRemarkupHeaderBlockRule' => 'PhutilRemarkupBlockRule', 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 @@ -11,7 +11,7 @@ * The primary role of this class is to provide an API for reading * Phabricator configuration, @{method:getEnvConfig}: * - * $value = PhabricatorEnv::getEnvConfig('some.key', $default); + * $value = PhabricatorEnv::getEnvConfig('some.key'); * * The class also handles some URI construction based on configuration, via * the methods @{method:getURI}, @{method:getProductionURI}, @@ -428,14 +428,29 @@ } + /** + * Check whenever an URI points to this very same Phorge installation. + * @param string $raw_uri + * @return bool True, if this URI points to Phorge itself. + */ public static function isSelfURI($raw_uri) { $uri = new PhutilURI($raw_uri); + // Relative paths always are self-URIs. $host = $uri->getDomain(); - if (!phutil_nonempty_string($host)) { + $prot = $uri->getProtocol(); + $empty_host = !phutil_nonempty_string($host); + $empty_prot = !phutil_nonempty_string($prot); + if ($empty_host && $empty_prot) { return true; } + // When we have a protocol, but no host, + // this is probably a 'mailto' or another external thing. + if ($empty_host) { + return false; + } + $host = phutil_utf8_strtolower($host); $self_map = self::getSelfURIMap(); diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php --- a/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php +++ b/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php @@ -44,7 +44,13 @@ protected function renderHyperlink($link, $name) { $engine = $this->getEngine(); + // Setup the utility that recognizes internal/external URIs. $uri = new PhutilURIHelper($link); + $uri_here = $engine->getConfig('uri.here'); + if ($uri_here) { + $uri->addExtraTrustedURI($uri_here); + } + $is_anchor = $uri->isAnchor(); $starts_with_slash = $uri->isStartingWithSlash(); if ($starts_with_slash) { @@ -52,7 +58,7 @@ $base = rtrim($base, '/'); $link = $base.$link; } else if ($is_anchor) { - $here = $engine->getConfig('uri.here'); + $here = $uri_here; $link = $here.$link; } diff --git a/src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php b/src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php --- a/src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php +++ b/src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php @@ -2,12 +2,15 @@ /** * Test cases for @{class:PhutilRemarkupEngine}. + * This needs to be a PhabricatorTestCase since it may want + * to access some Phorge URI configurations. * @TODO: This unit is not always triggered when you need it. * https://we.phorge.it/T15500 */ -final class PhutilRemarkupEngineTestCase extends PhutilTestCase { +final class PhutilRemarkupEngineTestCase extends PhabricatorTestCase { public function testEngine() { + // Test each '.txt' test file. $root = dirname(__FILE__).'/remarkup/'; foreach (Filesystem::listDirectory($root, $hidden = false) as $file) { $this->markupText($root.$file); @@ -48,6 +51,7 @@ $engine->setConfig('uri.same-window', true); break; case 'link-square.txt': + // Setup URI used as base in this Remarkup document. $engine->setConfig('uri.base', 'http://www.example.com/'); $engine->setConfig('uri.here', 'http://www.example.com/page/'); break; diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-brackets.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-brackets.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-brackets.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-brackets.txt @@ -1,5 +1,5 @@ ~~~~~~~~~~ -

http://www.zany.com/omg/weird_url,,,

+

http://www.zany.com/omg/weird_url,,,

~~~~~~~~~~ http://www.zany.com/omg/weird_url,,, diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-edge-cases.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-edge-cases.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-edge-cases.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-edge-cases.txt @@ -10,17 +10,17 @@ Quick! http://www.example.com/! ~~~~~~~~~~ -

http://www.example.com/

+

http://www.example.com/

-

(http://www.example.com/)

+

(http://www.example.com/)

-

http://www.example.com/

+

http://www.example.com/

-

http://www.example.com/wiki/example_(disambiguation)

+

http://www.example.com/wiki/example_(disambiguation)

-

(example http://www.example.com/)

+

(example http://www.example.com/)

-

Quick! http://www.example.com/!

+

Quick! http://www.example.com/!

~~~~~~~~~~ http://www.example.com/ diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mailto.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mailto.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mailto.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mailto.txt @@ -5,11 +5,11 @@ [[mailto:alincoln@example.com]] ~~~~~~~~~~ -

mail me

+

mail me

-

mail me

+

mail me

-

alincoln@example.com

+

alincoln@example.com

~~~~~~~~~~ mail me diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mixed.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mixed.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mixed.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-mixed.txt @@ -5,11 +5,11 @@ ~~~~~~~~~~ -

Example(http://www.alternate.org/)

+

Example(http://www.alternate.org/)

-

(http://www.alternate.org/)Example

+

(http://www.alternate.org/)Example

-

<http://www.example.com/ Example>

+

<http://www.example.com/ Example>

~~~~~~~~~~ Example (http://www.alternate.org/) diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-noreferrer.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-noreferrer.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-noreferrer.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-noreferrer.txt @@ -1,15 +1,27 @@ +I have no idea what should happen in this case. +Technically it does not seem to be an external link. +Browsers may make it external, though? +But I guess that the TARGET attribute is not the important part here... + [[ /\evil.com ]] [[ / /evil.com ]] ~~~~~~~~~~ -

/\evil.com

+

I have no idea what should happen in this case. +Technically it does not seem to be an external link. +Browsers may make it external, though? +But I guess that the TARGET attribute is not the important part here...

+ +

/\evil.com

/ +/evil.com" class="remarkup-link" rel="noreferrer">/ /evil.com

~~~~~~~~~~ +I have no idea what should happen in this case. Technically it does not seem to be an external link. Browsers may make it external, though? But I guess that the TARGET attribute is not the important part here... + /\evil.com / diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-same-window.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-same-window.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-same-window.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-same-window.txt @@ -2,9 +2,9 @@ http://www.example.com/ ~~~~~~~~~~ -

http://www.example.com/

+

http://www.example.com/

-

http://www.example.com/

+

http://www.example.com/

~~~~~~~~~~ http://www.example.com/ diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-square.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-square.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-square.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-square.txt @@ -1,3 +1,5 @@ +This specific test assumes that www.example.com is our Phorge. + [[http://www.example.com/]] [[http://www.example.com/ | example.com]] @@ -8,16 +10,20 @@ [[#anchor | Anchors ]] ~~~~~~~~~~ -

http://www.example.com/

+

This specific test assumes that www.example.com is our Phorge.

+ +

http://www.example.com/

-

example.com

+

example.com

-

http://www.example.com/x/

+

http://www.example.com/x/

http://www.example.com/page/#anchor

Anchors

~~~~~~~~~~ +This specific test assumes that www.example.com is our Phorge. + http://www.example.com/ example.com diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-tel.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-tel.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-tel.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-tel.txt @@ -5,11 +5,11 @@ [[tel:18005555555]] ~~~~~~~~~~ -

call me

+

call me

-

call me

+

call me

-

18005555555

+

18005555555

~~~~~~~~~~ call me <18005555555> diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-punctuation.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-punctuation.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-punctuation.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-punctuation.txt @@ -2,8 +2,8 @@ http://www.example.com/.. http://www.example.com/!!! ~~~~~~~~~~ -

http://www.example.com/, -http://www.example.com/.. -http://www.example.com/!!!

+

http://www.example.com/, +http://www.example.com/.. +http://www.example.com/!!!

~~~~~~~~~~ http://www.example.com/, http://www.example.com/.. http://www.example.com/!!! diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-tilde.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-tilde.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-tilde.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link-with-tilde.txt @@ -1,5 +1,5 @@ http://www.example.com/~~ ~~~~~~~~~~ -

http://www.example.com/~

+

http://www.example.com/~

~~~~~~~~~~ http://www.example.com/~~ diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/link.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/link.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/link.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/link.txt @@ -1,5 +1,5 @@ http://www.example.com/ ~~~~~~~~~~ -

http://www.example.com/

+

http://www.example.com/

~~~~~~~~~~ http://www.example.com/ diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/percent-block-multiline.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/percent-block-multiline.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/percent-block-multiline.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/percent-block-multiline.txt @@ -10,7 +10,7 @@
- second
- third

-

world

+

world

~~~~~~~~~~ **foo** diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/simple-table-with-link.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/simple-table-with-link.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/simple-table-with-link.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/simple-table-with-link.txt @@ -1,7 +1,7 @@ | [[ http://example.com | name ]] | [x] | ~~~~~~~~~~
- +
name[x]
name[x]
~~~~~~~~~~ | name | [x] | diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt @@ -13,11 +13,11 @@
  • http://www.example.com
    • -

    link_name

    +

    link_name

    bold

    -

    http://www.example.com

    +

    http://www.example.com

    ~~~~~~~~~~ [[ http://www.example.com/ | link_name ]] ========================================= diff --git a/src/infrastructure/parser/PhutilURIHelper.php b/src/infrastructure/parser/PhutilURIHelper.php --- a/src/infrastructure/parser/PhutilURIHelper.php +++ b/src/infrastructure/parser/PhutilURIHelper.php @@ -18,6 +18,15 @@ */ private $phutilUri; + /** + * Extra trusted domains. + * This is useful because your Phorge may have some globally trusted domains, + * but you also can temporarily consider a domain as trusted, + * for example in a specific Remarkup engine instance. + * @var array The array key is the domain, the value is always '1'. + */ + private $extraTrustedDomains = []; + /** * @param string|PhutilURI $uri */ @@ -40,7 +49,7 @@ public function isSelf() { // The backend prefers a PhutilURI object, if available. $uri = $this->phutilUri ? $this->phutilUri : $this->uriStr; - return PhabricatorEnv::isSelfURI($uri); + return PhabricatorEnv::isSelfURI($uri) || $this->isExtraSelfURI(); } /** @@ -59,6 +68,25 @@ return $this->isStartingWithChar('/'); } + /** + * Add an extra trusted URI (not preferred). + * @param $uri PhutilURI Extra URI to be considered trusted. + * @return self + */ + public function addExtraTrustedURI(string $uri): PhutilURIHelper { + return $this->addExtraTrustedPhutilURI(new PhutilURI($uri)); + } + + /** + * Add an extra trusted URI (preferred). + * @param $uri PhutilURI Extra URI to be considered trusted. + * @return self + */ + public function addExtraTrustedPhutilURI(PhutilURI $uri): PhutilURIHelper { + $this->extraTrustedDomains[$uri->getDomain()] = 1; + return $this; + } + /** * A sane default. */ @@ -66,6 +94,31 @@ return $this->uriStr; } + /** + * Check whenever the specified URI is an "extra" self URI. + * @return bool + */ + private function isExtraSelfURI(): bool { + // Micro-optimization to do not always create a PhutilURI object. + if (!$this->extraTrustedDomains) { + return false; + } + + $domain = $this->getPhutilURI()->getDomain(); + return isset($this->extraTrustedDomains[$domain]); + } + + /** + * Get or create a PhutilURI object. + * @return PhutilURI + */ + private function getPhutilURI(): PhutilURI { + if (!$this->phutilUri) { + $this->phutilUri = new PhutilURI($this->uriStr); + } + return $this->phutilUri; + } + /** * Check whenever the URI starts with the provided character. * @param string $char String that MUST have length of 1. diff --git a/src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php b/src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php --- a/src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php +++ b/src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php @@ -3,6 +3,9 @@ final class PhutilURIHelperTestCase extends PhabricatorTestCase { public function testPhutilURIHelper() { + // Setup the test environment. + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('phabricator.base-uri', 'https://phorge.localhost'); // Every row is a test. Every column is: // - 0: name of the test @@ -20,16 +23,26 @@ array('internal root dir', '/#asd', true, false, true), array('external', 'https://gnu.org/', false, false, false), array('external anchor', 'https://gnu.org/#asd', false, false, false), + array('mail', 'mailto:info@wikipedia.org', false, false, false), + array('tel', 'tel:+555555555', false, false, false), + array('alien protocol', 'foo://whatever', false, false, false), + array('base uri', 'https://phorge.localhost', true, false, false), + array( + 'base uri anchor', + 'https://phorge.localhost/#asd', + true, + false, + false, + ), + array( + 'base uri path', + 'https://phorge.localhost/something', + true, + false, + false, + ), ); - // Add additional self-tests if base URI is available. - $base = PhabricatorEnv::getEnvConfigIfExists('phabricator.base-uri'); - if ($base) { - $domain = id(new PhutilURI($base))->getDomain(); - $tests[] = array('base uri', $base, true, false, false); - $tests[] = array('base uri anchor', "{$base}#asd", true, false, false); - } - foreach ($tests as $test) { $name = $test[0]; $uri = $test[1]; @@ -58,6 +71,10 @@ $this->assertEqual($is_slash, $uri->isStartingWithSlash(), pht('%s - is starting with slash', $test_name)); } + } + + // Clear the test environment. + unset($env); } }