diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..ba4c09e --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,18 @@ +name: Test +on: [push, pull_request] +permissions: + contents: read +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: false # Standard-library-only module has no go.sum. + - run: go test -race ./... + - run: GOARCH=386 go test ./... + - run: go vet ./... + - run: go test -run '^$' -fuzz '^FuzzRoundTrip$' -fuzztime=15s -parallel=2 + - run: go test -run '^$' -fuzz '^FuzzIntegerConversions$' -fuzztime=10s -parallel=2 diff --git a/README.md b/README.md index c2dbfc7..3ea7b3c 100644 --- a/README.md +++ b/README.md @@ -1,154 +1,106 @@ # toml -Zero-dependency TOML encoder/decoder for Go. -Implements a practical subset of TOML v1.0 with documented deviations. -Standard library only. - -## Install - - go get github.com/lixenwraith/toml - -## Usage - -### Unmarshal +A deterministic TOML subset encoder/decoder using only the standard library. +Requires Go 1.27.1. ```go -input := []byte(` -initial = "Idle" - -[states.Idle] -parent = "Root" -transitions = [ - { trigger = "Start", target = "Active" } -] -`) - -type Transition struct { - Trigger string `toml:"trigger"` - Target string `toml:"target"` - Guard string `toml:"guard,omitempty"` +type Settings struct { + Name string `toml:"name"` + Port uint16 `toml:"port"` } -type State struct { - Parent string `toml:"parent"` - Transitions []Transition `toml:"transitions"` -} -type Config struct { - Initial string `toml:"initial"` - States map[string]*State `toml:"states"` -} - -var cfg Config -err := toml.Unmarshal(input, &cfg) +var settings Settings +err := toml.Unmarshal([]byte("name = \"service\"\nport = 8080\n"), &settings) +data, err := toml.Marshal(settings) ``` -### Marshal +`NewParser(data).Parse()` returns `map[string]any`. `Decode(data, &target)` +converts an already parsed value. Errors leave the destination unchanged. +Missing struct fields retain their existing values; present maps and slices +replace old containers. Existing pointer defaults are retained without writing +through the original pointer. Fixed arrays require an exact input length. +Tags use `toml:"name,omitempty"`; an empty tag name +falls back to the Go field name. Matching is case sensitive. Unknown input keys +and unexported fields are ignored. -```go -out, err := toml.Marshal(cfg) // deterministic: keys sorted alphabetically -``` +## Values and conversions -### Decode +| TOML value | Parsed Go value | Typed destinations | +| --- | --- | --- | +| Integer | `int64`, on every architecture | Checked integer and float types | +| Float | `float64` | Float types; integers only when integral and in range | +| String | `string` | `string`, `any` | +| Boolean | `bool` | `bool`, `any` | +| Array | `[]any` | Slices, fixed arrays, `any` | +| Table | `map[string]any` | Structs, string-keyed maps, `any` | +| Array of tables | `[]map[string]any` | Slices and fixed arrays, including pointer elements | -`Decode(data any, v any)` maps an already-parsed `map[string]any` onto a -target. `Unmarshal` = `Parse` + `Decode`. +Direct `Decode` supports all signed/unsigned numeric widths and named numeric +types, including the full `uint64` range. Conversions reject overflow, negative +unsigned inputs, fractional-to-integer truncation, non-finite floats, and +integer-to-float precision loss. Float64-to-float32 rounding is allowed when in +range. Strings are not coerced to numbers by this package. -## Architecture +`Decode` is transactional, not a deep-copy or synchronization API. Interface +destinations retain input values (including map and slice references); unchanged +fields may retain existing references. A nil input is a no-op. Callers own +snapshot isolation and synchronization when values are shared between goroutines. - Unmarshal: []byte → Lexer → tokens → Parser → map[string]any → Decode (reflection) → target - Marshal: value → encoder (reflection, two-pass) → []byte +TOML integer literals have a signed 64-bit range. Consequently `Marshal` rejects +unsigned values above `MaxInt64`; callers may explicitly encode decimal strings +and interpret those in their application. Integer parsing and narrow destination +checks also work on 32-bit systems. -### Lexer (`lexer.go`) +## Encoding and round trips -- Single-pass, UTF-8 aware byte scanner with rune lookahead (`peekAt`). -- Emits: ident, string, integer, float, bool, punctuation, newline, comment. -- Number/key disambiguation happens at the token level: `1.5` → Float, - `a.b` → Ident Dot Ident, `1.2.3` → Error (multi-dot). Hex/octal/binary - prefixes and exponent forms are validated during scanning. -- Basic strings only. Escapes: `\"` `\\` `\n` `\t` `\r`. Unknown escape - sequences are preserved verbatim (spec requires an error — see Deviations). +`Marshal` accepts a struct or string-keyed map, including pointers to them. Keys +are sorted. Scalar fields precede nested tables. Table path segments are quoted +individually, preserving dots, spaces, quotes and Unicode in keys. Mixed arrays +and nested table values use inline tables when needed. Struct `omitempty` is +honored; nil pointer/interface fields are omitted. Nil pointer/interface array +elements error, because TOML has no null value. Nil maps and slices encode as +empty containers. Duplicate struct tag names error. -### Parser (`parser.go`) +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 +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. -- Recursive descent with a two-token window (`curToken`/`peekToken`). - Comments are skipped during token advance. -- Output model: tables → `map[string]any`, arrays → `[]any`, - arrays of tables → `[]map[string]any`. -- Table headers (`[a.b]`) always resolve from the root; a `current` cursor - tracks the active table scope for subsequent key/value pairs. -- Dotted keys create intermediate maps. Traversal through an existing - `[[array]]` descends into its last element (TOML semantics). -- Conflict detection at assignment: duplicate keys, scalar-vs-table - redefinition, table-vs-array-of-tables shadowing. Numeric keys are - rejected in all positions (bare, quoted, dotted segments). -- Inline tables are immutable (later dotted keys or `[headers]` targeting them error); value nesting (arrays/inline tables) capped at 1000 levels. +Comments and original formatting are not preserved by `Parse`/`Marshal`. A +configuration editor can scan the original bytes with `NewLexer` and retain +`TokenComment.Literal` values, then prefix each with `#` and write them as a +preamble before the marshaled data. This preserves comment text (including +inline comments), but not its original location. Hashes inside strings are not +comment tokens. Stop scanning on `TokenEOF` or `TokenError`. -### Decoder (`decode.go`) +## Deliberate subset and deviations -- Kind-switch reflection. Pointers are auto-allocated at any depth - (`******int` works). Nil maps and slices are materialized. -- Struct fields match by `toml` tag first, then exact (case-sensitive) - field name. `toml:"-"` skips. Unexported fields are skipped. -- Numeric coercion: any parser-produced int/uint/float64 converts to the - target numeric kind. Unknown keys in input are ignored. -- Numeric decode errors on target overflow (`300` → `int8` errors); non-empty interface targets error unless assignable; unsupported target kinds (chan, func, complex, fixed arrays) error; `uint`/`uint64` sources above `MaxInt64` are rejected. +- Datetime syntax is unsupported. Use quoted strings and parse them in the + application; this package does not handle `time.Time` or custom marshal hooks. +- Literal strings and multiline strings, digit separators, and `inf`/`nan` are + 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. +- 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. +- Leading zeros are accepted in decimal numbers; `0123` means decimal 123, + not octal. Use `0o123` for octal. +- Embedded struct fields are not promoted. Custom marshal interfaces are not + supported. Decode input containers are the parser's generic maps/slices. -### Encoder (`encode.go`) +## Migration from the initial version -- Two-pass emission per table: scalars and inline arrays first, then - `[tables]` and `[[arrays of tables]]`. Guarantees keys are defined - before sub-tables, i.e. output is always valid TOML. -- Keys sorted alphabetically → deterministic output for diffing/VCS. -- Bare-key validation mirrors the lexer's rules: keys that would lex as - numbers or booleans (`true`, `123a`, `-1x`) are quoted, so output always - re-parses (round-trip safe). -- `omitempty` honored; nil pointers and `toml:"-"` fields skipped; - root must be a struct or map. -- Floats use shortest `'g'` form at the source bit width (large magnitudes emit exponent notation); `NaN`/`±Inf` return an error; arrays mixing tables and scalars return an error (parser accepts them — encode-side limitation); struct keys sort by resolved tag name. +Parsed integers now use `int64`, not machine-sized `int`. Update type assertions +on raw parser results. Typed struct destinations keep their declared types. +Previously lossy numeric conversions and nil array elements now error. Mixed +arrays can now be encoded. All changes have regression and fuzz coverage. -## Type Mapping +Run `go test -race ./...`, `GOARCH=386 go test ./...`, and, for example, +`go test -run '^$' -fuzz '^FuzzRoundTrip$' -fuzztime=30s`. -| TOML | Parser output (`map[string]any`) | Decode targets | -|-----------------|----------------------------------|-----------------------------------| -| string | `string` | `string`, `any` | -| integer | `int` | any int/uint kind, float kinds | -| float | `float64` | `float32`, `float64` | -| boolean | `bool` | `bool` | -| array | `[]any` | `[]T`, `any` | -| table | `map[string]any` | struct, `map[string]T`, `any` | -| array of tables | `[]map[string]any` | `[]T`, `[]*T` | - -## Deviations from TOML v1.0 - -Intentional restrictions: -- Numeric keys are forbidden everywhere: `123 = 1`, `[456]`, `"789"`, - `a.1.b` all error. Stricter than spec. - -Extensions (accepted, though spec forbids): -- Newlines inside inline tables (multi-line inline tables parse). -- Trailing commas in inline tables. - -Relaxations: -- Unknown string escapes are preserved instead of erroring. -- Explicit table redefinition (`[a]` … `[a]`) is not rejected; the second - header reopens the table. Value/table conflicts are still caught. - -## Limitations - -Not supported: -- Date/time types (all four TOML forms). Workaround: store RFC 3339 - strings and convert with `time.Parse` at the call site; strings - round-trip cleanly. -- Literal strings `'...'` and multi-line strings `"""..."""`, `'''...'''`. -- Underscore digit separators (`1_000`). -- Arrays mixing tables and scalar values are not encodable. -- `inf` / `nan` float literals. -- Custom marshaling interfaces (`Marshaler`/`Unmarshaler`, - `encoding.TextMarshaler`). -- Embedded struct field promotion in decode. - -Platform note: TOML integers parse to Go `int`; 64-bit platforms assumed (amd64/arm64) and enforced at compile time (32-bit build fails). -Overflow of int64 during parse errors; 32-bit truncation of `int(int64)` is not guarded. - -## License - -BSD-3-Clause (see LICENSE). +BSD-3-Clause; see LICENSE. diff --git a/audit_fix_test.go b/audit_fix_test.go index f00f328..f6eb57c 100644 --- a/audit_fix_test.go +++ b/audit_fix_test.go @@ -2,6 +2,8 @@ package toml import ( "math" + "reflect" + "strconv" "strings" "testing" ) @@ -71,7 +73,7 @@ func TestDecode_Uint64Wrap(t *testing.T) { if err := Decode(map[string]any{"v": uint64(math.MaxUint64)}, &tgt); err == nil { t.Errorf("MaxUint64 into int64 must error, got %d", tgt.V) } - if err := Decode(map[string]any{"v": uint(math.MaxUint64)}, &tgt); err == nil { + if err := Decode(map[string]any{"v": ^uint(0)}, &tgt); strconv.IntSize == 64 && err == nil { t.Errorf("uint wrap into int64 must error, got %d", tgt.V) } if err := Decode(map[string]any{"v": uint64(42)}, &tgt); err != nil || tgt.V != 42 { @@ -118,23 +120,23 @@ func TestMarshal_NaNInf(t *testing.T) { } } -func TestMarshal_SliceHomogeneity(t *testing.T) { - if _, err := Marshal(map[string]any{"x": []any{map[string]any{"a": 1}, 2}}); err == nil { - t.Error("mixed table/scalar slice must error") - } - if _, err := Marshal(map[string]any{"x": []any{1, nil, 2}}); err == nil { - t.Error("nil interface element must error") - } - - type Item struct { - N int `toml:"n"` - } - b, err := Marshal(map[string]any{"items": []*Item{nil, {N: 1}}}) +func TestMarshal_MixedArraysAndNil(t *testing.T) { + input := map[string]any{"x": []any{map[string]any{"a": int64(1)}, int64(2)}} + data, err := Marshal(input) if err != nil { - t.Fatalf("nil pointer in table slice must be skipped, not error: %v", err) + t.Fatal(err) } - if strings.Count(string(b), "[[items]]") != 1 { - t.Errorf("expected exactly one [[items]]:\n%s", b) + var got map[string]any + if err := Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(input, got) { + t.Fatalf("got %#v", got) + } + for _, value := range []any{[]any{1, nil}, []*struct{ N int }{nil}} { + if _, err := Marshal(map[string]any{"x": value}); err == nil { + t.Fatal("nil array element accepted") + } } } diff --git a/decode.go b/decode.go index e1b2f46..8666640 100644 --- a/decode.go +++ b/decode.go @@ -17,18 +17,33 @@ func Unmarshal(data []byte, v any) error { return Decode(parsedMap, v) } -// Decode maps a generic map[string]any to a struct/slice/etc using reflection. -// It prioritizes `toml` tags and falls back to field names. +// Decode maps parser values to typed destinations using reflection. +// It uses toml tags, falling back to field names, and leaves v unchanged on error. +// Interface destinations retain the input value, including container references. func Decode(data any, v any) error { val := reflect.ValueOf(v) if val.Kind() != reflect.Ptr || val.IsNil() { return fmt.Errorf("target must be a non-nil pointer") } - return decodeValue(data, val.Elem()) + // Stage changes; pointer, map and slice branches allocate before writing. + next := reflect.New(val.Elem().Type()).Elem() + next.Set(val.Elem()) + if err := decodeValue(data, next); err != nil { + return err + } + val.Elem().Set(next) + return nil } func decodeValue(data any, val reflect.Value) error { + return decodeAt(data, val, 0) +} + +func decodeAt(data any, val reflect.Value, depth int) error { + if depth > maxValueDepth { + return fmt.Errorf("decode nesting exceeds %d", maxValueDepth) + } if data == nil { return nil } @@ -37,7 +52,10 @@ func decodeValue(data any, val reflect.Value) error { case reflect.Ptr: elemType := val.Type().Elem() newVal := reflect.New(elemType) - if err := decodeValue(data, newVal.Elem()); err != nil { + if !val.IsNil() { + newVal.Elem().Set(val.Elem()) + } + if err := decodeAt(data, newVal.Elem(), depth+1); err != nil { return err } val.Set(newVal) @@ -47,9 +65,9 @@ func decodeValue(data any, val reflect.Value) error { if !ok { return fmt.Errorf("expected map for struct, got %T", data) } - return decodeStruct(dataMap, val) + return decodeStruct(dataMap, val, depth+1) - case reflect.Slice: + case reflect.Slice, reflect.Array: dataSlice, ok := data.([]any) if !ok { if mapSlice, ok := data.([]map[string]any); ok { @@ -62,10 +80,18 @@ func decodeValue(data any, val reflect.Value) error { } } - newSlice := reflect.MakeSlice(val.Type(), len(dataSlice), len(dataSlice)) + var newSlice reflect.Value + if val.Kind() == reflect.Array { + if len(dataSlice) != val.Len() { + return fmt.Errorf("expected array length %d, got %d", val.Len(), len(dataSlice)) + } + newSlice = reflect.New(val.Type()).Elem() + } else { + newSlice = reflect.MakeSlice(val.Type(), len(dataSlice), len(dataSlice)) + } for i := 0; i < len(dataSlice); i++ { - if err := decodeValue(dataSlice[i], newSlice.Index(i)); err != nil { - return err + if err := decodeAt(dataSlice[i], newSlice.Index(i), depth+1); err != nil { + return fmt.Errorf("index %d: %w", i, err) } } val.Set(newSlice) @@ -78,14 +104,14 @@ func decodeValue(data any, val reflect.Value) error { if !ok { return fmt.Errorf("expected map, got %T", data) } - newMap := reflect.MakeMap(val.Type()) + newMap := reflect.MakeMapWithSize(val.Type(), len(dataMap)) elemType := val.Type().Elem() for k, vData := range dataMap { newVal := reflect.New(elemType).Elem() - if err := decodeValue(vData, newVal); err != nil { + if err := decodeAt(vData, newVal, depth+1); err != nil { return fmt.Errorf("map key %s: %w", k, err) } - newMap.SetMapIndex(reflect.ValueOf(k), newVal) + newMap.SetMapIndex(reflect.ValueOf(k).Convert(val.Type().Key()), newVal) } val.Set(newMap) @@ -109,21 +135,19 @@ func decodeValue(data any, val reflect.Value) error { val.SetInt(i) case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - i, ok := toInt64(data) + u, ok := toUint64(data) if !ok { - return fmt.Errorf("cannot convert %T to uint", data) - } - if i < 0 { - return fmt.Errorf("cannot convert negative value %d to uint", i) + // Formatting a rejected container's value could recurse through a cycle. + return fmt.Errorf("cannot convert %T to uint without loss", data) } // Overflow check - if val.OverflowUint(uint64(i)) { - return fmt.Errorf("value %d overflows %s", i, val.Type()) + if val.OverflowUint(u) { + return fmt.Errorf("value %d overflows %s", u, val.Type()) } - val.SetUint(uint64(i)) + val.SetUint(u) case reflect.Float32, reflect.Float64: - f, ok := toFloat(data) + f, ok := toFloat(data, val.Type().Bits()) if !ok { return fmt.Errorf("cannot convert %T to float", data) } @@ -155,7 +179,7 @@ func decodeValue(data any, val reflect.Value) error { return nil } -func decodeStruct(data map[string]any, val reflect.Value) error { +func decodeStruct(data map[string]any, val reflect.Value, depth int) error { typ := val.Type() for i := 0; i < val.NumField(); i++ { @@ -174,12 +198,14 @@ func decodeStruct(data map[string]any, val reflect.Value) error { if parts[0] == "-" { continue } - key = parts[0] + if parts[0] != "" { + key = parts[0] + } } // Look up in data map (case sensitive) if vData, ok := data[key]; ok { - if err := decodeValue(vData, field); err != nil { + if err := decodeAt(vData, field, depth+1); err != nil { return fmt.Errorf("%s.%s: %w", typ.Name(), fieldType.Name, err) } } @@ -187,67 +213,79 @@ func decodeStruct(data map[string]any, val reflect.Value) error { return nil } -// toInt64 converts numeric types to int64 +// Numeric conversions check before conversion: rounded float64(MaxInt64) is +// 2^63, so the upper bound must be exclusive. No float intermediary for integers. func toInt64(v any) (int64, bool) { - switch i := v.(type) { - case int: - return int64(i), true - case int8: - return int64(i), true - case int16: - return int64(i), true - case int32: - return int64(i), true - case int64: - return i, true - case uint: - // Reject wrap - if uint64(i) > math.MaxInt64 { + r := reflect.ValueOf(v) + if !r.IsValid() { + return 0, false + } + switch r.Kind() { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return r.Int(), true + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + u := r.Uint() + return int64(u), u <= math.MaxInt64 + case reflect.Float32, reflect.Float64: + f := r.Float() + if math.IsNaN(f) || f < -0x1p63 || f >= 0x1p63 || math.Trunc(f) != f { return 0, false } - case uint8: - return int64(i), true - case uint16: - return int64(i), true - case uint32: - return int64(i), true - case uint64: - // Reject values that wrap negative through int64 - if i > math.MaxInt64 { - return 0, false - } - return int64(i), true - case float64: - return int64(i), true + return int64(f), true } return 0, false } -func toFloat(v any) (float64, bool) { - switch i := v.(type) { - case int: - return float64(i), true - case int8: - return float64(i), true - case int16: - return float64(i), true - case int32: - return float64(i), true - case int64: - return float64(i), true - case uint: - return float64(i), true - case uint8: - return float64(i), true - case uint16: - return float64(i), true - case uint32: - return float64(i), true - case uint64: - return float64(i), true - case float64: - return i, true +func toUint64(v any) (uint64, bool) { + r := reflect.ValueOf(v) + if !r.IsValid() { + return 0, false + } + switch r.Kind() { + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return r.Uint(), true + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + i := r.Int() + return uint64(i), i >= 0 + case reflect.Float32, reflect.Float64: + f := r.Float() + if math.IsNaN(f) || f < 0 || f >= 0x1p64 || math.Trunc(f) != f { + return 0, false + } + return uint64(f), true } return 0, false } +func toFloat(v any, width int) (float64, bool) { + r := reflect.ValueOf(v) + if !r.IsValid() { + return 0, false + } + var f float64 + switch r.Kind() { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + i := r.Int() + f = float64(i) + if width == 32 { + f = float64(float32(f)) + } + if f >= 0x1p63 || f < -0x1p63 || int64(f) != i { + return 0, false + } + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + u := r.Uint() + f = float64(u) + if width == 32 { + f = float64(float32(f)) + } + if f >= 0x1p64 || uint64(f) != u { + return 0, false + } + case reflect.Float32, reflect.Float64: + f = r.Float() + default: + return 0, false + } + return f, !math.IsNaN(f) && !math.IsInf(f, 0) +} diff --git a/decode_fsm_test.go b/decode_fsm_test.go index b2c33a5..f78e8b1 100644 --- a/decode_fsm_test.go +++ b/decode_fsm_test.go @@ -308,7 +308,7 @@ transitions = [ t.Error("GuardArgs is nil") } else if ms, ok := grw.Transitions[0].GuardArgs["ms"]; !ok { t.Error("GuardArgs missing 'ms' key") - } else if msInt, ok := ms.(int); !ok || msInt != 2000 { + } else if msInt, ok := ms.(int64); !ok || msInt != 2000 { t.Errorf("GuardArgs.ms mismatch: %T %v", ms, ms) } } @@ -841,4 +841,4 @@ binary_val = 0b1010 if cfg.Edge.Cases.BinaryVal != 0b1010 { t.Errorf("BinaryVal = %d, want %d", cfg.Edge.Cases.BinaryVal, 0b1010) } -} \ No newline at end of file +} diff --git a/doc/audit.md b/doc/audit.md new file mode 100644 index 0000000..6156798 --- /dev/null +++ b/doc/audit.md @@ -0,0 +1,59 @@ +# Configuration integration audit + +The audit fixes concrete failures in the original release: + +- Numeric conversion truncated fractions, accepted float values outside integer + range, rejected ordinary `uint` input, and could not decode full `uint64`. + Checked conversions now operate in the source integer domain and reject loss. +- Parser integers depended on 64-bit `int`. They are now explicitly `int64`; + narrowed destinations retain overflow checks on every architecture. +- Failed decoding could partially overwrite a destination; decoding now commits + only after success. Named string map keys no longer panic, and tags with an + empty name correctly fall back to the field name. Existing pointer defaults + survive updates, and errors do not mutate the original pointee or containers. +- Fixed arrays could be encoded but not decoded. They now decode with exact + length and element-range checks, including nested arrays and arrays of tables. +- Nested table headers failed to quote their component keys. Arrays of interface + table values could encode empty tables. Mixed/nested arrays now encode using + inline tables, and nested keys use the same escaping as scalar keys. +- Backspace/form-feed strings did not decode the escapes emitted by Marshal. + Invalid UTF-8, raw string controls, missing array/inline-table commas and + missing statement separators are rejected. +- Cyclic encoding could overflow the stack. Encoding/decoding and dotted paths + now have depth limits. Numeric keys and unsupported uint64 literals fail + explicitly instead of generating unreadable output. Nil array elements error. +- Numeric key checks now cover decimal keys of any length, including values + beyond int64; the parser and encoder apply the same restriction. +- Formatting an invalid unsigned input could recurse through a cyclic map. + Conversion errors now report the source type without formatting its contents. +- Map decoding and encoder entry collection preallocate from known sizes. All + operation state stays local; no global cache or lock was added. + +No datetime, custom marshal interface, literal-string or multiline-string +feature was added. The supported subset and remaining deviations are in README. + +This branch consolidates the prior `audit/numeric-hardening-20260928` and +`audit/config-roundtrip-hardening` commits, retaining both in its ancestry. +Only this repository is changed. Consumer migration, watcher publication and +snapshot ownership remain work for the separate config audit. + +Verification for this pass: unit and race suites and `go vet` pass on Go 1.27.1; +the 386 test binary cross-compiles. This host cannot execute 386 binaries, so CI +executes that suite. The first CI run caught a test fixture that cast an int64 +through machine-sized int before Decode; that fixture now retains its int64. +Regression tests for fixed arrays, pointer defaults and oversized numeric keys +were also run against recovered commit `6384895` and reproduced the failures. + +The 15-second round-trip fuzz run passed 71,221 executions. Its property now +fails on unexpected Marshal errors instead of silently skipping them; only the +documented whole-tree depth bound is excluded. Integer fuzzing covers full +uint64 preservation, signed boundaries and uint8 narrowing. CI runs these bounded +fuzz checks on every push and pull request. Refer to the PR checks for final CI +results. + +Integration limits: parsed integers are int64; raw assertions must migrate. +Direct Decode can retain the full uint64 range, but file encoding rejects values +above MaxInt64. Interface destinations retain source references, so config must +own its snapshots. Comment text can be collected with the existing lexer and +written as a preamble; Marshal does not preserve source positions. README +documents these contracts and the intentionally limited TOML syntax. diff --git a/encode.go b/encode.go index 5b6df7b..adf54a5 100644 --- a/encode.go +++ b/encode.go @@ -2,460 +2,304 @@ package toml import ( "bytes" + "cmp" "fmt" "math" "reflect" - "sort" + "slices" "strconv" "strings" + "unicode/utf8" ) -// Marshal returns the TOML encoding of v -// -// Marshal supports struct and map types as the root object -// It follows standard TOML formatting: -// - Comments and whitespace are not preserved from original decoding -// - Dates are not supported (as per requirements). -// - Nil pointers are skipped -// - Unexported fields are skipped -// - Fields with `omitempty` are skipped if zero -// - Struct fields/Map keys are sorted alphabetically for determinism -// - Fully numerical keys are rejected +// Marshal encodes a struct or string-keyed map with deterministic key order. +// Nil pointer/interface fields are omitted; nil pointer/interface array elements, +// cycles/excessive nesting, +// non-finite floats, and integers outside TOML's signed 64-bit range error. +// Comments and whitespace are not retained. Datetimes are not supported. func Marshal(v any) ([]byte, error) { - val := reflect.ValueOf(v) - - // Dereference pointer if necessary - if val.Kind() == reflect.Ptr { - if val.IsNil() { - return nil, fmt.Errorf("marshal: cannot marshal nil pointer") - } - val = val.Elem() - } - - // Root must be a Table (Struct or Map) - if val.Kind() != reflect.Struct && val.Kind() != reflect.Map { - return nil, fmt.Errorf("marshal: root must be struct or map, got %v", val.Kind()) - } - - buf := new(bytes.Buffer) - enc := &encoder{w: buf} - - if err := enc.encodeTable(val, ""); err != nil { + var e encoder + if err := e.table(reflect.ValueOf(v), "", 0); err != nil { return nil, err } - - return buf.Bytes(), nil + return e.buf.Bytes(), nil } -type encoder struct { - w *bytes.Buffer +type encoder struct{ buf bytes.Buffer } +type entry struct { + key string + value reflect.Value } -// encodeTable writes the fields of a struct or map. -// It uses a two-pass approach: -// 1. Write all scalar values (primitives, inline arrays). -// 2. Recurse and write nested tables (structs, maps, arrays of tables). -// This ensures valid TOML where keys are defined before sub-tables. -func (e *encoder) encodeTable(rv reflect.Value, prefix string) error { - // 1. Gather keys - keys, err := e.getSortedKeys(rv) +func indirect(v reflect.Value) (reflect.Value, error) { + for depth := 0; v.IsValid() && (v.Kind() == reflect.Interface || v.Kind() == reflect.Pointer); depth++ { + if depth > maxValueDepth { + return reflect.Value{}, fmt.Errorf("pointer nesting exceeds %d", maxValueDepth) + } + if v.IsNil() { + return reflect.Value{}, nil + } + v = v.Elem() + } + return v, nil +} + +func entries(v reflect.Value) ([]entry, error) { + v, err := indirect(v) + if err != nil { + return nil, err + } + if !v.IsValid() { + return nil, fmt.Errorf("nil table") + } + var out []entry + switch v.Kind() { + case reflect.Map: + if v.Type().Key().Kind() != reflect.String { + return nil, fmt.Errorf("map keys must be strings") + } + out = make([]entry, 0, v.Len()) + it := v.MapRange() + for it.Next() { + out = append(out, entry{it.Key().String(), it.Value()}) + } + case reflect.Struct: + t := v.Type() + out = make([]entry, 0, t.NumField()) + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !f.IsExported() { + continue + } + parts := strings.Split(f.Tag.Get("toml"), ",") + if parts[0] == "-" { + continue + } + key := f.Name + if parts[0] != "" { + key = parts[0] + } + if slices.Contains(parts[1:], "omitempty") && isEmptyValue(v.Field(i)) { + continue + } + out = append(out, entry{key, v.Field(i)}) + } + default: + return nil, fmt.Errorf("expected struct or map, got %s", v.Kind()) + } + slices.SortFunc(out, func(a, b entry) int { return cmp.Compare(a.key, b.key) }) + result := out[:0] + previous := "" + for i, x := range out { + if _, err := keyText(x.key); err != nil { + return nil, err + } + if i > 0 && previous == x.key { + return nil, fmt.Errorf("duplicate TOML key %q", x.key) + } + previous = x.key + value, err := indirect(x.value) + if err != nil { + return nil, err + } + if value.IsValid() { + result = append(result, entry{x.key, value}) + } + } + return result, nil +} + +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) + } + fields, err := entries(v) if err != nil { return err } - - // 2. Separation: Identify which keys are scalars (printed now) vs tables (printed later) - var scalars []string - var tables []string - - for _, k := range keys { - fieldVal := e.resolveValue(rv, k) - if !fieldVal.IsValid() { - continue // Skip invalid/nil - } - - // Check if we should skip (omitempty, unexported handled in getSortedKeys) - if e.shouldSkip(rv, k, fieldVal) { + var tables []entry + for _, x := range fields { + if isTable(x.value) || isTableArray(x.value) { + tables = append(tables, x) continue } - - // isTable validates slice homogeneity - isTab, err := e.isTable(fieldVal) - if err != nil { - return fmt.Errorf("key %q: %w", e.getKeyName(rv, k), err) - } - if isTab { - tables = append(tables, k) - } else { - scalars = append(scalars, k) + 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) } + e.buf.WriteByte('\n') } - - // 3. Pass 1: Write Scalars - for _, k := range scalars { - val := e.resolveValue(rv, k) - keyName := e.getKeyName(rv, k) - - if err := e.writeKey(keyName); err != nil { - return err - } - if _, err := e.w.WriteString(" = "); err != nil { - return err - } - if err := e.encodeValue(val); err != nil { - return fmt.Errorf("key %q: %w", keyName, err) - } - e.w.WriteString("\n") - } - - // 4. Pass 2: Write Tables - for _, k := range tables { - val := e.resolveValue(rv, k) - keyName := e.getKeyName(rv, k) - - // Determine full path for header - fullKey := keyName + for _, x := range tables { + key, _ := keyText(x.key) if prefix != "" { - fullKey = prefix + "." + keyName + key = prefix + "." + key } - - // Handle specific table types - switch val.Kind() { - case reflect.Struct, reflect.Map: - // [header] - e.w.WriteString("\n") - e.w.WriteString("[" + fullKey + "]\n") - if err := e.encodeTable(val, fullKey); err != nil { + if isTable(x.value) { + e.buf.WriteString("\n[" + key + "]\n") + if err := e.table(x.value, key, depth+1); err != nil { return err } - - case reflect.Slice, reflect.Array: - // [[header]] - for i := 0; i < val.Len(); i++ { - elem := val.Index(i) - // Dereference pointer elements in slice - if elem.Kind() == reflect.Ptr { - if elem.IsNil() { - continue - } - elem = elem.Elem() - } - - e.w.WriteString("\n") - e.w.WriteString("[[" + fullKey + "]]\n") - if err := e.encodeTable(elem, fullKey); err != nil { + } else { + for i := 0; i < x.value.Len(); i++ { + e.buf.WriteString("\n[[" + key + "]]\n") + if err := e.table(x.value.Index(i), key, depth+1); err != nil { return err } } } } - return nil } -// encodeValue writes a single primitive value or inline array -func (e *encoder) encodeValue(v reflect.Value) error { +func isTable(v reflect.Value) bool { return v.Kind() == reflect.Map || v.Kind() == reflect.Struct } +func isTableArray(v reflect.Value) bool { + if v.Kind() != reflect.Slice && v.Kind() != reflect.Array || v.Len() == 0 { + return false + } + for i := 0; i < v.Len(); i++ { + x, err := indirect(v.Index(i)) + if err != nil || !x.IsValid() || !isTable(x) { + return false + } + } + return true +} + +func (e *encoder) value(v reflect.Value, depth int) error { + if depth > maxValueDepth { + return fmt.Errorf("encode nesting exceeds %d (possible cycle)", maxValueDepth) + } + v, err := indirect(v) + if err != nil { + return err + } + if !v.IsValid() { + return fmt.Errorf("nil array element has no TOML representation") + } switch v.Kind() { case reflect.Bool: - if v.Bool() { - e.w.WriteString("true") - } else { - e.w.WriteString("false") - } - + e.buf.WriteString(strconv.FormatBool(v.Bool())) case reflect.String: - e.encodeString(v.String()) - + if !utf8.ValidString(v.String()) { + return fmt.Errorf("invalid UTF-8 string") + } + e.string(v.String()) case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - e.w.WriteString(strconv.FormatInt(v.Int(), 10)) - + e.buf.WriteString(strconv.FormatInt(v.Int(), 10)) case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - e.w.WriteString(strconv.FormatUint(v.Uint(), 10)) - + if v.Uint() > math.MaxInt64 { + return fmt.Errorf("unsigned integer %d exceeds TOML int64 range", v.Uint()) + } + e.buf.WriteString(strconv.FormatUint(v.Uint(), 10)) case reflect.Float32, reflect.Float64: f := v.Float() - // NaN/Inf have no TOML representation; emitting them produces - // output the parser rejects if math.IsNaN(f) || math.IsInf(f, 0) { - return fmt.Errorf("cannot encode %v as TOML float", f) + return fmt.Errorf("non-finite float") } - // bitSize matches kind (float32 emitted noise digits); - // 'g' avoids ~100-digit 'f' expansions for large exponents - bits := 64 - if v.Kind() == reflect.Float32 { - bits = 32 + s := strconv.FormatFloat(f, 'g', -1, v.Type().Bits()) + if !strings.ContainsAny(s, ".eE") { + s += ".0" } - str := strconv.FormatFloat(f, 'g', -1, bits) - if !strings.ContainsAny(str, ".eE") { - str += ".0" - } - e.w.WriteString(str) - + e.buf.WriteString(s) case reflect.Slice, reflect.Array: - // Inline array: [1, 2, "3"] - e.w.WriteString("[") + e.buf.WriteByte('[') for i := 0; i < v.Len(); i++ { if i > 0 { - e.w.WriteString(", ") + e.buf.WriteString(", ") } - if err := e.encodeValue(v.Index(i)); err != nil { + if err := e.value(v.Index(i), depth+1); err != nil { return err } } - e.w.WriteString("]") - - case reflect.Interface: - if v.IsNil() { - return nil // Should be handled by caller usually + e.buf.WriteByte(']') + case reflect.Struct, reflect.Map: + fields, err := entries(v) + if err != nil { + return err } - return e.encodeValue(v.Elem()) - + e.buf.WriteByte('{') + for i, x := range fields { + if i > 0 { + e.buf.WriteString(", ") + } + key, _ := keyText(x.key) + e.buf.WriteString(key + " = ") + if err := e.value(x.value, depth+1); err != nil { + return err + } + } + e.buf.WriteByte('}') default: - return fmt.Errorf("unsupported type: %v", v.Kind()) + return fmt.Errorf("unsupported type %s", v.Type()) } return nil } -// --- Helpers --- - -// getSortedKeys returns all field names (struct) or keys (map) sorted -func (e *encoder) getSortedKeys(rv reflect.Value) ([]string, error) { - var keys []string - - if rv.Kind() == reflect.Map { - for _, key := range rv.MapKeys() { - if key.Kind() != reflect.String { - return nil, fmt.Errorf("map key must be string, got %v", key.Kind()) - } - keys = append(keys, key.String()) - } - } else if rv.Kind() == reflect.Struct { - typ := rv.Type() - for i := 0; i < rv.NumField(); i++ { - field := typ.Field(i) - // Skip unexported - if field.PkgPath != "" { - continue - } - // Skip if tag is "-" - tag := field.Tag.Get("toml") - if tag == "-" { - continue - } - keys = append(keys, field.Name) - } +func keyText(s string) (string, error) { + if !utf8.ValidString(s) { + return "", fmt.Errorf("invalid UTF-8 key") } - // Sort by emitted key (tag-resolved), not Go field name. - // Identity for maps (getKeyName returns the key itself). - sort.Slice(keys, func(i, j int) bool { - return e.getKeyName(rv, keys[i]) < e.getKeyName(rv, keys[j]) - }) - return keys, nil + if numericKey(s) { + return "", fmt.Errorf("numeric keys are forbidden: %q", s) + } + if isBareKey(s) { + return s, nil + } + var e encoder + e.string(s) + return e.buf.String(), nil } -// resolveValue extracts the value from struct field or map key -// It also handles interface unwrapping for map[string]any. -func (e *encoder) resolveValue(container reflect.Value, key string) reflect.Value { - var val reflect.Value - if container.Kind() == reflect.Map { - val = container.MapIndex(reflect.ValueOf(key)) - } else { - val = container.FieldByName(key) - } - - // Unwrap interface if needed - if val.Kind() == reflect.Interface && !val.IsNil() { - val = val.Elem() - } - - // Dereference pointer if needed (but keep nil ptrs for checking) - if val.Kind() == reflect.Ptr && !val.IsNil() { - val = val.Elem() - } - - return val -} - -// getKeyName resolves the TOML key name (handles struct tags) -// For maps, the key name is the key itself -func (e *encoder) getKeyName(container reflect.Value, realName string) string { - if container.Kind() == reflect.Map { - return realName - } - // Struct: lookup tag - field, _ := container.Type().FieldByName(realName) - tag := field.Tag.Get("toml") - if tag != "" { - parts := strings.Split(tag, ",") - if parts[0] != "" { - return parts[0] +func (e *encoder) string(s string) { + e.buf.WriteByte('"') + for _, r := range s { + switch r { + case '"': + e.buf.WriteString(`\"`) + case '\\': + e.buf.WriteString(`\\`) + case '\n': + e.buf.WriteString(`\n`) + case '\r': + e.buf.WriteString(`\r`) + case '\t': + e.buf.WriteString(`\t`) + case '\b': + e.buf.WriteString(`\b`) + case '\f': + e.buf.WriteString(`\f`) + default: + if r < 0x20 || r == 0x7f { + fmt.Fprintf(&e.buf, `\u%04X`, r) + } else { + e.buf.WriteRune(r) + } } } - return realName + e.buf.WriteByte('"') } -// shouldSkip returns true if the field should be omitted (nil ptr, omitempty) -func (e *encoder) shouldSkip(container reflect.Value, realName string, val reflect.Value) bool { - // Skip nil pointers / interfaces - if (val.Kind() == reflect.Ptr || val.Kind() == reflect.Interface) && val.IsNil() { - return true - } - - // Maps don't have tags, so we only skip nil values - if container.Kind() == reflect.Map { +func isBareKey(s string) bool { + if s == "" || s == "true" || s == "false" { return false } - - // Structs: check omitempty - field, _ := container.Type().FieldByName(realName) - tag := field.Tag.Get("toml") - if strings.Contains(tag, "omitempty") && isEmptyValue(val) { - return true + if s[0] >= '0' && s[0] <= '9' || len(s) > 1 && s[0] == '-' && s[1] >= '0' && s[1] <= '9' { + return false } - - return false -} - -// isTable determines if a value renders as [Table] / [[Array of Tables]]. -// Slices must be homogeneous: mixing table and scalar elements is an error -// (parser accepts such arrays; encoder cannot represent them — documented -// limitation). Nil pointer elements are allowed in table slices and skipped -// at emission; nil interface elements are an error. -func (e *encoder) isTable(v reflect.Value) (bool, error) { - if v.Kind() == reflect.Interface { - v = v.Elem() - } - if v.Kind() == reflect.Ptr { - v = v.Elem() - } - - switch v.Kind() { - case reflect.Struct, reflect.Map: - return true, nil - case reflect.Slice, reflect.Array: - tables, scalars := 0, 0 - for i := 0; i < v.Len(); i++ { - elem := v.Index(i) - if elem.Kind() == reflect.Interface { - if elem.IsNil() { - return false, fmt.Errorf("nil element at index %d", i) - } - elem = elem.Elem() - } - if elem.Kind() == reflect.Ptr { - if elem.IsNil() { - tables++ // nil pointers are only legal in [[table]] context (skipped) - continue - } - elem = elem.Elem() - } - switch elem.Kind() { - case reflect.Struct, reflect.Map: - tables++ - default: - scalars++ - } + for _, r := range s { + if !isAlpha(r) && !isDigit(r) && r != '_' && r != '-' { + return false } - if tables > 0 && scalars > 0 { - return false, fmt.Errorf("mixed table/scalar elements in array") - } - return tables > 0, nil } - return false, nil + return true } func isEmptyValue(v reflect.Value) bool { switch v.Kind() { case reflect.Array, reflect.Map, reflect.Slice, reflect.String: return v.Len() == 0 - case reflect.Bool: - return !v.Bool() - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return v.Int() == 0 - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return v.Uint() == 0 - case reflect.Float32, reflect.Float64: - return v.Float() == 0 - case reflect.Interface, reflect.Ptr: - return v.IsNil() + default: + return v.IsZero() } - return false } - -func (e *encoder) writeKey(s string) error { - if isBareKey(s) { - _, err := e.w.WriteString(s) - return err - } - e.encodeString(s) - return nil -} - -func (e *encoder) encodeString(s string) { - e.w.WriteString("\"") - for _, r := range s { - switch r { - case '"': - e.w.WriteString(`\"`) - case '\\': - e.w.WriteString(`\\`) - case '\n': - e.w.WriteString(`\n`) - case '\r': - e.w.WriteString(`\r`) - case '\t': - e.w.WriteString(`\t`) - case '\b': - e.w.WriteString(`\b`) - case '\f': - e.w.WriteString(`\f`) - default: - if r < 0x20 || r == 0x7F { - e.w.WriteString(fmt.Sprintf(`\u%04X`, r)) - } else { - e.w.WriteRune(r) - } - } - } - e.w.WriteString("\"") -} - -// isBareKey determines if a string can be written as a bare key -// It follows the TOML spec (A-Za-z0-9_-) but also respects the provided Lexer's behavior -// If the Lexer would interpret the string as a Number or Boolean, it must be quoted because the Parser expects TokenIdent (or TokenString) for keys -func isBareKey(s string) bool { - if s == "" { - return false - } - - // 1. Valid bare key characters: A-Za-z0-9_- - for _, r := range s { - if !((r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '_' || r == '-') { - return false - } - } - - // 2. Avoid collision with Booleans - // Lexer emits TokenBool for these, Parser expects TokenIdent. - if s == "true" || s == "false" { - return false - } - - // 3. Avoid collision with Numbers - // The provided Lexer triggers number parsing if the token starts with a digit, - // or a '-' followed by a digit. - // Since the Lexer never produces TokenIdent for these cases (it produces TokenInteger/Float), - // and the Parser rejects numeric tokens as keys, we must quote them. - c0 := s[0] - if c0 >= '0' && c0 <= '9' { - return false - } - if c0 == '-' && len(s) > 1 { - c1 := s[1] - if c1 >= '0' && c1 <= '9' { - return false - } - } - - return true -} - diff --git a/fuzz_test.go b/fuzz_test.go index b1f835d..e48bac5 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -2,8 +2,8 @@ package toml import ( "reflect" + "strings" "testing" - "unicode/utf8" ) func FuzzParse(f *testing.F) { @@ -57,24 +57,25 @@ func FuzzRoundTrip(f *testing.F) { `m = { x = 1, y = { z = "q" } }`, `u = "日本語 \u00E9"`, "big = 1e100\nsmall = 1e-100", + `"a.b" = { "c d" = 1 }`, + `mixed = [1, {value = "x"}, [{nested = true}]]`, + "[[outer]]\n[outer.\"inner.key\"]\nx = \"\\b\\f\"\n[[outer]]\nx = 2", } { f.Add([]byte(s)) } f.Fuzz(func(t *testing.T, data []byte) { - // Encoder normalizes invalid UTF-8 to U+FFFD (WriteRune); byte-exact - // round trip is only a property of valid input. - if !utf8.Valid(data) { - t.Skip() - } m1, err := NewParser(data).Parse() if err != nil { t.Skip() } out, err := Marshal(m1) if err != nil { - // Parser accepts values the encoder cannot represent - // (mixed table/scalar arrays). Documented asymmetry. - t.Skip() + // Separate dotted paths may form a tree deeper than the encoder's + // documented whole-value bound. No other encode failure is allowed. + if strings.Contains(err.Error(), "nesting exceeds") { + t.Skip() + } + t.Fatalf("parsed value cannot be encoded: %v", err) } m2, err := NewParser(out).Parse() if err != nil { diff --git a/go.mod b/go.mod index e988fa4..9438af2 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/lixenwraith/toml -go 1.24.0 +go 1.27.1 diff --git a/hardening_test.go b/hardening_test.go new file mode 100644 index 0000000..0b42be4 --- /dev/null +++ b/hardening_test.go @@ -0,0 +1,141 @@ +package toml + +import ( + "math" + "reflect" + "strings" + "testing" +) + +func TestCheckedNumericConversions(t *testing.T) { + type Count uint64 + tests := []struct { + value any + target any + want any + valid bool + }{ + {uint(42), new(int64), int64(42), true}, + {Count(math.MaxUint64), new(uint64), uint64(math.MaxUint64), true}, + {uint64(math.MaxUint64), new(int64), nil, false}, + {int64(-1), new(uint64), nil, false}, + {float64(0x1p63), new(int64), nil, false}, + {float64(-0x1p63), new(int64), int64(math.MinInt64), true}, + {float64(0x1p64), new(uint64), nil, false}, + {1.5, new(int), nil, false}, + {math.NaN(), new(int), nil, false}, + {math.Inf(1), new(uint64), nil, false}, + {float32(12), new(uint8), uint8(12), true}, + {uint64(1<<53) + 1, new(float64), nil, false}, + {int64(1<<24) + 1, new(float32), nil, false}, + {uint64(1 << 63), new(float64), float64(1 << 63), true}, + {1e100, new(float32), nil, false}, + } + for _, tt := range tests { + err := Decode(tt.value, tt.target) + if (err == nil) != tt.valid { + t.Errorf("%v (%T) -> %T: %v", tt.value, tt.value, tt.target, err) + } + if tt.valid && !reflect.DeepEqual(reflect.ValueOf(tt.target).Elem().Interface(), tt.want) { + t.Errorf("got %v, want %v", tt.target, tt.want) + } + } +} + +func TestDecodeTransactionAndNamedKeys(t *testing.T) { + type Key string + var m map[Key]int + if err := Decode(map[string]any{"x": 4}, &m); err != nil || m["x"] != 4 { + t.Fatalf("%v %#v", err, m) + } + type C struct { + First int `toml:",omitempty"` + Second int8 + } + c := C{7, 8} + if err := Decode(map[string]any{"First": 11, "Second": 400}, &c); err == nil || c != (C{7, 8}) { + t.Fatalf("partial update: %+v %v", c, err) + } + if err := Decode(map[string]any{"First": 11}, &c); err != nil || c.First != 11 { + t.Fatalf("empty tag name: %+v %v", c, err) + } +} + +func TestRoundTripSupportedValues(t *testing.T) { + input := map[string]any{ + "dot.key": map[string]any{"quoted\"key": map[string]any{"x": "\b\f\x00\x7f界"}}, + "items": []any{map[string]any{"x": int64(3)}, map[string]any{"x": int64(4)}}, + "nested": []any{[]any{map[string]any{"x": true}}}, + "mixed": []any{int64(1), map[string]any{"y": "text"}}, + } + data, err := Marshal(input) + if err != nil { + t.Fatal(err) + } + got, err := NewParser(data).Parse() + if err != nil { + t.Fatalf("%s: %v", data, err) + } + if !reflect.DeepEqual(normalize(input), normalize(got)) { + t.Fatalf("round trip: %#v", got) + } +} + +func TestMarshalRejectsInvalidValues(t *testing.T) { + cycle := map[string]any{} + cycle["self"] = cycle + for _, value := range []any{ + map[string]any{"u": uint64(math.MaxUint64)}, + map[string]any{"s": string([]byte{0xff})}, + map[string]any{"123": 1}, + struct { + A int `toml:"x"` + B int `toml:"x"` + }{}, + cycle, + } { + if _, err := Marshal(value); err == nil { + t.Errorf("invalid %T accepted", value) + } + } +} + +func TestParseBoundariesAndSyntax(t *testing.T) { + m, err := NewParser([]byte("min = -9223372036854775808\nmax = 9223372036854775807")).Parse() + if err != nil || m["min"] != int64(math.MinInt64) || m["max"] != int64(math.MaxInt64) { + t.Fatalf("%#v %v", m, err) + } + for _, s := range []string{"x=9223372036854775808", "x=1 y=2", "[x] y=2", "x=[1\n2]", "x={a=1\nb=2}", "x=\"\x00\"", "x=\"\xff\"", "[" + strings.Repeat("a.", 1100) + "b]"} { + if _, err := NewParser([]byte(s)).Parse(); err == nil { + t.Errorf("accepted invalid input %q", s) + } + } +} + +func FuzzIntegerConversions(f *testing.F) { + f.Add(uint64(math.MaxUint64)) + f.Add(uint64(1<<53) + 1) + f.Add(uint64(0)) + f.Add(uint64(255)) + f.Add(uint64(256)) + f.Add(uint64(1 << 63)) + f.Fuzz(func(t *testing.T, n uint64) { + var u uint64 + if err := Decode(n, &u); err != nil || u != n { + t.Fatalf("uint64 changed: %d -> %d (%v)", n, u, err) + } + var i int64 + err := Decode(n, &i) + if (err == nil) != (n <= math.MaxInt64) { + t.Fatalf("signed boundary: %d %v", n, err) + } + if err == nil && uint64(i) != n { + t.Fatal("signed conversion changed value") + } + var b uint8 + err = Decode(n, &b) + if (err == nil) != (n <= math.MaxUint8) || err == nil && uint64(b) != n { + t.Fatalf("narrow conversion changed value: %d -> %d (%v)", n, b, err) + } + }) +} diff --git a/integration_hardening_test.go b/integration_hardening_test.go new file mode 100644 index 0000000..2d72414 --- /dev/null +++ b/integration_hardening_test.go @@ -0,0 +1,141 @@ +package toml + +import ( + "math" + "reflect" + "strconv" + "strings" + "testing" +) + +func TestDecodeFixedArrayRoundTrip(t *testing.T) { + type Item struct { + Value uint16 `toml:"value"` + } + type Config struct { + Numbers [3]int64 `toml:"numbers"` + Items [2]*Item `toml:"items"` + Matrix [2][2]int8 `toml:"matrix"` + Empty [0]string `toml:"empty"` + } + want := Config{ + Numbers: [3]int64{math.MinInt64, 0, math.MaxInt64}, + Items: [2]*Item{{1}, {math.MaxUint16}}, + Matrix: [2][2]int8{{-128, 127}, {0, 1}}, + } + data, err := Marshal(want) + if err != nil { + t.Fatal(err) + } + var got Config + if err := Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("got %#v, want %#v", got, want) + } + for _, input := range []any{[]any{1}, []any{1, 2, 3}, []any{1, 256}} { + dst := [2]uint8{7, 8} + if err := Decode(input, &dst); err == nil || dst != [2]uint8{7, 8} { + t.Fatalf("invalid array changed target: %v (%v)", dst, err) + } + } +} + +func TestDecodePointerDefaultsAndRollback(t *testing.T) { + type Settings struct { + Port uint16 `toml:"port"` + Name string `toml:"name"` + } + original := &Settings{Port: 8080, Name: "default"} + dst := original + if err := Decode(map[string]any{"name": "updated"}, &dst); err != nil { + t.Fatal(err) + } + if dst.Port != 8080 || dst.Name != "updated" || dst == original || original.Name != "default" { + t.Fatalf("defaults or original pointer changed: dst=%+v original=%+v", dst, original) + } + type Config struct { + Settings *Settings + Map map[string]int + Slice []int + Limit uint8 + } + cfg := Config{original, map[string]int{"old": 1}, []int{2}, 3} + err := Decode(map[string]any{ + "Settings": map[string]any{"port": 9090}, + "Map": map[string]any{"new": 4}, + "Slice": []any{5}, + "Limit": 256, + }, &cfg) + if err == nil || cfg.Settings != original || *original != (Settings{8080, "default"}) || + !reflect.DeepEqual(cfg.Map, map[string]int{"old": 1}) || !reflect.DeepEqual(cfg.Slice, []int{2}) || cfg.Limit != 3 { + t.Fatalf("partial update: %+v (%v)", cfg, err) + } +} + +func TestDecodeCyclicInputErrors(t *testing.T) { + cycle := map[string]any{} + cycle["self"] = cycle + var number uint64 + if err := Decode(cycle, &number); err == nil { + t.Fatal("accepted cyclic map as unsigned number") + } + type Node struct { + Next *Node `toml:"self"` + } + var node Node + if err := Decode(cycle, &node); err == nil || node.Next != nil { + t.Fatalf("recursive decode must fail without publication: %v", err) + } +} + +func TestNumericKeyRangeIndependence(t *testing.T) { + for _, key := range []string{"0", "-1", "+1", "9223372036854775808", "-9223372036854775809", strings.Repeat("9", 100)} { + if _, err := Marshal(map[string]any{key: 1}); err == nil { + t.Errorf("encoded numeric key %q", key) + } + if _, err := NewParser([]byte(strconv.Quote(key) + " = 1")).Parse(); err == nil { + t.Errorf("parsed numeric key %q", key) + } + } +} + +func TestDecodeIntegerWidths(t *testing.T) { + for _, typ := range []reflect.Type{ + reflect.TypeFor[int](), reflect.TypeFor[int8](), reflect.TypeFor[int16](), reflect.TypeFor[int32](), reflect.TypeFor[int64](), + reflect.TypeFor[uint](), reflect.TypeFor[uint8](), reflect.TypeFor[uint16](), reflect.TypeFor[uint32](), reflect.TypeFor[uint64](), + } { + t.Run(typ.String(), func(t *testing.T) { + bits := typ.Bits() + signed := typ.Kind() >= reflect.Int && typ.Kind() <= reflect.Int64 + max := uint64(math.MaxUint64) >> (64 - bits) + if signed { + max >>= 1 + } + dst := reflect.New(typ) + if err := Decode(max, dst.Interface()); err != nil { + t.Fatal(err) + } + if max < math.MaxUint64 { + before := dst.Elem().Interface() + if err := Decode(max+1, dst.Interface()); err == nil || dst.Elem().Interface() != before { + t.Fatalf("overflow was accepted or mutated destination: %v", err) + } + } + if signed { + min := -int64(max) - 1 + if err := Decode(min, dst.Interface()); err != nil || dst.Elem().Int() != min { + t.Fatalf("minimum changed: %v", err) + } + if min > math.MinInt64 { + if err := Decode(min-1, dst.Interface()); err == nil || dst.Elem().Int() != min { + t.Fatalf("underflow accepted: %v", err) + } + } + } else if err := Decode(int64(-1), dst.Interface()); err == nil { + t.Fatal("negative signed integer accepted") + } + }) + } +} diff --git a/lexer.go b/lexer.go index 92f2014..8d28d46 100644 --- a/lexer.go +++ b/lexer.go @@ -329,7 +329,7 @@ func (l *Lexer) readString() Token { for l.pos < len(l.input) { ch := l.peek() - if ch == '\n' { + if ch < 0x20 && ch != '\t' || ch == 0x7f { return l.newToken(TokenError, "unterminated string: newline in basic string") } if ch == '"' && !escaped { @@ -353,6 +353,10 @@ func (l *Lexer) readString() Token { result = append(result, '\t') case 'r': result = append(result, '\r') + case 'b': + result = append(result, '\b') + case 'f': + result = append(result, '\f') case 'u', 'U': n := 4 if ch == 'U' { @@ -427,4 +431,3 @@ func isOctalDigit(r rune) bool { func isBinaryDigit(r rune) bool { return r == '0' || r == '1' } - diff --git a/monkey_test.go b/monkey_test.go index 546f559..db23b2a 100644 --- a/monkey_test.go +++ b/monkey_test.go @@ -62,7 +62,8 @@ func TestDecode_DeepPointers(t *testing.T) { func TestDecode_LargeIntPrecision(t *testing.T) { largeVal := int64(4611686018427387905) - data := map[string]any{"id": int(largeVal)} + // Keep the source value intact on 32-bit platforms too. + data := map[string]any{"id": largeVal} type T struct { ID int64 `toml:"id"` @@ -191,11 +192,11 @@ mid_val = 2 } a := res["a"].(map[string]any) - if a["root_val"] != 1 { + if a["root_val"] != int64(1) { t.Errorf("Missing root_val: %v", a["root_val"]) } b := a["b"].(map[string]any) - if b["mid_val"] != 2 { + if b["mid_val"] != int64(2) { t.Errorf("Missing mid_val: %v", b["mid_val"]) } } @@ -384,7 +385,7 @@ i = 5 a := res["a"].(map[string]any) b := a["b"].(map[string]any) - if b["i"] != 5 { + if b["i"] != int64(5) { t.Errorf("Value 'i' lost in table reentry. Got %v", b["i"]) } if _, ok := b["c"].(map[string]any); !ok { @@ -514,4 +515,4 @@ func TestParser_KeyValueContext(t *testing.T) { if err == nil { t.Error("Float key should have been rejected") } -} \ No newline at end of file +} diff --git a/parser.go b/parser.go index c919a49..1e74c93 100644 --- a/parser.go +++ b/parser.go @@ -4,11 +4,9 @@ import ( "fmt" "reflect" "strconv" + "unicode/utf8" ) -// Enforce 64-bit platform; fails compilation on 32-bit targets where int(int64) in parseInteger would truncate -const _ uint = 1<<63 - 1 - // Parser parses TOML tokens into a map[string]any type Parser struct { root map[string]any @@ -49,6 +47,9 @@ func (p *Parser) nextToken() { } func (p *Parser) Parse() (map[string]any, error) { + if !utf8.Valid(p.lexer.input) { + return nil, fmt.Errorf("invalid UTF-8") + } for p.curToken.Type != TokenEOF { if p.curToken.Type == TokenNewline { p.nextToken() @@ -58,6 +59,9 @@ func (p *Parser) Parse() (map[string]any, error) { if err := p.parseStatement(); err != nil { return nil, err } + if p.curToken.Type != TokenNewline && p.curToken.Type != TokenEOF { + return nil, fmt.Errorf("expected newline at line %d", p.curToken.Line) + } } return p.root, nil } @@ -254,7 +258,7 @@ func (p *Parser) parseKeyParts() ([]string, error) { if p.curToken.Type == TokenString { // Rule: Even quoted strings shouldn't be pure numbers per instruction - if _, err := strconv.Atoi(p.curToken.Literal); err == nil { + if numericKey(p.curToken.Literal) { return nil, fmt.Errorf("numeric string keys are forbidden: %q", p.curToken.Literal) } } @@ -263,6 +267,9 @@ func (p *Parser) parseKeyParts() ([]string, error) { return nil, fmt.Errorf("expected key, got %s", p.curToken.String()) } + if len(keys) >= maxValueDepth { + return nil, fmt.Errorf("key nesting exceeds %d", maxValueDepth) + } keys = append(keys, p.curToken.Literal) p.nextToken() @@ -314,7 +321,7 @@ func (p *Parser) parseValue() (any, error) { return nil, fmt.Errorf("unexpected value token %s at line %d", p.curToken.String(), p.curToken.Line) } -func (p *Parser) parseInteger(lit string) (int, error) { +func (p *Parser) parseInteger(lit string) (int64, error) { // Handle optional leading sign negative := false numLit := lit @@ -329,26 +336,23 @@ func (p *Parser) parseInteger(lit string) (int, error) { if len(numLit) > 2 && numLit[0] == '0' { switch numLit[1] { case 'x', 'X': - val, err = strconv.ParseInt(numLit[2:], 16, 64) + val, err = strconv.ParseInt(signPrefix(negative)+numLit[2:], 16, 64) case 'o', 'O': - val, err = strconv.ParseInt(numLit[2:], 8, 64) + val, err = strconv.ParseInt(signPrefix(negative)+numLit[2:], 8, 64) case 'b', 'B': - val, err = strconv.ParseInt(numLit[2:], 2, 64) + val, err = strconv.ParseInt(signPrefix(negative)+numLit[2:], 2, 64) default: val, err = strconv.ParseInt(lit, 10, 64) - return int(val), err + return val, err } if err != nil { return 0, err } - if negative { - val = -val - } - return int(val), nil + return val, nil } val, err = strconv.ParseInt(lit, 10, 64) - return int(val), err + return val, err } func (p *Parser) parseArray() ([]any, error) { @@ -367,15 +371,12 @@ func (p *Parser) parseArray() ([]any, error) { } arr = append(arr, val) + for p.curToken.Type == TokenNewline { + p.nextToken() + } if p.curToken.Type == TokenComma { p.nextToken() } else if p.curToken.Type != TokenRBracket { - // Check for newlines between elements if missing comma? TOML usually requires comma. - // Relaxed parser: require comma unless followed immediately by bracket (trailing comma allowed) - if p.curToken.Type == TokenNewline { - p.nextToken() - continue - } return nil, fmt.Errorf("expected comma or closing bracket in array at line %d", p.curToken.Line) } } @@ -414,13 +415,12 @@ func (p *Parser) parseInlineTable() (map[string]any, error) { return nil, err } + for p.curToken.Type == TokenNewline { + p.nextToken() + } if p.curToken.Type == TokenComma { p.nextToken() } else if p.curToken.Type != TokenRBrace { - if p.curToken.Type == TokenNewline { - p.nextToken() - continue - } return nil, fmt.Errorf("expected comma or closing brace in inline table at line %d", p.curToken.Line) } } @@ -433,3 +433,26 @@ func (p *Parser) parseInlineTable() (map[string]any, error) { p.frozen[reflect.ValueOf(m).Pointer()] = true return m, nil } + +func signPrefix(negative bool) string { + if negative { + return "-" + } + return "" +} + +// Keep the package's decimal-key restriction independent of numeric range. +func numericKey(s string) bool { + if len(s) > 0 && (s[0] == '+' || s[0] == '-') { + s = s[1:] + } + if s == "" { + return false + } + for _, c := range s { + if !isDigit(c) { + return false + } + } + return true +} diff --git a/toml_test.go b/toml_test.go index 8cdf2d6..a6d6dc1 100644 --- a/toml_test.go +++ b/toml_test.go @@ -81,7 +81,7 @@ active = false } // Check int conversion in dynamic map (parser returns int/float, decode handles struct fields, but map keeps raw parser types) // Parser likely returns int for 55. - if id, ok := cfg.Owner["id"].(int); !ok || id != 55 { + if id, ok := cfg.Owner["id"].(int64); !ok || id != 55 { // Fallback check if parser returned generic float for number if fId, okf := cfg.Owner["id"].(float64); !okf || fId != 55 { t.Errorf("Owner.ID mismatch: got %T %v", cfg.Owner["id"], cfg.Owner["id"]) @@ -481,4 +481,4 @@ speed = 3.0 if cfg.Entities["enemy"] == nil || cfg.Entities["enemy"].Health != 50 { t.Errorf("enemy mismatch: %+v", cfg.Entities["enemy"]) } -} \ No newline at end of file +}