Skip to content

Commit deb6130

Browse files
h9jianggopherbot
authored andcommitted
gopls/internal/golang: fix hover panic in raw strings with CRLF
Because raw string literals discard '\r', using the physical file offset to index the parsed string caused out-of-bounds panics on files with Windows line endings. Fixes golang/go#77675 Change-Id: Iaf348c37eceb3a8da70df26c6aa6998d0696d57e Reviewed-on: https://go-review.googlesource.com/c/tools/+/748322 Reviewed-by: Alan Donovan <adonovan@google.com> Auto-Submit: Hongxiang Jiang <hxjiang@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 5f1186b commit deb6130

5 files changed

Lines changed: 214 additions & 41 deletions

File tree

gopls/internal/golang/hover.go

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,42 +1056,32 @@ func hoverConstantExpr(pgf *parsego.File, expr ast.Expr, tv types.TypeAndValue,
10561056
}
10571057
}
10581058
case constant.String:
1059-
// Locate the unicode escape sequence under the current cursor
1060-
// position.
1061-
litOffset, err := safetoken.Offset(pgf.Tok, lit.Pos())
1059+
index, err := astutil.OffsetInStringLiteral(lit, posRange.Pos())
10621060
if err != nil {
10631061
return protocol.Range{}, nil, err
10641062
}
1065-
startOffset, err := safetoken.Offset(pgf.Tok, posRange.Pos())
1063+
// pos is the point in the literal where the current rune starts.
1064+
pos, err := astutil.PosInStringLiteral(lit, index)
10661065
if err != nil {
10671066
return protocol.Range{}, nil, err
10681067
}
1069-
for i := startOffset - litOffset; i > 0; i-- {
1070-
// Start at the cursor position and search backward for the beginning of a rune escape sequence.
1071-
rr, _ := utf8.DecodeRuneInString(lit.Value[i:])
1072-
if rr == utf8.RuneError {
1073-
return protocol.Range{}, nil, fmt.Errorf("rune error")
1074-
}
1075-
if rr == '\\' {
1076-
// Got the beginning, decode it.
1077-
rr, _, tail, err := strconv.UnquoteChar(lit.Value[i:], '"')
1078-
if err != nil {
1079-
// If the conversion fails, it's because of an invalid
1080-
// syntax, therefore is no rune to be found.
1081-
return protocol.Range{}, nil, nil
1082-
}
1083-
// Only the rune escape sequence part of the string has to
1084-
// be highlighted, recompute the range.
1085-
runeLen := len(lit.Value) - (i + len(tail))
1086-
pStart := token.Pos(int(lit.Pos()) + i)
1087-
pEnd := token.Pos(int(pStart) + runeLen)
1088-
1089-
if pEnd >= posRange.End() {
1090-
start, end = pStart, pEnd
1091-
r = rr
1092-
}
1093-
break
1068+
1069+
rest, err := pgf.PosText(pos, lit.End())
1070+
if err != nil {
1071+
return protocol.Range{}, nil, err
1072+
}
1073+
1074+
// Is the selected character denoted by an escape sequence?
1075+
if bytes.HasPrefix(rest, []byte(`\`)) {
1076+
rr, _, tail, err := strconv.UnquoteChar(string(rest), lit.Value[0])
1077+
if err != nil {
1078+
return protocol.Range{}, nil, err
10941079
}
1080+
1081+
rawSize := len(rest) - len(tail)
1082+
1083+
start, end = pos, pos+token.Pos(rawSize)
1084+
r = rr
10951085
}
10961086
default:
10971087
panic("unexpected constant.Kind")
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright 2026 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package hover
6+
7+
import (
8+
"os"
9+
"testing"
10+
11+
. "golang.org/x/tools/gopls/internal/test/integration"
12+
"golang.org/x/tools/gopls/internal/util/bug"
13+
)
14+
15+
func TestMain(m *testing.M) {
16+
bug.PanicOnBugs = true
17+
os.Exit(Main(m))
18+
}
19+
20+
// TestIssue77675 verifies that hovering over a raw string literal containing
21+
// Windows line endings (\r\n) does not cause an out-of-bounds panic.
22+
func TestIssue77675(t *testing.T) {
23+
const src = `
24+
-- go.mod --
25+
module mod.com
26+
27+
go 1.20
28+
29+
-- main.go --
30+
package main
31+
32+
func _() {
33+
_ = ` + "`" + `foo
34+
35+
bar
36+
baz` + "`" + `
37+
}
38+
`
39+
WithOptions(
40+
WindowsLineEndings(),
41+
).Run(t, src, func(t *testing.T, env *Env) {
42+
env.OpenFile("main.go")
43+
env.Await(env.DoneWithOpen())
44+
loc := env.RegexpSearch("main.go", "baz()")
45+
content, l := env.Hover(loc)
46+
if !l.Empty() {
47+
t.Errorf("hover expect empty range got: %v", l)
48+
}
49+
if content != nil {
50+
t.Errorf("hover expect empty result got: %v", content)
51+
}
52+
})
53+
}

gopls/internal/test/marker/testdata/hover/basiclit.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ func _() {
1313
_ = 0x61 //@hover("0x61", "0x61", latinAHex)
1414

1515
_ = '\u2211' //@hover("'\\u2211'", "'\\u2211'", summation)
16+
_ = '\u2211' //@hover(re"u22()11", "'\\u2211'", summation)
1617
_ = 0x2211 //@hover("0x2211", "0x2211", summationHex)
1718
_ = "foo \u2211 bar" //@hover("\\u2211", "\\u2211", summation)
1819

internal/astutil/stringlit.go

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,30 +40,64 @@ func PosInStringLiteral(lit *ast.BasicLit, offset int) (token.Pos, error) {
4040
return 0, fmt.Errorf("invalid offset")
4141
}
4242

43-
// Did the scanner normalize one or more carriage
44-
// returns (\r) in a raw string literal?
43+
pos, _ := walkStringLiteral(lit, lit.End(), offset)
44+
return pos, nil
45+
}
46+
47+
// OffsetInStringLiteral returns the byte offset within the logical (unquoted)
48+
// string corresponding to the specified source position.
49+
func OffsetInStringLiteral(lit *ast.BasicLit, pos token.Pos) (int, error) {
50+
if !NodeContainsPos(lit, pos) {
51+
return 0, fmt.Errorf("invalid position")
52+
}
53+
54+
raw := lit.Value
55+
56+
value, err := strconv.Unquote(raw)
57+
if err != nil {
58+
return 0, err
59+
}
60+
61+
_, offset := walkStringLiteral(lit, pos, len(value))
62+
return offset, nil
63+
}
64+
65+
// walkStringLiteral iterates through the raw string literal to map between
66+
// a file position and a logical byte offset. It stops when it reaches
67+
// either the targetPos or the targetOffset.
68+
//
69+
// TODO(hxjiang): consider making an iterator.
70+
func walkStringLiteral(lit *ast.BasicLit, targetPos token.Pos, targetOffset int) (token.Pos, int) {
71+
raw := lit.Value
4572
norm := int(lit.End()-lit.Pos()) > len(lit.Value)
4673

4774
// remove quotes
4875
quote := raw[0] // '"' or '`'
4976
raw = raw[1 : len(raw)-1]
5077

5178
var (
52-
i = 0 // byte index within logical value
53-
pos = lit.ValuePos + 1 // position within literal
79+
i = 0 // byte index within logical value
80+
pos = lit.Pos() + 1 // position within literal
5481
)
55-
for raw != "" && i < offset {
82+
83+
for raw != "" {
5684
r, _, rest, _ := strconv.UnquoteChar(raw, quote) // can't fail
5785
sz := len(raw) - len(rest) // length of literal char in raw bytes
58-
pos += token.Pos(sz)
59-
// If any \r normalization occurred,
60-
// assume each \n was preceded by \r.
61-
// (This is just a heuristic.)
86+
87+
nextPos := pos + token.Pos(sz)
6288
if norm && r == '\n' {
63-
pos++
89+
nextPos++
6490
}
91+
nextI := i + utf8.RuneLen(r) // length of logical char in "cooked" bytes
92+
93+
if nextPos > targetPos || nextI > targetOffset {
94+
break
95+
}
96+
6597
raw = raw[sz:]
66-
i += utf8.RuneLen(r)
98+
i = nextI
99+
pos = nextPos
67100
}
68-
return pos, nil
101+
102+
return pos, i
69103
}

internal/astutil/stringlit_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,98 @@ func TestPosInStringLiteral(t *testing.T) {
9898
}
9999
}
100100
}
101+
102+
func TestOffsetInStringLiteral(t *testing.T) {
103+
// Each string is Go source for a string literal with | marker at expected
104+
// physical pos, and $ marker at the expected logical unquoted index.
105+
tests := []string{
106+
`"$|abc"`,
107+
`"a$|bc"`,
108+
`"ab$|c"`,
109+
`"abc$|"`,
110+
`"a\n$|b"`,
111+
`"\n$|"`,
112+
`"a\000$|"`,
113+
`"\x61$|"`,
114+
`"\u0061$|"`,
115+
`"\U00000061$|"`,
116+
`"$\U00|000061"`,
117+
`"€$|"`,
118+
`"a€$|b"`,
119+
"`abc$|`",
120+
"`a\n$|b`",
121+
// normalization of \r carriage returns:
122+
"`a\r\n$|b`",
123+
"`a\r\nb\r\nc\r\n$|d`",
124+
}
125+
for _, test := range tests {
126+
// The workaround for \r requires the go1.26 fix for https://go.dev/issue/76031.
127+
if strings.Contains(test, "\r") && testenv.Go1Point() < 26 {
128+
continue
129+
}
130+
131+
t.Logf("input: %#q", test)
132+
133+
const prefix = "package p; const _ = "
134+
src := prefix + test
135+
136+
// Remove the physical marker to produce valid Go source.
137+
offset := strings.Index(src, "|")
138+
if offset < 0 {
139+
t.Errorf("Source %q contains no marker", src)
140+
continue
141+
}
142+
src = src[:offset] + src[offset+1:]
143+
144+
// Parse.
145+
fset := token.NewFileSet()
146+
f, err := parser.ParseFile(fset, "p.go", src, 0)
147+
if err != nil {
148+
t.Errorf("Parse: %v", err)
149+
continue
150+
}
151+
152+
// Find literal.
153+
var lit *ast.BasicLit
154+
ast.Inspect(f, func(n ast.Node) bool {
155+
if b, ok := n.(*ast.BasicLit); ok {
156+
lit = b
157+
return false
158+
}
159+
return true
160+
})
161+
if lit == nil {
162+
t.Errorf("No literal")
163+
continue
164+
}
165+
166+
tfile := fset.File(f.Pos())
167+
pos := tfile.Pos(offset)
168+
169+
// Convert file position to logical index.
170+
index, err := astutil.OffsetInStringLiteral(lit, pos)
171+
if err != nil {
172+
t.Errorf("OffsetInStringLiteral(%d): %v", pos, err)
173+
continue
174+
}
175+
176+
value, err := strconv.Unquote(lit.Value)
177+
if err != nil {
178+
t.Errorf("Unquote: %v", err)
179+
continue
180+
}
181+
182+
if index < 0 || index > len(value) {
183+
t.Errorf("Returned index %d is out of bounds for logical string of length %d", index, len(value))
184+
continue
185+
}
186+
187+
// Check that cut offset in value is before marker.
188+
before, after := value[:index], value[index:]
189+
t.Logf("\t%q :: %q", before, after)
190+
if !strings.HasSuffix(before, "$") {
191+
t.Errorf("no marker at logical cut point, got before=%q", before)
192+
continue
193+
}
194+
}
195+
}

0 commit comments

Comments
 (0)