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 @@ -5776,6 +5776,7 @@ 'PhutilRemarkupQuotesBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupQuotesBlockRule.php', 'PhutilRemarkupReplyBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupReplyBlockRule.php', 'PhutilRemarkupRule' => 'infrastructure/markup/markuprule/PhutilRemarkupRule.php', + 'PhutilRemarkupRuleTestCase' => 'infrastructure/markup/markuprule/__tests__/PhutilRemarkupRuleTestCase.php', 'PhutilRemarkupSimpleTableBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupSimpleTableBlockRule.php', 'PhutilRemarkupTableBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php', 'PhutilRemarkupTestInterpreterRule' => 'infrastructure/markup/blockrule/PhutilRemarkupTestInterpreterRule.php', @@ -12678,6 +12679,7 @@ 'PhutilRemarkupQuotesBlockRule' => 'PhutilRemarkupQuotedBlockRule', 'PhutilRemarkupReplyBlockRule' => 'PhutilRemarkupQuotedBlockRule', 'PhutilRemarkupRule' => 'Phobject', + 'PhutilRemarkupRuleTestCase' => 'PhabricatorTestCase', 'PhutilRemarkupSimpleTableBlockRule' => 'PhutilRemarkupBlockRule', 'PhutilRemarkupTableBlockRule' => 'PhutilRemarkupBlockRule', 'PhutilRemarkupTestInterpreterRule' => 'PhutilRemarkupBlockInterpreter', 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 @@ -45,11 +45,11 @@ $engine = $this->getEngine(); $is_anchor = false; - if (strncmp($link, '/', 1) == 0) { + if (self::isURIStartingWithSlash($link)) { $base = phutil_string_cast($engine->getConfig('uri.base')); $base = rtrim($base, '/'); $link = $base.$link; - } else if (strncmp($link, '#', 1) == 0) { + } else if (self::isURIStartingWithAnchor($link)) { $here = $engine->getConfig('uri.here'); $link = $here.$link; @@ -76,7 +76,8 @@ return $name; } - $same_window = $engine->getConfig('uri.same-window', false); + $is_internal = self::isURILikelyInternal($link); + $same_window = $engine->getConfig('uri.same-window', $is_internal); if ($same_window) { $target = null; } else { @@ -88,11 +89,18 @@ $target = null; } + // Allow developers to style esternal links differently + $classes = array('remarkup-link'); + if (!$is_internal) { + $classes[] = 'remarkup-link-ext'; + } + $classes = implode(' ', $classes); + return phutil_tag( 'a', array( 'href' => $link, - 'class' => 'remarkup-link', + 'class' => $classes, 'target' => $target, 'rel' => 'noreferrer', ), diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php --- a/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php +++ b/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php @@ -116,18 +116,26 @@ $engine = $this->getEngine(); - $same_window = $engine->getConfig('uri.same-window', false); + $is_internal = self::isURIAbsoluteInternal($link); + $same_window = $engine->getConfig('uri.same-window', $is_internal); if ($same_window) { $target = null; } else { $target = '_blank'; } + // Allow web designers to style external links differently + $classes = array('remarkup-link'); + if (!$is_internal) { + $classes[] = 'remarkup-link-ext'; + } + $classes = implode(' ', $classes); + return phutil_tag( 'a', array( 'href' => $link, - 'class' => 'remarkup-link', + 'class' => $classes, 'target' => $target, 'rel' => 'noreferrer', ), diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php --- a/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php +++ b/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php @@ -106,4 +106,70 @@ return (strpos($text, PhutilRemarkupBlockStorage::MAGIC_BYTE) === false); } + /** + * Check if a generic URI points to an internal destination. + * This is NOT intended to cover all cases: this was designed + * just to guess whenever we should avoid to make a target="_blank". + * The URI can be relative or absolute. + * WARNING: This method is public just to make unit tests, so, + * don't adopt this method in production externally since it can + * change anytime. + * https://we.phorge.it/T15182 + * + * @param string $uri Example '/bar/' or 'http://example.com/bar/' + * @return bool + */ + public static function isURILikelyInternal($uri) { + return self::isURIStartingWithAnchor($uri) + || self::isURIStartingWithSlash($uri) + || self::isURIAbsoluteInternal($uri); + } + + /** + * Check whenever an URI is starting with an anchor. + * + * @param string $uri + * @return bool + */ + protected static function isURIStartingWithAnchor($uri) { + return strncmp($uri, '#', 1) === 0; + } + + /** + * Check whenever an URI in a Remarkup starts with a slash. + * + * @param string $uri Example '/path' etc. + * @return bool + */ + protected static function isURIStartingWithSlash($uri) { + return strncmp($uri, '/', 1) === 0; + } + + /** + * Check if an absolute URI points to Phorge itself. + * This is a simple compromise between performance and a sane logic. + * At the moment we don't read the crystal ball, so these are + * considered different websites: + * - http://example.com (the input) + * - https://example.com (the base-uri) + * @param string $uri Example 'http://example.com/bar/' + * @return bool + */ + protected static function isURIAbsoluteInternal($uri) { + + // Get the Base URI - full URI with protocol and ending with '/' + // It's difficult to imagine a case where this config is not defined, + // but better to be nice here. + $base_uri = + PhabricatorEnv::getEnvConfigIfExists('phabricator.base-uri', null); + + // Assume a safe default. + if (!$base_uri) { + return false; + } + + // Check if the URI starts with the base URI. + return strncmp($uri, $base_uri, strlen($base_uri)) === 0; + } + } diff --git a/src/infrastructure/markup/markuprule/__tests__/PhutilRemarkupRuleTestCase.php b/src/infrastructure/markup/markuprule/__tests__/PhutilRemarkupRuleTestCase.php new file mode 100644 --- /dev/null +++ b/src/infrastructure/markup/markuprule/__tests__/PhutilRemarkupRuleTestCase.php @@ -0,0 +1,34 @@ +assertEqual($tested_value, $test_expected, $test_name); + } + } +}