Skip to content

Commit 190a5f8

Browse files
dsnetgriesemer
dsnet
authored and
griesemer
committed
go/doc: simplify and robustify link detection logic
To fix golang#5043, we added logic to allow balanced pairs of parenthesis so that we could match URLs like: http://example.com/some_resource(foo) Howewer, such logic breaks when parsing something like the following: art by [https://example.com/person][Person Name]]. such that the following is considered the link: https://example.com/person][Person Since the logic added in golang#5043 was just a heuristic, we adjust the heuristic that in addition to requiring balanced pairs, the first parenthesis must be an opening one. For further robustness, we apply this heuristic to parenthesis, braces, and brackets. Fixes golang#22285 Change-Id: I23b728a644e35ce3995b05a79129cad2c1e3b1ce Reviewed-on: https://go-review.googlesource.com/c/94876 Run-TryBot: Robert Griesemer <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent a629399 commit 190a5f8

2 files changed

Lines changed: 28 additions & 61 deletions

File tree

‎src/go/doc/comment.go‎

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const (
5454
identRx = `[\pL_][\pL_0-9]*`
5555

5656
// Regexp for URLs
57-
// Match parens, and check in pairedParensPrefixLen for balance - see #5043
57+
// Match parens, and check later for balance - see #5043, #22285
5858
// Match .,:;?! within path, but not at end - see #18139, #16565
5959
// This excludes some rare yet valid urls ending in common punctuation
6060
// in order to allow sentences ending in URLs.
@@ -86,29 +86,6 @@ var (
8686
html_endh = []byte("</h3>\n")
8787
)
8888

89-
// pairedParensPrefixLen returns the length of the longest prefix of s containing paired parentheses.
90-
func pairedParensPrefixLen(s string) int {
91-
parens := 0
92-
l := len(s)
93-
for i, ch := range s {
94-
switch ch {
95-
case '(':
96-
if parens == 0 {
97-
l = i
98-
}
99-
parens++
100-
case ')':
101-
parens--
102-
if parens == 0 {
103-
l = len(s)
104-
} else if parens < 0 {
105-
return i
106-
}
107-
}
108-
}
109-
return l
110-
}
111-
11289
// Emphasize and escape a line of text for HTML. URLs are converted into links;
11390
// if the URL also appears in the words map, the link is taken from the map (if
11491
// the corresponding map value is the empty string, the URL is not converted
@@ -128,13 +105,27 @@ func emphasize(w io.Writer, line string, words map[string]string, nice bool) {
128105
// write text before match
129106
commentEscape(w, line[0:m[0]], nice)
130107

131-
// adjust match if necessary
108+
// adjust match for URLs
132109
match := line[m[0]:m[1]]
133-
if n := pairedParensPrefixLen(match); n < len(match) {
134-
// match contains unpaired parentheses (rare);
135-
// redo matching with shortened line for correct indices
136-
m = matchRx.FindStringSubmatchIndex(line[:m[0]+n])
137-
match = match[:n]
110+
if strings.Contains(match, "://") {
111+
m0, m1 := m[0], m[1]
112+
for _, s := range []string{"()", "{}", "[]"} {
113+
open, close := s[:1], s[1:] // E.g., "(" and ")"
114+
// require opening parentheses before closing parentheses (#22285)
115+
if i := strings.Index(match, close); i >= 0 && i < strings.Index(match, open) {
116+
m1 = m0 + i
117+
match = line[m0:m1]
118+
}
119+
// require balanced pairs of parentheses (#5043)
120+
for i := 0; strings.Count(match, open) != strings.Count(match, close) && i < 10; i++ {
121+
m1 = strings.LastIndexAny(line[:m1], s)
122+
match = line[m0:m1]
123+
}
124+
}
125+
if m1 != m[1] {
126+
// redo matching with shortened line for correct indices
127+
m = matchRx.FindStringSubmatchIndex(line[:m[0]+len(match)])
128+
}
138129
}
139130

140131
// analyze match

‎src/go/doc/comment_test.go‎

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ func TestToText(t *testing.T) {
151151
var emphasizeTests = []struct {
152152
in, out string
153153
}{
154+
{"", ""},
154155
{"http://[::1]:8080/foo.txt", `<a href="http://[::1]:8080/foo.txt">http://[::1]:8080/foo.txt</a>`},
155156
{"before (https://www.google.com) after", `before (<a href="https://www.google.com">https://www.google.com</a>) after`},
156157
{"before https://www.google.com:30/x/y/z:b::c. After", `before <a href="https://www.google.com:30/x/y/z:b::c">https://www.google.com:30/x/y/z:b::c</a>. After`},
@@ -169,7 +170,13 @@ var emphasizeTests = []struct {
169170
{"Hello http://example.com/%2f/ /world.", `Hello <a href="http://example.com/%2f/">http://example.com/%2f/</a> /world.`},
170171
{"Lorem http: ipsum //host/path", "Lorem http: ipsum //host/path"},
171172
{"javascript://is/not/linked", "javascript://is/not/linked"},
173+
{"http://foo", `<a href="http://foo">http://foo</a>`},
174+
{"art by [[https://www.example.com/person/][Person Name]]", `art by [[<a href="https://www.example.com/person/">https://www.example.com/person/</a>][Person Name]]`},
175+
{"please visit (http://golang.org/)", `please visit (<a href="http://golang.org/">http://golang.org/</a>)`},
176+
{"please visit http://golang.org/hello())", `please visit <a href="http://golang.org/hello()">http://golang.org/hello()</a>)`},
172177
{"http://git.qemu.org/?p=qemu.git;a=blob;f=qapi-schema.json;hb=HEAD", `<a href="http://git.qemu.org/?p=qemu.git;a=blob;f=qapi-schema.json;hb=HEAD">http://git.qemu.org/?p=qemu.git;a=blob;f=qapi-schema.json;hb=HEAD</a>`},
178+
{"https://foo.bar/bal/x(])", `<a href="https://foo.bar/bal/x(">https://foo.bar/bal/x(</a>])`}, // inner ] causes (]) to be cut off from URL
179+
{"foo [ http://bar(])", `foo [ <a href="http://bar(">http://bar(</a>])`}, // outer [ causes ]) to be cut off from URL
173180
}
174181

175182
func TestEmphasize(t *testing.T) {
@@ -183,37 +190,6 @@ func TestEmphasize(t *testing.T) {
183190
}
184191
}
185192

186-
var pairedParensPrefixLenTests = []struct {
187-
in, out string
188-
}{
189-
{"", ""},
190-
{"foo", "foo"},
191-
{"()", "()"},
192-
{"foo()", "foo()"},
193-
{"foo()()()", "foo()()()"},
194-
{"foo()((()()))", "foo()((()()))"},
195-
{"foo()((()()))bar", "foo()((()()))bar"},
196-
{"foo)", "foo"},
197-
{"foo))", "foo"},
198-
{"foo)))))", "foo"},
199-
{"(foo", ""},
200-
{"((foo", ""},
201-
{"(((((foo", ""},
202-
{"(foo)", "(foo)"},
203-
{"((((foo))))", "((((foo))))"},
204-
{"foo()())", "foo()()"},
205-
{"foo((()())", "foo"},
206-
{"foo((()())) (() foo ", "foo((()())) "},
207-
}
208-
209-
func TestPairedParensPrefixLen(t *testing.T) {
210-
for i, tt := range pairedParensPrefixLenTests {
211-
if out := tt.in[:pairedParensPrefixLen(tt.in)]; out != tt.out {
212-
t.Errorf("#%d: mismatch\nhave: %q\nwant: %q", i, out, tt.out)
213-
}
214-
}
215-
}
216-
217193
func TestCommentEscape(t *testing.T) {
218194
commentTests := []struct {
219195
in, out string

0 commit comments

Comments
 (0)