Skip to content

Commit 14846fb

Browse files
devnevkevinburke
authored andcommitted
all: rewrite the lexer to consume the entire input first
Previously we used the buffruneio package to buffer input. However, the error handling was not good, and we would often panic when parsing inputs. SSH config files are generally not large, on the order of kilobytes or megabytes, and it's fine to just read the entire thing into memory and then parse from there. This also simplifies the parser significantly and lets us remove a dependency and several defer calls. Add a test that panicked with the old version and then modify the code to ensure the test no longer panics. Thanks to Mark Nevill (@devnev) for the initial error report and failing test case. Fixes kevinburke#10. Fixes kevinburke#24.
1 parent 2e50c44 commit 14846fb

File tree

5 files changed

+60
-28
lines changed

5 files changed

+60
-28
lines changed

.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-
/bazel-*

config.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"errors"
3535
"fmt"
3636
"io"
37+
"io/ioutil"
3738
"os"
3839
osuser "os/user"
3940
"path/filepath"
@@ -195,26 +196,29 @@ func parseFile(filename string) (*Config, error) {
195196
}
196197

197198
func parseWithDepth(filename string, depth uint8) (*Config, error) {
198-
f, err := os.Open(filename)
199+
b, err := ioutil.ReadFile(filename)
199200
if err != nil {
200201
return nil, err
201202
}
202-
defer f.Close()
203-
return decode(f, isSystem(filename), depth)
203+
return decodeBytes(b, isSystem(filename), depth)
204204
}
205205

206206
func isSystem(filename string) bool {
207-
// TODO i'm not sure this is the best way to detect a system repo
207+
// TODO: not sure this is the best way to detect a system repo
208208
return strings.HasPrefix(filepath.Clean(filename), "/etc/ssh")
209209
}
210210

211211
// Decode reads r into a Config, or returns an error if r could not be parsed as
212212
// an SSH config file.
213213
func Decode(r io.Reader) (*Config, error) {
214-
return decode(r, false, 0)
214+
b, err := ioutil.ReadAll(r)
215+
if err != nil {
216+
return nil, err
217+
}
218+
return decodeBytes(b, false, 0)
215219
}
216220

217-
func decode(r io.Reader, system bool, depth uint8) (c *Config, err error) {
221+
func decodeBytes(b []byte, system bool, depth uint8) (c *Config, err error) {
218222
defer func() {
219223
if r := recover(); r != nil {
220224
if _, ok := r.(runtime.Error); ok {
@@ -228,7 +232,7 @@ func decode(r io.Reader, system bool, depth uint8) (c *Config, err error) {
228232
}
229233
}()
230234

231-
c = parseSSH(lexSSH(r), system, depth)
235+
c = parseSSH(lexSSH(b), system, depth)
232236
return c, err
233237
}
234238

lexer.go

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
package ssh_config
22

33
import (
4-
"io"
5-
6-
buffruneio "github.com/pelletier/go-buffruneio"
4+
"bytes"
75
)
86

97
// Define state functions
108
type sshLexStateFn func() sshLexStateFn
119

1210
type sshLexer struct {
13-
input *buffruneio.Reader // Textual source
14-
buffer []rune // Runes composing the current token
11+
inputIdx int
12+
input []rune // Textual source
13+
14+
buffer []rune // Runes composing the current token
1515
tokens chan token
1616
line int
1717
col int
@@ -114,16 +114,14 @@ func (s *sshLexer) lexRvalue() sshLexStateFn {
114114
}
115115

116116
func (s *sshLexer) read() rune {
117-
r, _, err := s.input.ReadRune()
118-
if err != nil {
119-
panic(err)
120-
}
117+
r := s.peek()
121118
if r == '\n' {
122119
s.endbufferLine++
123120
s.endbufferCol = 1
124121
} else {
125122
s.endbufferCol++
126123
}
124+
s.inputIdx++
127125
return r
128126
}
129127

@@ -197,21 +195,22 @@ func (s *sshLexer) emitWithValue(t tokenType, value string) {
197195
}
198196

199197
func (s *sshLexer) peek() rune {
200-
r, _, err := s.input.ReadRune()
201-
if err != nil {
202-
panic(err)
198+
if s.inputIdx >= len(s.input) {
199+
return eof
203200
}
204-
s.input.UnreadRune()
201+
202+
r := s.input[s.inputIdx]
205203
return r
206204
}
207205

208206
func (s *sshLexer) follow(next string) bool {
207+
inputIdx := s.inputIdx
209208
for _, expectedRune := range next {
210-
r, _, err := s.input.ReadRune()
211-
defer s.input.UnreadRune()
212-
if err != nil {
213-
panic(err)
209+
if inputIdx >= len(s.input) {
210+
return false
214211
}
212+
r := s.input[inputIdx]
213+
inputIdx++
215214
if expectedRune != r {
216215
return false
217216
}
@@ -226,10 +225,10 @@ func (s *sshLexer) run() {
226225
close(s.tokens)
227226
}
228227

229-
func lexSSH(input io.Reader) chan token {
230-
bufferedInput := buffruneio.NewReader(input)
228+
func lexSSH(input []byte) chan token {
229+
runes := bytes.Runes(input)
231230
l := &sshLexer{
232-
input: bufferedInput,
231+
input: runes,
233232
tokens: make(chan token),
234233
line: 1,
235234
col: 1,

parser.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ func (p *sshParser) parseComment() sshParserStateFn {
169169
}
170170

171171
func parseSSH(flow chan token, system bool, depth uint8) *Config {
172+
// Ensure we consume tokens to completion even if parser exits early
173+
defer func() {
174+
for range flow {
175+
}
176+
}()
177+
172178
result := newConfig()
173179
result.position = Position{1, 1}
174180
parser := &sshParser{

parser_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package ssh_config
2+
3+
import (
4+
"errors"
5+
"testing"
6+
)
7+
8+
type errReader struct {
9+
}
10+
11+
func (b *errReader) Read(p []byte) (n int, err error) {
12+
return 0, errors.New("read error occurred")
13+
}
14+
15+
func TestIOError(t *testing.T) {
16+
buf := &errReader{}
17+
_, err := Decode(buf)
18+
if err == nil {
19+
t.Fatal("expected non-nil err, got nil")
20+
}
21+
if err.Error() != "read error occurred" {
22+
t.Errorf("expected read error msg, got %v", err)
23+
}
24+
}

0 commit comments

Comments
 (0)