From 18ae215998aa91642d6265f8b51f1432b5509143 Mon Sep 17 00:00:00 2001 From: Lixen Wraith Date: Sat, 3 Oct 2026 23:14:11 -0400 Subject: [PATCH] v0.1.3 bound parse memory and errors, conform better to toml spec for escpae and control chars --- README.md | 20 +++++--- audit_fix_test.go | 125 ++++++++++++++++++++++++++++++++++++++++++++++ decode.go | 33 ++++++++++-- encode.go | 8 ++- fuzz_test.go | 3 ++ lexer.go | 32 +++++++++--- parser.go | 98 ++++++++++++++++++++++++++++-------- token.go | 4 +- 8 files changed, 283 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index e7fcbb6..bbbc49e 100644 --- a/README.md +++ b/README.md @@ -61,11 +61,18 @@ elements error, because TOML has no null value. Nil maps and slices encode as empty containers. Duplicate struct tag names error. Basic-string escapes emitted by the encoder (`\b`, `\f`, `\n`, `\r`, `\t`, -quotes, backslashes, Unicode escapes) decode symmetrically. Invalid UTF-8 and -unescaped control characters in strings are rejected. Encoding and typed decoding +quotes, backslashes, Unicode escapes) decode symmetrically; the TOML 1.1 `\e` +and `\xHH` decode too. Invalid UTF-8 and unescaped control characters in +strings are rejected, as are control characters other than tab in comments and +a carriage return outside CRLF. Encoding and typed decoding have recursion bounds (1000 levels), so cyclic traversals fail instead of overflowing the stack. Parsing bounds individual value nesting and dotted paths; -combining paths can still produce trees that exceed the encoder's bound. +combining paths can still produce trees that exceed the encoder's bound. A +document may create at most `DefaultMaxTables` (65536) tables, counting every +dotted-key segment and header part; set `Parser.MaxTables` before `Parse` to +change it. Errors quote keys and cut long literals, and never repeat a string +value. `Marshal` writes a table inline once its header path would pass 256 +bytes, so output stays linear in depth. Comments and original formatting are not preserved by `Parse`/`Marshal`. A configuration editor can scan the original bytes with `NewLexer` and retain @@ -82,9 +89,10 @@ comment tokens. Stop scanning on `TokenEOF` or `TokenError`. unsupported. - Decimal integer keys (including quoted keys, optional signs and values beyond int64) are forbidden. This restriction also applies to the encoder. -- Unknown string escapes are preserved verbatim as a documented relaxation. -- Explicit table headers may reopen existing tables. Inline tables cannot be - extended later. +- String escapes invalid in every TOML version are preserved verbatim as a + documented relaxation. +- Explicit table headers may reopen existing tables, and dotted keys may extend + a table a header defined. Inline tables cannot be extended later. - Inline tables allow newlines and trailing commas. Arrays require commas, including between lines. Top-level statements require newline separation. - Signed radix integers and uppercase radix prefixes are accepted extensions. diff --git a/audit_fix_test.go b/audit_fix_test.go index f6eb57c..3414b93 100644 --- a/audit_fix_test.go +++ b/audit_fix_test.go @@ -1,6 +1,8 @@ package toml import ( + "errors" + "fmt" "math" "reflect" "strconv" @@ -200,3 +202,126 @@ func TestParser_ValueDepthLimit(t *testing.T) { t.Error("expected nesting depth error") } } + +// Each dotted-key segment and header part is a map; the document's budget of +// them bounds memory, which input size alone does not (x167 for dotted keys). +func TestParser_TableBudget(t *testing.T) { + doc := []byte("a" + strings.Repeat(".a", 101) + " = 1\n") // 101 maps + p := NewParser(doc) + p.MaxTables = 100 + if _, err := p.Parse(); err == nil || !strings.Contains(err.Error(), "exceeds 100 tables") { + t.Fatalf("101 tables under a budget of 100: %v", err) + } + p = NewParser(doc) + p.MaxTables = 101 + if _, err := p.Parse(); err != nil { + t.Fatalf("within budget: %v", err) + } + var b strings.Builder + for i := range DefaultMaxTables { + fmt.Fprintf(&b, "[t%d]\n", i) + } + if _, err := NewParser([]byte(b.String() + "[last]\n")).Parse(); err == nil { + t.Fatal("the default budget does not hold") + } +} + +// TOML 1.1 escapes decode as TOML 1.1 readers decode them +func TestLexer_TOML11Escapes(t *testing.T) { + m, err := NewParser([]byte(`s = "\x61\e\xe9"`)).Parse() + if err != nil || m["s"] != "a\x1bé" { + t.Fatalf("got %q, %v", m["s"], err) + } + if _, err := NewParser([]byte(`s = "\x6"`)).Parse(); err == nil { + t.Fatal(`\x with one hex digit accepted`) + } +} + +// Outside strings a control character can only hide text from a reader: it +// is refused in comments, and a CR is accepted only as part of CRLF. +func TestLexer_ControlCharactersOutsideStrings(t *testing.T) { + for _, doc := range []string{ + "a = 1 # see \x1b[8m\nb = 2", + "a = 1 # hidden\rb = 2\n", + "a = 1\rb = 2\n", + "a =\r1\n", + "a = 1 # del \x7f\n", + } { + if _, err := NewParser([]byte(doc)).Parse(); err == nil { + t.Errorf("%q accepted", doc) + } + } + m, err := NewParser([]byte("a = 1 # c\t\r\n[t] # d\r\nb = 2\r\n")).Parse() + if err != nil || m["a"] != int64(1) { + t.Fatalf("CRLF document: %v %v", m, err) + } +} + +// The doubled brackets of an array-of-tables header are one delimiter +func TestParser_ArrayTableDelimitersAdjacent(t *testing.T) { + for _, doc := range []string{"[ [a] ]\n", "[[a] ]\n", "[ [a]]\n"} { + if _, err := NewParser([]byte(doc)).Parse(); err == nil { + t.Errorf("%q accepted as a header", doc) + } + } + for _, doc := range []string{"[[a]]\n", "[ a ]\n", "[[ a ]]\n"} { + if _, err := NewParser([]byte(doc)).Parse(); err != nil { + t.Errorf("%q: %v", doc, err) + } + } +} + +// Errors stay small and never carry a string's content, which may be a +// secret (a verifier in a credentials file) +func TestErrorsBoundedWithoutStringContent(t *testing.T) { + for _, doc := range []string{`stored_key "SECRETVALUE"`, `password = SECRETVALUE`} { + _, err := NewParser([]byte(doc + "\n")).Parse() + if err == nil || strings.Contains(err.Error(), "SECRET") { + t.Fatalf("value content in error: %v", err) + } + } + _, err := NewParser([]byte("n = 1" + strings.Repeat("0", 1<<20) + "\n")).Parse() + if err == nil || len(err.Error()) > 200 { + t.Fatalf("error of %d bytes", len(err.Error())) + } +} + +// A decode error holds its path once, not a copy of the message per level +func TestDecodeErrorPathIsLinear(t *testing.T) { + type deep map[string]deep + var data any = "not a table" + for range 500 { + data = map[string]any{"key": data} + } + err := Decode(data, new(deep)) + var total int + for e := err; e != nil; e = errors.Unwrap(e) { + total += len(e.Error()) + } + if err == nil || total > 3*len(err.Error()) { + t.Fatalf("messages along the chain: %d bytes for a %d-byte error", total, len(err.Error())) + } +} + +// Header paths are bounded: deep nesting is written inline past the bound, +// so output stays linear and still round-trips +func TestMarshal_HeaderLengthBounded(t *testing.T) { + root := map[string]any{} + cur := root + for range 300 { + next := map[string]any{"v": int64(1)} + cur["long_key_0123"] = next + cur = next + } + out, err := Marshal(root) + if err != nil { + t.Fatal(err) + } + if len(out) > 64*1024 { + t.Fatalf("%d bytes for 300 levels", len(out)) + } + back, err := NewParser(out).Parse() + if err != nil || !reflect.DeepEqual(normalize(back), normalize(root)) { + t.Fatalf("round trip: %v", err) + } +} diff --git a/decode.go b/decode.go index 8666640..d659d31 100644 --- a/decode.go +++ b/decode.go @@ -40,6 +40,33 @@ func decodeValue(data any, val reflect.Value) error { return decodeAt(data, val, 0) } +// pathError names where decoding failed. Each level adds its segment instead +// of wrapping the message, which would copy the message once per level. +type pathError struct { + path []string // innermost first + err error +} + +func (e *pathError) Error() string { + var b strings.Builder + for i := len(e.path) - 1; i >= 0; i-- { + b.WriteString(e.path[i]) + b.WriteString(": ") + } + b.WriteString(e.err.Error()) + return b.String() +} + +func (e *pathError) Unwrap() error { return e.err } + +func atPath(segment string, err error) error { + if pe, ok := err.(*pathError); ok { + pe.path = append(pe.path, segment) + return pe + } + return &pathError{path: []string{segment}, err: err} +} + func decodeAt(data any, val reflect.Value, depth int) error { if depth > maxValueDepth { return fmt.Errorf("decode nesting exceeds %d", maxValueDepth) @@ -91,7 +118,7 @@ func decodeAt(data any, val reflect.Value, depth int) error { } for i := 0; i < len(dataSlice); i++ { if err := decodeAt(dataSlice[i], newSlice.Index(i), depth+1); err != nil { - return fmt.Errorf("index %d: %w", i, err) + return atPath(fmt.Sprintf("index %d", i), err) } } val.Set(newSlice) @@ -109,7 +136,7 @@ func decodeAt(data any, val reflect.Value, depth int) error { for k, vData := range dataMap { newVal := reflect.New(elemType).Elem() if err := decodeAt(vData, newVal, depth+1); err != nil { - return fmt.Errorf("map key %s: %w", k, err) + return atPath("map key "+brief(k), err) } newMap.SetMapIndex(reflect.ValueOf(k).Convert(val.Type().Key()), newVal) } @@ -206,7 +233,7 @@ func decodeStruct(data map[string]any, val reflect.Value, depth int) error { // Look up in data map (case sensitive) if vData, ok := data[key]; ok { if err := decodeAt(vData, field, depth+1); err != nil { - return fmt.Errorf("%s.%s: %w", typ.Name(), fieldType.Name, err) + return atPath(typ.Name()+"."+fieldType.Name, err) } } } diff --git a/encode.go b/encode.go index adf54a5..62209ba 100644 --- a/encode.go +++ b/encode.go @@ -109,6 +109,10 @@ func entries(v reflect.Value) ([]entry, error) { return result, nil } +// Every header repeats its table's full path, so deep nesting under headers +// alone grows quadratically; past this length a table is written inline. +const maxHeaderBytes = 256 + func (e *encoder) table(v reflect.Value, prefix string, depth int) error { if depth > maxValueDepth { return fmt.Errorf("encode nesting exceeds %d (possible cycle)", maxValueDepth) @@ -119,11 +123,11 @@ func (e *encoder) table(v reflect.Value, prefix string, depth int) error { } var tables []entry for _, x := range fields { - if isTable(x.value) || isTableArray(x.value) { + key, _ := keyText(x.key) + if (isTable(x.value) || isTableArray(x.value)) && len(prefix)+len(key) < maxHeaderBytes { tables = append(tables, x) continue } - key, _ := keyText(x.key) e.buf.WriteString(key + " = ") if err := e.value(x.value, depth+1); err != nil { return fmt.Errorf("key %q: %w", x.key, err) diff --git a/fuzz_test.go b/fuzz_test.go index e48bac5..8bc5bc6 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -15,6 +15,9 @@ func FuzzParse(f *testing.F) { "t = { a = 1, b = { c = [1, 2] } }", `s = "esc \u00E9 \t"`, "n = -0x10\nb = 0b101\no = 0o17", + "e = \"\\e\\x41\"\r\n# c\r\n[[a]]\r\n", + "a = 1 # \x1b[8m\nb = 2", + "[ [a] ]\nk =\r1", } { f.Add([]byte(s)) } diff --git a/lexer.go b/lexer.go index 8d28d46..59b5feb 100644 --- a/lexer.go +++ b/lexer.go @@ -91,7 +91,7 @@ func (l *Lexer) NextToken() Token { } l.advance() - return l.newToken(TokenError, fmt.Sprintf("unexpected character: %c", ch)) + return l.newToken(TokenError, fmt.Sprintf("unexpected character: %q", ch)) } func (l *Lexer) readNumber() Token { @@ -302,10 +302,12 @@ func (l *Lexer) peekAt(n int) rune { return r } +// skipWhitespace takes a CR only as part of CRLF: a lone one would let a +// terminal hide what precedes it on the line, so it lexes as an error. func (l *Lexer) skipWhitespace() { for l.pos < len(l.input) { ch := l.peek() - if ch == ' ' || ch == '\t' || ch == '\r' { + if ch == ' ' || ch == '\t' || ch == '\r' && l.peekAt(1) == '\n' { l.advance() } else { break @@ -313,10 +315,19 @@ func (l *Lexer) skipWhitespace() { } } +// readComment refuses control characters but tab, as TOML does: an ESC or a +// lone CR in a comment can hide the key after it from a reader. func (l *Lexer) readComment() Token { l.advance() // '#' start := l.pos - for l.pos < len(l.input) && l.peek() != '\n' { + for l.pos < len(l.input) { + ch := l.peek() + if ch == '\n' || ch == '\r' && l.peekAt(1) == '\n' { + break + } + if ch < 0x20 && ch != '\t' || ch == 0x7f { + return l.newToken(TokenError, fmt.Sprintf("control character %q in comment", ch)) + } l.advance() } return l.newToken(TokenComment, string(l.input[start:l.pos])) @@ -357,12 +368,17 @@ func (l *Lexer) readString() Token { result = append(result, '\b') case 'f': result = append(result, '\f') - case 'u', 'U': - n := 4 - if ch == 'U' { + case 'e': + result = append(result, 0x1b) + case 'x', 'u', 'U': + n := 2 + switch ch { + case 'u': + n = 4 + case 'U': n = 8 } - l.advance() // consume 'u'/'U' + l.advance() // consume 'x'/'u'/'U' var code rune for j := 0; j < n; j++ { d, ok := hexDigitVal(l.peek()) @@ -383,7 +399,7 @@ func (l *Lexer) readString() Token { escaped = false continue // hex digits already consumed; skip trailing l.advance() default: - // Unknown escape: preserve backslash and full rune + // Unknown escape, invalid in every TOML version: preserve backslash and full rune result = append(result, '\\') var buf [utf8.UTFMax]byte n := utf8.EncodeRune(buf[:], ch) diff --git a/parser.go b/parser.go index 1e74c93..de12d00 100644 --- a/parser.go +++ b/parser.go @@ -1,6 +1,8 @@ package toml import ( + "cmp" + "errors" "fmt" "reflect" "strconv" @@ -18,11 +20,19 @@ type Parser struct { peekToken Token // Value nesting depth (arrays / inline tables) depth int + // Tables (maps) a document may create; 0 means DefaultMaxTables + MaxTables int + tables int } // Recursion bound for parseValue -> parseArray/parseInlineTable const maxValueDepth = 1000 +// DefaultMaxTables bounds the maps one document creates. Every dotted-key +// segment and header part is a map of about 350 bytes, so without a bound a +// 10 MiB file of dotted keys holds near 2 GB. +const DefaultMaxTables = 1 << 16 + func NewParser(input []byte) *Parser { l := NewLexer(input) p := &Parser{ @@ -36,6 +46,15 @@ func NewParser(input []byte) *Parser { return p } +// newTable makes a map, counted against the document's table budget +func (p *Parser) newTable() (map[string]any, error) { + p.tables++ + if limit := cmp.Or(p.MaxTables, DefaultMaxTables); p.tables > limit { + return nil, fmt.Errorf("document exceeds %d tables at line %d", limit, p.curToken.Line) + } + return make(map[string]any), nil +} + func (p *Parser) nextToken() { p.curToken = p.peekToken p.peekToken = p.lexer.NextToken() @@ -81,10 +100,11 @@ func (p *Parser) parseStatement() error { } } -// parseTableDeclaration handles [key] and [[key]] +// parseTableDeclaration handles [key] and [[key]]; the doubled brackets of +// an array of tables are one delimiter, so "[ [a] ]" is no header func (p *Parser) parseTableDeclaration() error { isArray := false - if p.peekToken.Type == TokenLBracket { + if p.peekToken.Type == TokenLBracket && adjacent(p.curToken, p.peekToken) { // It is [[ ... p.nextToken() // consume first [ isArray = true @@ -98,8 +118,8 @@ func (p *Parser) parseTableDeclaration() error { } if isArray { - if p.curToken.Type != TokenRBracket { - return fmt.Errorf("expected closing bracket for array table at line %d", p.curToken.Line) + if p.curToken.Type != TokenRBracket || p.peekToken.Type != TokenRBracket || !adjacent(p.curToken, p.peekToken) { + return fmt.Errorf("expected ]] closing array table at line %d", p.curToken.Line) } p.nextToken() // consume first ] } @@ -122,7 +142,7 @@ func (p *Parser) setTableScope(keys []string, isArrayOfTables bool) error { isLast := i == len(keys)-1 currentMap, ok := ptr.(map[string]any) if !ok { - return fmt.Errorf("key path conflict: %s is not a map", key) + return fmt.Errorf("key path conflict: %q is not a map", key) } if isLast { @@ -133,13 +153,16 @@ func (p *Parser) setTableScope(keys []string, isArrayOfTables bool) error { if s, ok := val.([]map[string]any); ok { slice = s } else { - return fmt.Errorf("key conflict: %s is not an array of tables", key) + return fmt.Errorf("key conflict: %q is not an array of tables", key) } } else { slice = make([]map[string]any, 0) } - newMap := make(map[string]any) + newMap, err := p.newTable() + if err != nil { + return err + } slice = append(slice, newMap) currentMap[key] = slice p.current = newMap @@ -154,10 +177,13 @@ func (p *Parser) setTableScope(keys []string, isArrayOfTables bool) error { } targetMap = m } else { - return fmt.Errorf("key conflict: %s is not a table", key) + return fmt.Errorf("key conflict: %q is not a table", key) } } else { - targetMap = make(map[string]any) + var err error + if targetMap, err = p.newTable(); err != nil { + return err + } currentMap[key] = targetMap } p.current = targetMap @@ -174,14 +200,17 @@ func (p *Parser) setTableScope(keys []string, isArrayOfTables bool) error { ptr = m } else if slice, ok := val.([]map[string]any); ok { if len(slice) == 0 { - return fmt.Errorf("cannot traverse empty array table %s", key) + return fmt.Errorf("cannot traverse empty array table %q", key) } ptr = slice[len(slice)-1] } else { - return fmt.Errorf("intermediate key %s is not a map", key) + return fmt.Errorf("intermediate key %q is not a map", key) } } else { - newMap := make(map[string]any) + newMap, err := p.newTable() + if err != nil { + return err + } currentMap[key] = newMap ptr = newMap } @@ -224,7 +253,7 @@ func (p *Parser) assignValue(scope any, keys []string, val any) error { if i == len(keys)-1 { // Final key, assign value if _, exists := currentMap[key]; exists { - return fmt.Errorf("duplicate key %s at line %d", key, p.curToken.Line) + return fmt.Errorf("duplicate key %q at line %d", key, p.curToken.Line) } currentMap[key] = val } else { @@ -236,10 +265,13 @@ func (p *Parser) assignValue(scope any, keys []string, val any) error { } currentMap = m } else { - return fmt.Errorf("intermediate key %s is not a map", key) + return fmt.Errorf("intermediate key %q is not a map", key) } } else { - newMap := make(map[string]any) + newMap, err := p.newTable() + if err != nil { + return err + } currentMap[key] = newMap currentMap = newMap } @@ -253,13 +285,13 @@ func (p *Parser) parseKeyParts() ([]string, error) { for { // Rule: Tokens identified as Numbers are forbidden as keys if p.curToken.Type == TokenInteger || p.curToken.Type == TokenFloat { - return nil, fmt.Errorf("numeric keys are forbidden: %q", p.curToken.Literal) + return nil, fmt.Errorf("numeric keys are forbidden: %s", brief(p.curToken.Literal)) } if p.curToken.Type == TokenString { // Rule: Even quoted strings shouldn't be pure numbers per instruction if numericKey(p.curToken.Literal) { - return nil, fmt.Errorf("numeric string keys are forbidden: %q", p.curToken.Literal) + return nil, fmt.Errorf("numeric string keys are forbidden: %s", brief(p.curToken.Literal)) } } @@ -298,14 +330,14 @@ func (p *Parser) parseValue() (any, error) { case TokenInteger: val, err := p.parseInteger(p.curToken.Literal) if err != nil { - return nil, fmt.Errorf("invalid integer %q at line %d: %w", p.curToken.Literal, p.curToken.Line, err) + return nil, fmt.Errorf("invalid integer %s at line %d: %w", brief(p.curToken.Literal), p.curToken.Line, numCause(err)) } p.nextToken() return val, nil case TokenFloat: val, err := strconv.ParseFloat(p.curToken.Literal, 64) if err != nil { - return nil, fmt.Errorf("invalid float %q at line %d: %w", p.curToken.Literal, p.curToken.Line, err) + return nil, fmt.Errorf("invalid float %s at line %d: %w", brief(p.curToken.Literal), p.curToken.Line, numCause(err)) } p.nextToken() return val, nil @@ -318,6 +350,10 @@ func (p *Parser) parseValue() (any, error) { case TokenLBrace: return p.parseInlineTable() } + if p.curToken.Type == TokenIdent { + // An unquoted value may be a secret: say what it is, not what it says + return nil, fmt.Errorf("unquoted value at line %d: strings need quotes", p.curToken.Line) + } return nil, fmt.Errorf("unexpected value token %s at line %d", p.curToken.String(), p.curToken.Line) } @@ -386,7 +422,10 @@ func (p *Parser) parseArray() ([]any, error) { func (p *Parser) parseInlineTable() (map[string]any, error) { p.nextToken() // consume { - m := make(map[string]any) + m, err := p.newTable() + if err != nil { + return nil, err + } for p.curToken.Type != TokenRBrace { if p.curToken.Type == TokenNewline { @@ -434,6 +473,25 @@ func (p *Parser) parseInlineTable() (map[string]any, error) { return m, nil } +// adjacent reports b starting right after the one-byte token a +func adjacent(a, b Token) bool { return a.Line == b.Line && b.Col == a.Col+1 } + +// brief quotes s for an error, cut to 32 bytes: a literal can be megabytes +func brief(s string) string { + if len(s) > 32 { + return fmt.Sprintf("%q... (%d bytes)", s[:32], len(s)) + } + return fmt.Sprintf("%q", s) +} + +// numCause drops strconv's copy of the literal, which brief already shows +func numCause(err error) error { + if ne, ok := errors.AsType[*strconv.NumError](err); ok { + return ne.Err + } + return err +} + func signPrefix(negative bool) string { if negative { return "-" diff --git a/token.go b/token.go index 99d4e40..8fb83fa 100644 --- a/token.go +++ b/token.go @@ -46,9 +46,11 @@ func (t Token) String() string { return fmt.Sprintf("Error(%s)", t.Literal) case TokenNewline: return "Newline" + case TokenString: + return "string" // its content may be a secret, as in a credentials file } if len(t.Literal) > 20 { return fmt.Sprintf("%q...", t.Literal[:20]) } return fmt.Sprintf("%q", t.Literal) -} \ No newline at end of file +}