Skip to content

Commit a815d2d

Browse files
author
epriestley
committed
Don't detect long sentences which happen to have a colon in them as Git URIs
Summary: Some very liberal code is currently trying to parse commit summaries aggressively as Git URIs, and leading to this: > Failed to parse URI "Summary: Ref T11137. This addresses three general issues:" as a Git URI. Currently visible here: > https://secure.phabricator.com/rPHU8bb124c37de3dfa29fcd23d4be53a5696b705c81 Don't detect these as Git URIs. Test Plan: Unit tests. Reviewers: chad Reviewed By: chad Subscribers: 20after4 Maniphest Tasks: T11137 Differential Revision: https://secure.phabricator.com/D16103
1 parent 8bb124c commit a815d2d

File tree

2 files changed

+11
-1
lines changed

2 files changed

+11
-1
lines changed

src/parser/PhutilURI.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function __construct($uri) {
6161
$host = '(?P<host>[^/:]+)';
6262
$path = ':(?P<path>.*)';
6363

64-
$ok = preg_match('(^\s*'.$user.$host.$path.'\z)', $uri, $matches);
64+
$ok = preg_match('(^'.$user.$host.$path.'\z)', $uri, $matches);
6565
if (!$ok) {
6666
throw new Exception(
6767
pht(
@@ -344,6 +344,12 @@ private function isGitURIPattern($uri) {
344344
$head = $matches['head'];
345345
$last = $matches['last'];
346346

347+
// If any part of this has spaces in it, it's not a Git URI. We fail here
348+
// so we fall back and don't fail more abruptly later.
349+
if (preg_match('(\s)', $head.$last)) {
350+
return false;
351+
}
352+
347353
// If the first part has a "." or an "@" in it, interpret it as a domain
348354
// or a "user@host" string.
349355
if (preg_match('([.@])', $head)) {

src/parser/__tests__/PhutilURITestCase.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,10 @@ public function testUnusualURIs() {
151151
'x' => '/',
152152
),
153153
$uri->getQueryParams());
154+
155+
// This is not a legitimate URI and should not parse as one.
156+
$uri = new PhutilURI('fruit.list: apple banana cherry');
157+
$this->assertEqual('', $uri->getDomain());
154158
}
155159

156160
public function testAmbiguousURIs() {

0 commit comments

Comments
 (0)