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 @@
+<?php
+
+final class PhutilRemarkupRuleTestCase extends PhabricatorTestCase {
+
+ public function testIsURILikelyInternal() {
+ $tests = array(
+ // name, value, expected
+ array('anchor', '#asd', true),
+ array('relative dir', '/foo/', true),
+ array('root dir', '/', true),
+ array('external uri', 'https://gnu.org/', false),
+ );
+
+ // if the base URI is set, also try against ourself
+ $base_uri =
+ PhabricatorEnv::getEnvConfigIfExists('phabricator.base-uri', null);
+ if ($base_uri) {
+ // name, value, expected
+ $tests[] = array('base uri', $base_uri, true);
+ }
+
+ foreach ($tests as $test) {
+ $test_name = $test[0];
+ $test_value = $test[1];
+ $test_expected = $test[2];
+
+ // This method is just exposed to do unit tests here.
+ // Our goal is not to consider this method as stable.
+ $tested_value = PhutilRemarkupRule::isURILikelyInternal($test_value);
+
+ $this->assertEqual($tested_value, $test_expected, $test_name);
+ }
+ }
+}