Skip to content

Commit 209f171

Browse files
author
Christian Weichel
committed
Make sourcemaps more resilient against out-of-range modifications
Fixes #6
1 parent 4f4e489 commit 209f171

File tree

4 files changed

+235
-45
lines changed

4 files changed

+235
-45
lines changed

Diff for: ‎.gitpod.setup.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ git clone https://github.com/bcmi-labs/arduino-editor
99
cd arduino-editor
1010
yarn
1111

12-
echo "start an Arduino IDE with: yarn --cwd /workspace/arduino-editor/browser-app start"
12+
echo "starting an Arduino IDE with: yarn --cwd /workspace/arduino-editor/browser-app start"
13+
yarn --cwd /workspace/arduino-editor/browser-app start

Diff for: ‎handler/handler.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -312,15 +312,18 @@ func (handler *InoHandler) createFileData(ctx context.Context, sourceURI lsp.Doc
312312
return data, targetBytes, nil
313313
}
314314

315-
func (handler *InoHandler) updateFileData(data *FileData, change *lsp.TextDocumentContentChangeEvent) error {
315+
func (handler *InoHandler) updateFileData(data *FileData, change *lsp.TextDocumentContentChangeEvent) (err error) {
316316
rang := change.Range
317317
if rang == nil || rang.Start.Line != rang.End.Line {
318318
// Update the source text and regenerate the cpp code
319319
var newSourceText string
320320
if rang == nil {
321321
newSourceText = change.Text
322322
} else {
323-
newSourceText = applyTextChange(data.sourceText, *rang, change.Text)
323+
newSourceText, err = applyTextChange(data.sourceText, *rang, change.Text)
324+
if err != nil {
325+
return err
326+
}
324327
}
325328
targetBytes, err := updateCpp([]byte(newSourceText), uriToPath(data.sourceURI), handler.config.SelectedBoard.Fqbn, false, uriToPath(data.targetURI))
326329
if err != nil {
@@ -353,7 +356,10 @@ func (handler *InoHandler) updateFileData(data *FileData, change *lsp.TextDocume
353356
} else {
354357
// Apply an update to a single line both to the source and the target text
355358
targetLine := data.targetLineMap[rang.Start.Line]
356-
data.sourceText = applyTextChange(data.sourceText, *rang, change.Text)
359+
data.sourceText, err = applyTextChange(data.sourceText, *rang, change.Text)
360+
if err != nil {
361+
return err
362+
}
357363
updateSourceMaps(data.sourceLineMap, data.targetLineMap, 0, rang.Start.Line, change.Text)
358364

359365
rang.Start.Line = targetLine

Diff for: ‎handler/sourcemap.go

+78-20
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package handler
22

33
import (
44
"bufio"
5+
"fmt"
56
"io"
67
"strconv"
78
"strings"
@@ -113,34 +114,91 @@ func copyMappings(sourceLineMap, targetLineMap, newMappings map[int]int) {
113114
}
114115
}
115116

116-
func applyTextChange(text string, rang lsp.Range, insertText string) string {
117-
if rang.Start.Line != rang.End.Line {
118-
startOffset := getLineOffset(text, rang.Start.Line) + rang.Start.Character
119-
endOffset := getLineOffset(text, rang.End.Line) + rang.End.Character
120-
return text[:startOffset] + insertText + text[endOffset:]
121-
} else if rang.Start.Character != rang.End.Character {
122-
lineOffset := getLineOffset(text, rang.Start.Line)
123-
startOffset := lineOffset + rang.Start.Character
124-
endOffset := lineOffset + rang.End.Character
125-
return text[:startOffset] + insertText + text[endOffset:]
126-
} else {
127-
offset := getLineOffset(text, rang.Start.Line) + rang.Start.Character
128-
return text[:offset] + insertText + text[offset:]
117+
// OutOfRangeError returned if one attempts to access text out of its range
118+
type OutOfRangeError struct {
119+
Max int
120+
Req lsp.Position
121+
}
122+
123+
func (oor OutOfRangeError) Error() string {
124+
return fmt.Sprintf("text access out of range: max=%d requested=%d", oor.Max, oor.Req)
125+
}
126+
127+
func applyTextChange(text string, rang lsp.Range, insertText string) (res string, err error) {
128+
start, err := getOffset(text, rang.Start)
129+
if err != nil {
130+
return "", err
131+
}
132+
end, err := getOffset(text, rang.End)
133+
if err != nil {
134+
return "", err
135+
}
136+
137+
return text[:start] + insertText + text[end:], nil
138+
}
139+
140+
// getOffset computes the offset in the text expressed by the lsp.Position.
141+
// Returns OutOfRangeError if the position is out of range.
142+
func getOffset(text string, pos lsp.Position) (off int, err error) {
143+
// find line
144+
lineOffset := getLineOffset(text, pos.Line)
145+
if lineOffset < 0 {
146+
return -1, OutOfRangeError{len(text), pos}
147+
}
148+
off = lineOffset
149+
150+
// walk towards the character
151+
var charFound bool
152+
for offset, c := range text[off:] {
153+
if c == '\n' {
154+
// We've reached the end of line. LSP spec says we should default back to the line length.
155+
// See https://microsoft.github.io/language-server-protocol/specifications/specification-3-14/#position
156+
off += offset
157+
charFound = true
158+
break
159+
}
160+
161+
// we've fond the character
162+
if offset == pos.Character {
163+
off += offset
164+
charFound = true
165+
break
166+
}
129167
}
168+
if !charFound {
169+
return -1, OutOfRangeError{Max: len(text), Req: pos}
170+
}
171+
172+
return off, nil
130173
}
131174

175+
// getLineOffset finds the offset/position of the beginning of a line within the text.
176+
// For example:
177+
// text := "foo\nfoobar\nbaz"
178+
// getLineOffset(text, 0) == 0
179+
// getLineOffset(text, 1) == 4
180+
// getLineOffset(text, 2) == 11
132181
func getLineOffset(text string, line int) int {
182+
if line < 0 {
183+
return -1
184+
}
133185
if line == 0 {
134186
return 0
135187
}
136-
count := 0
188+
189+
// find the line and return its offset within the text
190+
var count int
137191
for offset, c := range text {
138-
if c == '\n' {
139-
count++
140-
if count == line {
141-
return offset + 1
142-
}
192+
if c != '\n' {
193+
continue
194+
}
195+
196+
count++
197+
if count == line {
198+
return offset + 1
143199
}
144200
}
145-
return len(text)
201+
202+
// we didn't find the line in the text
203+
return -1
146204
}

Diff for: ‎handler/sourcemap_test.go

+146-21
Original file line numberDiff line numberDiff line change
@@ -72,32 +72,157 @@ func TestUpdateSourceMaps2(t *testing.T) {
7272
}
7373

7474
func TestApplyTextChange(t *testing.T) {
75-
text1 := applyTextChange("foo\nbar\nbaz\n!", lsp.Range{
76-
Start: lsp.Position{Line: 1, Character: 1},
77-
End: lsp.Position{Line: 2, Character: 2},
78-
}, "i")
79-
if text1 != "foo\nbiz\n!" {
80-
t.Error(text1)
75+
tests := []struct {
76+
InitialText string
77+
Range lsp.Range
78+
Insertion string
79+
Expectation string
80+
Err error
81+
}{
82+
{
83+
"foo\nbar\nbaz\n!",
84+
lsp.Range{
85+
Start: lsp.Position{Line: 1, Character: 1},
86+
End: lsp.Position{Line: 2, Character: 2},
87+
},
88+
"i",
89+
"foo\nbiz\n!",
90+
nil,
91+
},
92+
{
93+
"foo\nbar\nbaz\n!",
94+
lsp.Range{
95+
Start: lsp.Position{Line: 1, Character: 1},
96+
End: lsp.Position{Line: 1, Character: 2},
97+
},
98+
"ee",
99+
"foo\nbeer\nbaz\n!",
100+
nil,
101+
},
102+
{
103+
"foo\nbar\nbaz\n!",
104+
lsp.Range{
105+
Start: lsp.Position{Line: 1, Character: 1},
106+
End: lsp.Position{Line: 1, Character: 1},
107+
},
108+
"eer from the st",
109+
"foo\nbeer from the star\nbaz\n!",
110+
nil,
111+
},
112+
{
113+
"foo\nbar\nbaz\n!",
114+
lsp.Range{
115+
Start: lsp.Position{Line: 0, Character: 10},
116+
End: lsp.Position{Line: 2, Character: 20},
117+
},
118+
"i",
119+
"fooi\n!",
120+
nil,
121+
},
122+
{
123+
"foo\nbar\nbaz\n!",
124+
lsp.Range{
125+
// out of range start offset
126+
Start: lsp.Position{Line: 0, Character: 100},
127+
End: lsp.Position{Line: 2, Character: 0},
128+
},
129+
"i",
130+
"fooibaz\n!",
131+
nil,
132+
},
133+
{
134+
"foo\nbar\nbaz\n!",
135+
lsp.Range{
136+
// out of range start offset
137+
Start: lsp.Position{Line: 20, Character: 0},
138+
End: lsp.Position{Line: 2, Character: 0},
139+
},
140+
"i",
141+
"",
142+
OutOfRangeError{13, lsp.Position{Line: 20, Character: 0}},
143+
},
144+
{
145+
"foo\nbar\nbaz\n!",
146+
lsp.Range{
147+
// out of range start offset
148+
Start: lsp.Position{Line: 0, Character: 0},
149+
End: lsp.Position{Line: 20, Character: 0},
150+
},
151+
"i",
152+
"",
153+
OutOfRangeError{13, lsp.Position{Line: 20, Character: 0}},
154+
},
81155
}
82-
text2 := applyTextChange("foo\nbar\nbaz\n!", lsp.Range{
83-
Start: lsp.Position{Line: 1, Character: 1},
84-
End: lsp.Position{Line: 1, Character: 2},
85-
}, "ee")
86-
if text2 != "foo\nbeer\nbaz\n!" {
87-
t.Error(text2)
156+
157+
for _, test := range tests {
158+
initial := strings.ReplaceAll(test.InitialText, "\n", "\\n")
159+
insertion := strings.ReplaceAll(test.Insertion, "\n", "\\n")
160+
expectation := strings.ReplaceAll(test.Expectation, "\n", "\\n")
161+
162+
t.Logf("applyTextChange(\"%s\", %v, \"%s\") == \"%s\"", initial, test.Range, insertion, expectation)
163+
act, err := applyTextChange(test.InitialText, test.Range, test.Insertion)
164+
if act != test.Expectation {
165+
t.Errorf("applyTextChange(\"%s\", %v, \"%s\") != \"%s\", got \"%s\"", initial, test.Range, insertion, expectation, strings.ReplaceAll(act, "\n", "\\n"))
166+
}
167+
if err != test.Err {
168+
t.Errorf("applyTextChange(\"%s\", %v, \"%s\") error != %v, got %v instead", initial, test.Range, insertion, test.Err, err)
169+
}
170+
}
171+
}
172+
173+
func TestGetOffset(t *testing.T) {
174+
tests := []struct {
175+
Text string
176+
Line int
177+
Char int
178+
Exp int
179+
Err error
180+
}{
181+
{"foo\nfoobar\nbaz", 0, 0, 0, nil},
182+
{"foo\nfoobar\nbaz", 1, 0, 4, nil},
183+
{"foo\nfoobar\nbaz", 1, 3, 7, nil},
184+
{"foo\nba\nr\nbaz\n!", 3, 0, 9, nil},
185+
{"foo\nba\nr\nbaz\n!", 1, 10, 6, nil},
186+
{"foo\nba\nr\nbaz\n!", -1, 0, -1, OutOfRangeError{14, lsp.Position{Line: -1, Character: 0}}},
187+
{"foo\nba\nr\nbaz\n!", 4, 20, -1, OutOfRangeError{14, lsp.Position{Line: 4, Character: 20}}},
88188
}
89-
text3 := applyTextChange("foo\nbar\nbaz\n!", lsp.Range{
90-
Start: lsp.Position{Line: 1, Character: 1},
91-
End: lsp.Position{Line: 1, Character: 1},
92-
}, "eer from the st")
93-
if text3 != "foo\nbeer from the star\nbaz\n!" {
94-
t.Error(text3)
189+
190+
for _, test := range tests {
191+
st := strings.Replace(test.Text, "\n", "\\n", -1)
192+
193+
t.Logf("getOffset(\"%s\", {Line: %d, Character: %d}) == %d", st, test.Line, test.Char, test.Exp)
194+
act, err := getOffset(test.Text, lsp.Position{Line: test.Line, Character: test.Char})
195+
if act != test.Exp {
196+
t.Errorf("getOffset(\"%s\", {Line: %d, Character: %d}) != %d, got %d instead", st, test.Line, test.Char, test.Exp, act)
197+
}
198+
if err != test.Err {
199+
t.Errorf("getOffset(\"%s\", {Line: %d, Character: %d}) error != %v, got %v instead", st, test.Line, test.Char, test.Err, err)
200+
}
95201
}
96202
}
97203

98204
func TestGetLineOffset(t *testing.T) {
99-
offset := getLineOffset("foo\nba\nr\nbaz\n!", 3)
100-
if offset != 9 {
101-
t.Error(offset)
205+
tests := []struct {
206+
Text string
207+
Line int
208+
Offset int
209+
}{
210+
{"foo\nfoobar\nbaz", 0, 0},
211+
{"foo\nfoobar\nbaz", 1, 4},
212+
{"foo\nfoobar\nbaz", 2, 11},
213+
{"foo\nfoobar\nbaz", 3, -1},
214+
{"foo\nba\nr\nbaz\n!", 3, 9},
215+
{"foo\nba\nr\nbaz\n!", -1, -1},
216+
{"foo\nba\nr\nbaz\n!", 20, -1},
217+
}
218+
219+
for _, test := range tests {
220+
st := strings.Replace(test.Text, "\n", "\\n", -1)
221+
222+
t.Logf("getLineOffset(\"%s\", %d) == %d", st, test.Line, test.Offset)
223+
act := getLineOffset(test.Text, test.Line)
224+
if act != test.Offset {
225+
t.Errorf("getLineOffset(\"%s\", %d) != %d, got %d instead", st, test.Line, test.Offset, act)
226+
}
102227
}
103228
}

0 commit comments

Comments
 (0)