perf: cut Parse, Unmarshal and Marshal allocations
Unmarshal: map fields keep their state in a slice indexed by field slot, size their maps and struct elements from a count of matching keys, and find pending elements linearly up to 16 names. 47 to 16 allocs, +47% MB/s. Parse: duplicate keys are found after parsing with a seeded open-addressing table on the stack instead of a map. 5 to 2 allocs, +50% MB/s. Warnings stay in line order. Marshal: the output is sized from the map lengths, name and order buffers are reused across maps, and needsQuote scans once. 19 to 10 allocs. Fixes found by fuzzing: a quoted escape that decodes to invalid UTF-8 is kept as written, and Set refuses a key starting with U+FEFF.
- commit
- 59c327c479f1be24edb1943b4bb4d18c9d1d292b
- author
- brice · · 2026-10-10 07:00 UTC
- parent
- ebc8127
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +380 -93
M decode.go +129 -65
| @@ -74,10 +74,12 @@ type field struct { | |||
| 74 | 74 | maxText string | |
| 75 | 75 | oneof []string | |
| 76 | 76 | sep string | |
| 77 | - | item *field // element of a slice | |
| 78 | - | val *field // element of a map of values | |
| 79 | - | elem *schema // element of a map of structs | |
| 80 | - | defaults bool // map element implements Defaulter | |
| 77 | + | item *field // element of a slice | |
| 78 | + | val *field // element of a map of values | |
| 79 | + | elem *schema // element of a map of structs | |
| 80 | + | defaults bool // map element implements Defaulter | |
| 81 | + | elems reflect.Type // []element of a map of structs | |
| 82 | + | slot int // index of a map field in schema.prefixes | |
| 81 | 83 | } | |
| 82 | 84 | ||
| 83 | 85 | type schema struct { | |
| @@ -111,6 +113,9 @@ func buildSchema(t reflect.Type) (*schema, error) { | |||
| 111 | 113 | return nil, err | |
| 112 | 114 | } | |
| 113 | 115 | slices.SortStableFunc(s.prefixes, func(a, b *field) int { return len(b.key) - len(a.key) }) | |
| 116 | + | for i, f := range s.prefixes { | |
| 117 | + | f.slot = i | |
| 118 | + | } | |
| 114 | 119 | return s, nil | |
| 115 | 120 | } | |
| 116 | 121 | ||
| @@ -170,7 +175,7 @@ func (s *schema) add(t reflect.Type, prefix string, index []int) error { | |||
| 170 | 175 | if elem.catchAll != nil || len(elem.prefixes) > 0 { | |
| 171 | 176 | return fmt.Errorf("kvconf: %s: a map element cannot hold maps or kv:\"*\"", where) | |
| 172 | 177 | } | |
| 173 | - | f = &field{elem: elem, defaults: reflect.PointerTo(et).Implements(defaulterType)} | |
| 178 | + | f = &field{elem: elem, elems: reflect.SliceOf(et), defaults: reflect.PointerTo(et).Implements(defaulterType)} | |
| 174 | 179 | } | |
| 175 | 180 | f.key, f.index, f.typ = prefix+strings.TrimSuffix(name, "*"), idx, sf.Type | |
| 176 | 181 | s.prefixes = append(s.prefixes, f) | |
| @@ -287,9 +292,10 @@ func Decode(entries []Entry, v any, opts Options) ([]Warning, error) { | |||
| 287 | 292 | if err != nil { | |
| 288 | 293 | return nil, err | |
| 289 | 294 | } | |
| 290 | - | st := decodeState{root: rv.Elem()} | |
| 295 | + | st := decodeState{root: rv.Elem(), entries: entries} | |
| 291 | 296 | p := problems{strict: opts.Strict} | |
| 292 | 297 | for i := range entries { | |
| 298 | + | st.next = i | |
| 293 | 299 | e := &entries[i] | |
| 294 | 300 | msg, known := s.apply(&st, e) | |
| 295 | 301 | if !known { | |
| @@ -302,7 +308,7 @@ func Decode(entries []Entry, v any, opts Options) ([]Warning, error) { | |||
| 302 | 308 | return p.list, p.err() | |
| 303 | 309 | } | |
| 304 | 310 | } | |
| 305 | - | st.flush() | |
| 311 | + | st.flush(s) | |
| 306 | 312 | if val, ok := v.(Validator); ok { | |
| 307 | 313 | if err := val.Validate(); err != nil { | |
| 308 | 314 | return p.list, err | |
| @@ -314,66 +320,128 @@ func Decode(entries []Entry, v any, opts Options) ([]Warning, error) { | |||
| 314 | 320 | // decodeState holds the scratch values of one Decode call, so a key of a | |
| 315 | 321 | // map does not allocate. | |
| 316 | 322 | type decodeState struct { | |
| 317 | - | root reflect.Value | |
| 318 | - | // scratch key and element per map field, reused for every entry. | |
| 319 | - | keys, vals map[*field]reflect.Value | |
| 320 | - | // pending struct elements, written back to their maps once at the end | |
| 321 | - | // so each name is copied out of its map once, not once per key. | |
| 322 | - | pending map[*field]map[string]*pendingElem | |
| 323 | + | root reflect.Value | |
| 324 | + | entries []Entry | |
| 325 | + | next int // index in entries of the entry being applied | |
| 326 | + | maps []mapState // by field.slot; nil until a key reaches a map | |
| 327 | + | } | |
| 328 | + | ||
| 329 | + | // mapState decodes the keys of one kv:"x.*" field. | |
| 330 | + | type mapState struct { | |
| 331 | + | m reflect.Value // the map field; a map of values is created on first use | |
| 332 | + | plain map[string]string // m, for the map[string]string fast path | |
| 333 | + | hint int // upper bound of the keys reaching it, to size its storage once | |
| 334 | + | key, val reflect.Value // scratch, reused for every entry | |
| 335 | + | // Struct elements are decoded in elems, then written back once by | |
| 336 | + | // flush, so each name is copied out of the map once, not once per key. | |
| 337 | + | elems reflect.Value // []element | |
| 338 | + | names []pendingName // names[i] is elems[i] | |
| 339 | + | index map[string]int | |
| 323 | 340 | } | |
| 324 | 341 | ||
| 325 | - | type pendingElem struct { | |
| 326 | - | val reflect.Value | |
| 327 | - | set bool // at least one key was accepted | |
| 342 | + | type pendingName struct { | |
| 343 | + | name string | |
| 344 | + | set bool // at least one key was accepted | |
| 328 | 345 | } | |
| 329 | 346 | ||
| 330 | - | func (st *decodeState) scratch(f *field) (key, val reflect.Value) { | |
| 331 | - | if st.keys == nil { | |
| 332 | - | st.keys, st.vals = map[*field]reflect.Value{}, map[*field]reflect.Value{} | |
| 347 | + | // linearNames is how many names are searched linearly before index is built. | |
| 348 | + | const linearNames = 16 | |
| 349 | + | ||
| 350 | + | // mapFor returns the state of the map field f. The first call sets every | |
| 351 | + | // map's hint from the entries left, so a config without map keys never | |
| 352 | + | // counts. A key is counted for every prefix it starts with, so a key that | |
| 353 | + | // apply passes on to a shorter prefix is counted there. | |
| 354 | + | func (st *decodeState) mapFor(s *schema, f *field) *mapState { | |
| 355 | + | if st.maps == nil { | |
| 356 | + | st.maps = make([]mapState, len(s.prefixes)) | |
| 357 | + | for i := st.next; i < len(st.entries); i++ { | |
| 358 | + | key := st.entries[i].Key | |
| 359 | + | for _, p := range s.prefixes { | |
| 360 | + | if len(key) > len(p.key) && key[:len(p.key)] == p.key { | |
| 361 | + | st.maps[p.slot].hint++ | |
| 362 | + | } | |
| 363 | + | } | |
| 364 | + | } | |
| 333 | 365 | } | |
| 334 | - | key, ok := st.keys[f] | |
| 335 | - | if !ok { | |
| 336 | - | key = reflect.New(f.typ.Key()).Elem() | |
| 337 | - | val = reflect.New(f.typ.Elem()).Elem() | |
| 338 | - | st.keys[f], st.vals[f] = key, val | |
| 366 | + | ms := &st.maps[f.slot] | |
| 367 | + | if !ms.m.IsValid() { | |
| 368 | + | ms.m = st.root.FieldByIndex(f.index) | |
| 369 | + | if f.elem == nil { | |
| 370 | + | if ms.m.IsNil() { | |
| 371 | + | ms.m.Set(reflect.MakeMapWithSize(f.typ, ms.hint)) | |
| 372 | + | } | |
| 373 | + | if f.typ == stringMapType && f.val.plain() { | |
| 374 | + | ms.plain = ms.m.Interface().(map[string]string) | |
| 375 | + | } else { | |
| 376 | + | ms.val = reflect.New(f.typ.Elem()).Elem() | |
| 377 | + | } | |
| 378 | + | } | |
| 379 | + | if ms.plain == nil { | |
| 380 | + | ms.key = reflect.New(f.typ.Key()).Elem() | |
| 381 | + | } | |
| 339 | 382 | } | |
| 340 | - | return key, st.vals[f] | |
| 383 | + | return ms | |
| 341 | 384 | } | |
| 342 | 385 | ||
| 343 | - | // element returns the addressable pending element name of the map field f. | |
| 344 | - | func (st *decodeState) element(f *field, m reflect.Value, name string) *pendingElem { | |
| 345 | - | if st.pending == nil { | |
| 346 | - | st.pending = map[*field]map[string]*pendingElem{} | |
| 347 | - | } | |
| 348 | - | byName := st.pending[f] | |
| 349 | - | if byName == nil { | |
| 350 | - | byName = map[string]*pendingElem{} | |
| 351 | - | st.pending[f] = byName | |
| 386 | + | // element returns the index in ms.elems of the pending element name. | |
| 387 | + | func (ms *mapState) element(f *field, name string) int { | |
| 388 | + | if ms.index != nil { | |
| 389 | + | if i, ok := ms.index[name]; ok { | |
| 390 | + | return i | |
| 391 | + | } | |
| 392 | + | } else { | |
| 393 | + | for i := range ms.names { | |
| 394 | + | if ms.names[i].name == name { | |
| 395 | + | return i | |
| 396 | + | } | |
| 397 | + | } | |
| 352 | 398 | } | |
| 353 | - | if p, ok := byName[name]; ok { | |
| 354 | - | return p | |
| 399 | + | i := len(ms.names) | |
| 400 | + | if !ms.elems.IsValid() { | |
| 401 | + | // Every name comes from a key counted in hint: elems never grows. | |
| 402 | + | ms.elems = reflect.MakeSlice(f.elems, ms.hint, ms.hint) | |
| 403 | + | ms.names = make([]pendingName, 0, ms.hint) | |
| 404 | + | } | |
| 405 | + | ms.names = append(ms.names, pendingName{name: name}) | |
| 406 | + | if ms.index != nil { | |
| 407 | + | ms.index[name] = i | |
| 408 | + | } else if len(ms.names) > linearNames { | |
| 409 | + | ms.index = make(map[string]int, ms.hint) | |
| 410 | + | for j, p := range ms.names { | |
| 411 | + | ms.index[p.name] = j | |
| 412 | + | } | |
| 355 | 413 | } | |
| 356 | - | p := &pendingElem{val: reflect.New(f.typ.Elem()).Elem()} | |
| 357 | - | key, _ := st.scratch(f) | |
| 358 | - | key.SetString(name) | |
| 359 | - | if cur := m.MapIndex(key); cur.IsValid() { | |
| 360 | - | p.val.Set(cur) | |
| 414 | + | ev := ms.elems.Index(i) | |
| 415 | + | ms.key.SetString(name) | |
| 416 | + | if cur := ms.m.MapIndex(ms.key); cur.IsValid() { | |
| 417 | + | ev.Set(cur) | |
| 361 | 418 | } else if f.defaults { | |
| 362 | - | p.val.Addr().Interface().(Defaulter).SetDefaults() | |
| 419 | + | ev.Addr().Interface().(Defaulter).SetDefaults() | |
| 363 | 420 | } | |
| 364 | - | byName[name] = p | |
| 365 | - | return p | |
| 421 | + | return i | |
| 366 | 422 | } | |
| 367 | 423 | ||
| 368 | - | // flush writes the pending struct elements back to their maps. | |
| 369 | - | func (st *decodeState) flush() { | |
| 370 | - | for f, byName := range st.pending { | |
| 371 | - | m := st.root.FieldByIndex(f.index) | |
| 372 | - | key, _ := st.scratch(f) | |
| 373 | - | for name, p := range byName { | |
| 424 | + | // flush writes the pending struct elements back to their maps. A map that | |
| 425 | + | // a key reached exists afterwards, even when every key was rejected. | |
| 426 | + | func (st *decodeState) flush(s *schema) { | |
| 427 | + | for slot := range st.maps { | |
| 428 | + | ms, f := &st.maps[slot], s.prefixes[slot] | |
| 429 | + | if f.elem == nil || !ms.m.IsValid() { | |
| 430 | + | continue | |
| 431 | + | } | |
| 432 | + | if ms.m.IsNil() { | |
| 433 | + | n := 0 | |
| 434 | + | for _, p := range ms.names { | |
| 435 | + | if p.set { | |
| 436 | + | n++ | |
| 437 | + | } | |
| 438 | + | } | |
| 439 | + | ms.m.Set(reflect.MakeMapWithSize(f.typ, n)) | |
| 440 | + | } | |
| 441 | + | for i, p := range ms.names { | |
| 374 | 442 | if p.set { | |
| 375 | - | key.SetString(name) | |
| 376 | - | m.SetMapIndex(key, p.val) | |
| 443 | + | ms.key.SetString(p.name) | |
| 444 | + | ms.m.SetMapIndex(ms.key, ms.elems.Index(i)) | |
| 377 | 445 | } | |
| 378 | 446 | } | |
| 379 | 447 | } | |
| @@ -404,26 +472,22 @@ func (s *schema) apply(st *decodeState, e *Entry) (msg string, known bool) { | |||
| 404 | 472 | continue | |
| 405 | 473 | } | |
| 406 | 474 | } | |
| 407 | - | m := root.FieldByIndex(f.index) | |
| 408 | - | if m.IsNil() { | |
| 409 | - | m.Set(reflect.MakeMap(f.typ)) | |
| 410 | - | } | |
| 475 | + | ms := st.mapFor(s, f) | |
| 411 | 476 | if sub != nil { | |
| 412 | 477 | // A name whose every key is rejected is not created. | |
| 413 | - | p := st.element(f, m, name) | |
| 414 | - | if msg = sub.set(p.val.FieldByIndex(sub.index), e.Value); msg == "" { | |
| 415 | - | p.set = true | |
| 478 | + | i := ms.element(f, name) | |
| 479 | + | if msg = sub.set(ms.elems.Index(i).FieldByIndex(sub.index), e.Value); msg == "" { | |
| 480 | + | ms.names[i].set = true | |
| 416 | 481 | } | |
| 417 | 482 | return msg, true | |
| 418 | 483 | } | |
| 419 | - | if f.typ == stringMapType && f.val.plain() { | |
| 420 | - | m.Interface().(map[string]string)[name] = e.Value | |
| 484 | + | if ms.plain != nil { | |
| 485 | + | ms.plain[name] = e.Value | |
| 421 | 486 | return "", true | |
| 422 | 487 | } | |
| 423 | - | key, val := st.scratch(f) | |
| 424 | - | if msg = f.val.set(val, e.Value); msg == "" { | |
| 425 | - | key.SetString(name) | |
| 426 | - | m.SetMapIndex(key, val) | |
| 488 | + | if msg = f.val.set(ms.val, e.Value); msg == "" { | |
| 489 | + | ms.key.SetString(name) | |
| 490 | + | ms.m.SetMapIndex(ms.key, ms.val) | |
| 427 | 491 | } | |
| 428 | 492 | return msg, true | |
| 429 | 493 | } | |
| @@ -74,10 +74,12 @@ type field struct { | |||||
| 74 | maxText string | 74 | maxText string | ||
| 75 | oneof []string | 75 | oneof []string | ||
| 76 | sep string | 76 | sep string | ||
| 77 | - | item *field // element of a slice | 77 | + | item *field // element of a slice |
| 78 | - | val *field // element of a map of values | 78 | + | val *field // element of a map of values |
| 79 | - | elem *schema // element of a map of structs | 79 | + | elem *schema // element of a map of structs |
| 80 | - | defaults bool // map element implements Defaulter | 80 | + | defaults bool // map element implements Defaulter |
| 81 | + | elems reflect.Type // []element of a map of structs | |||
| 82 | + | slot int // index of a map field in schema.prefixes | |||
| 81 | } | 83 | } | ||
| 82 | 84 | ||||
| 83 | type schema struct { | 85 | type schema struct { | ||
| @@ -111,6 +113,9 @@ func buildSchema(t reflect.Type) (*schema, error) { | |||||
| 111 | return nil, err | 113 | return nil, err | ||
| 112 | } | 114 | } | ||
| 113 | slices.SortStableFunc(s.prefixes, func(a, b *field) int { return len(b.key) - len(a.key) }) | 115 | slices.SortStableFunc(s.prefixes, func(a, b *field) int { return len(b.key) - len(a.key) }) | ||
| 116 | + | for i, f := range s.prefixes { | |||
| 117 | + | f.slot = i | |||
| 118 | + | } | |||
| 114 | return s, nil | 119 | return s, nil | ||
| 115 | } | 120 | } | ||
| 116 | 121 | ||||
| @@ -170,7 +175,7 @@ func (s *schema) add(t reflect.Type, prefix string, index []int) error { | |||||
| 170 | if elem.catchAll != nil || len(elem.prefixes) > 0 { | 175 | if elem.catchAll != nil || len(elem.prefixes) > 0 { | ||
| 171 | return fmt.Errorf("kvconf: %s: a map element cannot hold maps or kv:\"*\"", where) | 176 | return fmt.Errorf("kvconf: %s: a map element cannot hold maps or kv:\"*\"", where) | ||
| 172 | } | 177 | } | ||
| 173 | - | f = &field{elem: elem, defaults: reflect.PointerTo(et).Implements(defaulterType)} | 178 | + | f = &field{elem: elem, elems: reflect.SliceOf(et), defaults: reflect.PointerTo(et).Implements(defaulterType)} |
| 174 | } | 179 | } | ||
| 175 | f.key, f.index, f.typ = prefix+strings.TrimSuffix(name, "*"), idx, sf.Type | 180 | f.key, f.index, f.typ = prefix+strings.TrimSuffix(name, "*"), idx, sf.Type | ||
| 176 | s.prefixes = append(s.prefixes, f) | 181 | s.prefixes = append(s.prefixes, f) | ||
| @@ -287,9 +292,10 @@ func Decode(entries []Entry, v any, opts Options) ([]Warning, error) { | |||||
| 287 | if err != nil { | 292 | if err != nil { | ||
| 288 | return nil, err | 293 | return nil, err | ||
| 289 | } | 294 | } | ||
| 290 | - | st := decodeState{root: rv.Elem()} | 295 | + | st := decodeState{root: rv.Elem(), entries: entries} |
| 291 | p := problems{strict: opts.Strict} | 296 | p := problems{strict: opts.Strict} | ||
| 292 | for i := range entries { | 297 | for i := range entries { | ||
| 298 | + | st.next = i | |||
| 293 | e := &entries[i] | 299 | e := &entries[i] | ||
| 294 | msg, known := s.apply(&st, e) | 300 | msg, known := s.apply(&st, e) | ||
| 295 | if !known { | 301 | if !known { | ||
| @@ -302,7 +308,7 @@ func Decode(entries []Entry, v any, opts Options) ([]Warning, error) { | |||||
| 302 | return p.list, p.err() | 308 | return p.list, p.err() | ||
| 303 | } | 309 | } | ||
| 304 | } | 310 | } | ||
| 305 | - | st.flush() | 311 | + | st.flush(s) |
| 306 | if val, ok := v.(Validator); ok { | 312 | if val, ok := v.(Validator); ok { | ||
| 307 | if err := val.Validate(); err != nil { | 313 | if err := val.Validate(); err != nil { | ||
| 308 | return p.list, err | 314 | return p.list, err | ||
| @@ -314,66 +320,128 @@ func Decode(entries []Entry, v any, opts Options) ([]Warning, error) { | |||||
| 314 | // decodeState holds the scratch values of one Decode call, so a key of a | 320 | // decodeState holds the scratch values of one Decode call, so a key of a | ||
| 315 | // map does not allocate. | 321 | // map does not allocate. | ||
| 316 | type decodeState struct { | 322 | type decodeState struct { | ||
| 317 | - | root reflect.Value | 323 | + | root reflect.Value |
| 318 | - | // scratch key and element per map field, reused for every entry. | 324 | + | entries []Entry |
| 319 | - | keys, vals map[*field]reflect.Value | 325 | + | next int // index in entries of the entry being applied |
| 320 | - | // pending struct elements, written back to their maps once at the end | 326 | + | maps []mapState // by field.slot; nil until a key reaches a map |
| 321 | - | // so each name is copied out of its map once, not once per key. | 327 | + | } |
| 322 | - | pending map[*field]map[string]*pendingElem | 328 | + | |
| 329 | + | // mapState decodes the keys of one kv:"x.*" field. | |||
| 330 | + | type mapState struct { | |||
| 331 | + | m reflect.Value // the map field; a map of values is created on first use | |||
| 332 | + | plain map[string]string // m, for the map[string]string fast path | |||
| 333 | + | hint int // upper bound of the keys reaching it, to size its storage once | |||
| 334 | + | key, val reflect.Value // scratch, reused for every entry | |||
| 335 | + | // Struct elements are decoded in elems, then written back once by | |||
| 336 | + | // flush, so each name is copied out of the map once, not once per key. | |||
| 337 | + | elems reflect.Value // []element | |||
| 338 | + | names []pendingName // names[i] is elems[i] | |||
| 339 | + | index map[string]int | |||
| 323 | } | 340 | } | ||
| 324 | 341 | ||||
| 325 | - | type pendingElem struct { | 342 | + | type pendingName struct { |
| 326 | - | val reflect.Value | 343 | + | name string |
| 327 | - | set bool // at least one key was accepted | 344 | + | set bool // at least one key was accepted |
| 328 | } | 345 | } | ||
| 329 | 346 | ||||
| 330 | - | func (st *decodeState) scratch(f *field) (key, val reflect.Value) { | 347 | + | // linearNames is how many names are searched linearly before index is built. |
| 331 | - | if st.keys == nil { | 348 | + | const linearNames = 16 |
| 332 | - | st.keys, st.vals = map[*field]reflect.Value{}, map[*field]reflect.Value{} | 349 | + | |
| 350 | + | // mapFor returns the state of the map field f. The first call sets every | |||
| 351 | + | // map's hint from the entries left, so a config without map keys never | |||
| 352 | + | // counts. A key is counted for every prefix it starts with, so a key that | |||
| 353 | + | // apply passes on to a shorter prefix is counted there. | |||
| 354 | + | func (st *decodeState) mapFor(s *schema, f *field) *mapState { | |||
| 355 | + | if st.maps == nil { | |||
| 356 | + | st.maps = make([]mapState, len(s.prefixes)) | |||
| 357 | + | for i := st.next; i < len(st.entries); i++ { | |||
| 358 | + | key := st.entries[i].Key | |||
| 359 | + | for _, p := range s.prefixes { | |||
| 360 | + | if len(key) > len(p.key) && key[:len(p.key)] == p.key { | |||
| 361 | + | st.maps[p.slot].hint++ | |||
| 362 | + | } | |||
| 363 | + | } | |||
| 364 | + | } | |||
| 333 | } | 365 | } | ||
| 334 | - | key, ok := st.keys[f] | 366 | + | ms := &st.maps[f.slot] |
| 335 | - | if !ok { | 367 | + | if !ms.m.IsValid() { |
| 336 | - | key = reflect.New(f.typ.Key()).Elem() | 368 | + | ms.m = st.root.FieldByIndex(f.index) |
| 337 | - | val = reflect.New(f.typ.Elem()).Elem() | 369 | + | if f.elem == nil { |
| 338 | - | st.keys[f], st.vals[f] = key, val | 370 | + | if ms.m.IsNil() { |
| 371 | + | ms.m.Set(reflect.MakeMapWithSize(f.typ, ms.hint)) | |||
| 372 | + | } | |||
| 373 | + | if f.typ == stringMapType && f.val.plain() { | |||
| 374 | + | ms.plain = ms.m.Interface().(map[string]string) | |||
| 375 | + | } else { | |||
| 376 | + | ms.val = reflect.New(f.typ.Elem()).Elem() | |||
| 377 | + | } | |||
| 378 | + | } | |||
| 379 | + | if ms.plain == nil { | |||
| 380 | + | ms.key = reflect.New(f.typ.Key()).Elem() | |||
| 381 | + | } | |||
| 339 | } | 382 | } | ||
| 340 | - | return key, st.vals[f] | 383 | + | return ms |
| 341 | } | 384 | } | ||
| 342 | 385 | ||||
| 343 | - | // element returns the addressable pending element name of the map field f. | 386 | + | // element returns the index in ms.elems of the pending element name. |
| 344 | - | func (st *decodeState) element(f *field, m reflect.Value, name string) *pendingElem { | 387 | + | func (ms *mapState) element(f *field, name string) int { |
| 345 | - | if st.pending == nil { | 388 | + | if ms.index != nil { |
| 346 | - | st.pending = map[*field]map[string]*pendingElem{} | 389 | + | if i, ok := ms.index[name]; ok { |
| 347 | - | } | 390 | + | return i |
| 348 | - | byName := st.pending[f] | 391 | + | } |
| 349 | - | if byName == nil { | 392 | + | } else { |
| 350 | - | byName = map[string]*pendingElem{} | 393 | + | for i := range ms.names { |
| 351 | - | st.pending[f] = byName | 394 | + | if ms.names[i].name == name { |
| 395 | + | return i | |||
| 396 | + | } | |||
| 397 | + | } | |||
| 352 | } | 398 | } | ||
| 353 | - | if p, ok := byName[name]; ok { | 399 | + | i := len(ms.names) |
| 354 | - | return p | 400 | + | if !ms.elems.IsValid() { |
| 401 | + | // Every name comes from a key counted in hint: elems never grows. | |||
| 402 | + | ms.elems = reflect.MakeSlice(f.elems, ms.hint, ms.hint) | |||
| 403 | + | ms.names = make([]pendingName, 0, ms.hint) | |||
| 404 | + | } | |||
| 405 | + | ms.names = append(ms.names, pendingName{name: name}) | |||
| 406 | + | if ms.index != nil { | |||
| 407 | + | ms.index[name] = i | |||
| 408 | + | } else if len(ms.names) > linearNames { | |||
| 409 | + | ms.index = make(map[string]int, ms.hint) | |||
| 410 | + | for j, p := range ms.names { | |||
| 411 | + | ms.index[p.name] = j | |||
| 412 | + | } | |||
| 355 | } | 413 | } | ||
| 356 | - | p := &pendingElem{val: reflect.New(f.typ.Elem()).Elem()} | 414 | + | ev := ms.elems.Index(i) |
| 357 | - | key, _ := st.scratch(f) | 415 | + | ms.key.SetString(name) |
| 358 | - | key.SetString(name) | 416 | + | if cur := ms.m.MapIndex(ms.key); cur.IsValid() { |
| 359 | - | if cur := m.MapIndex(key); cur.IsValid() { | 417 | + | ev.Set(cur) |
| 360 | - | p.val.Set(cur) | |||
| 361 | } else if f.defaults { | 418 | } else if f.defaults { | ||
| 362 | - | p.val.Addr().Interface().(Defaulter).SetDefaults() | 419 | + | ev.Addr().Interface().(Defaulter).SetDefaults() |
| 363 | } | 420 | } | ||
| 364 | - | byName[name] = p | 421 | + | return i |
| 365 | - | return p | |||
| 366 | } | 422 | } | ||
| 367 | 423 | ||||
| 368 | - | // flush writes the pending struct elements back to their maps. | 424 | + | // flush writes the pending struct elements back to their maps. A map that |
| 369 | - | func (st *decodeState) flush() { | 425 | + | // a key reached exists afterwards, even when every key was rejected. |
| 370 | - | for f, byName := range st.pending { | 426 | + | func (st *decodeState) flush(s *schema) { |
| 371 | - | m := st.root.FieldByIndex(f.index) | 427 | + | for slot := range st.maps { |
| 372 | - | key, _ := st.scratch(f) | 428 | + | ms, f := &st.maps[slot], s.prefixes[slot] |
| 373 | - | for name, p := range byName { | 429 | + | if f.elem == nil || !ms.m.IsValid() { |
| 430 | + | continue | |||
| 431 | + | } | |||
| 432 | + | if ms.m.IsNil() { | |||
| 433 | + | n := 0 | |||
| 434 | + | for _, p := range ms.names { | |||
| 435 | + | if p.set { | |||
| 436 | + | n++ | |||
| 437 | + | } | |||
| 438 | + | } | |||
| 439 | + | ms.m.Set(reflect.MakeMapWithSize(f.typ, n)) | |||
| 440 | + | } | |||
| 441 | + | for i, p := range ms.names { | |||
| 374 | if p.set { | 442 | if p.set { | ||
| 375 | - | key.SetString(name) | 443 | + | ms.key.SetString(p.name) |
| 376 | - | m.SetMapIndex(key, p.val) | 444 | + | ms.m.SetMapIndex(ms.key, ms.elems.Index(i)) |
| 377 | } | 445 | } | ||
| 378 | } | 446 | } | ||
| 379 | } | 447 | } | ||
| @@ -404,26 +472,22 @@ func (s *schema) apply(st *decodeState, e *Entry) (msg string, known bool) { | |||||
| 404 | continue | 472 | continue | ||
| 405 | } | 473 | } | ||
| 406 | } | 474 | } | ||
| 407 | - | m := root.FieldByIndex(f.index) | 475 | + | ms := st.mapFor(s, f) |
| 408 | - | if m.IsNil() { | |||
| 409 | - | m.Set(reflect.MakeMap(f.typ)) | |||
| 410 | - | } | |||
| 411 | if sub != nil { | 476 | if sub != nil { | ||
| 412 | // A name whose every key is rejected is not created. | 477 | // A name whose every key is rejected is not created. | ||
| 413 | - | p := st.element(f, m, name) | 478 | + | i := ms.element(f, name) |
| 414 | - | if msg = sub.set(p.val.FieldByIndex(sub.index), e.Value); msg == "" { | 479 | + | if msg = sub.set(ms.elems.Index(i).FieldByIndex(sub.index), e.Value); msg == "" { |
| 415 | - | p.set = true | 480 | + | ms.names[i].set = true |
| 416 | } | 481 | } | ||
| 417 | return msg, true | 482 | return msg, true | ||
| 418 | } | 483 | } | ||
| 419 | - | if f.typ == stringMapType && f.val.plain() { | 484 | + | if ms.plain != nil { |
| 420 | - | m.Interface().(map[string]string)[name] = e.Value | 485 | + | ms.plain[name] = e.Value |
| 421 | return "", true | 486 | return "", true | ||
| 422 | } | 487 | } | ||
| 423 | - | key, val := st.scratch(f) | 488 | + | if msg = f.val.set(ms.val, e.Value); msg == "" { |
| 424 | - | if msg = f.val.set(val, e.Value); msg == "" { | 489 | + | ms.key.SetString(name) |
| 425 | - | key.SetString(name) | 490 | + | ms.m.SetMapIndex(ms.key, ms.val) |
| 426 | - | m.SetMapIndex(key, val) | |||
| 427 | } | 491 | } | ||
| 428 | return msg, true | 492 | return msg, true | ||
| 429 | } | 493 | } | ||
M decode_test.go +70 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "os" | |
| 7 | 7 | "path/filepath" | |
| 8 | 8 | "reflect" | |
| 9 | + | "strconv" | |
| 9 | 10 | "strings" | |
| 10 | 11 | "testing" | |
| 11 | 12 | "time" | |
| @@ -229,6 +230,75 @@ dir = rel/path | |||
| 229 | 230 | } | |
| 230 | 231 | } | |
| 231 | 232 | ||
| 233 | + | type nestedMaps struct { | |
| 234 | + | All map[string]string `kv:"m.*"` | |
| 235 | + | Outputs map[string]output `kv:"m.out.*"` | |
| 236 | + | } | |
| 237 | + | ||
| 238 | + | func TestDecodeMaps(t *testing.T) { | |
| 239 | + | var many strings.Builder | |
| 240 | + | want := map[string]output{} | |
| 241 | + | for i := range 40 { // past the linear search, twice per name | |
| 242 | + | name := "o" + strconv.Itoa(i) | |
| 243 | + | fmt.Fprintf(&many, "output.%s.scale = 2\noutput.%s.hdr = on\n", name, name) | |
| 244 | + | want[name] = output{Scale: 2, HDR: true, Bright: 203} | |
| 245 | + | } | |
| 246 | + | tests := []struct { | |
| 247 | + | name string | |
| 248 | + | in string | |
| 249 | + | start func(c *testConfig) | |
| 250 | + | check func(t *testing.T, c *testConfig) | |
| 251 | + | }{ | |
| 252 | + | { | |
| 253 | + | name: "many names", | |
| 254 | + | in: many.String(), | |
| 255 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, want) }, | |
| 256 | + | }, | |
| 257 | + | { | |
| 258 | + | name: "existing elements are updated, not reset", | |
| 259 | + | in: "output.A.hdr = on\noutput.B.scale = 3\nbind.x = y\n", | |
| 260 | + | start: func(c *testConfig) { | |
| 261 | + | c.Outputs = map[string]output{"A": {Scale: 2, Bright: 100}, "C": {Scale: 1}} | |
| 262 | + | c.Binds = map[string]string{"keep": "me"} | |
| 263 | + | }, | |
| 264 | + | check: func(t *testing.T, c *testConfig) { | |
| 265 | + | eq(t, c.Outputs, map[string]output{ | |
| 266 | + | "A": {Scale: 2, HDR: true, Bright: 100}, | |
| 267 | + | "B": {Scale: 3, Bright: 203}, | |
| 268 | + | "C": {Scale: 1}, | |
| 269 | + | }) | |
| 270 | + | eq(t, c.Binds, map[string]string{"keep": "me", "x": "y"}) | |
| 271 | + | }, | |
| 272 | + | }, | |
| 273 | + | { | |
| 274 | + | name: "rejected element leaves an existing map untouched", | |
| 275 | + | in: "output.A.scale = 9\n", | |
| 276 | + | start: func(c *testConfig) { c.Outputs = map[string]output{"A": {Scale: 2}} }, | |
| 277 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{"A": {Scale: 2}}) }, | |
| 278 | + | }, | |
| 279 | + | } | |
| 280 | + | for _, tt := range tests { | |
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |
| 282 | + | c := defaults() | |
| 283 | + | if tt.start != nil { | |
| 284 | + | tt.start(&c) | |
| 285 | + | } | |
| 286 | + | _, err := Unmarshal([]byte(tt.in), &c, Options{}) | |
| 287 | + | must(t, err) | |
| 288 | + | tt.check(t, &c) | |
| 289 | + | }) | |
| 290 | + | } | |
| 291 | + | ||
| 292 | + | t.Run("key matching two prefixes", func(t *testing.T) { | |
| 293 | + | // m.out.x.nope is no output field, so m.* takes it. | |
| 294 | + | var n nestedMaps | |
| 295 | + | _, err := Unmarshal([]byte("m.out.x.scale = 2\nm.out.x.nope = 1\nm.a = b\n"), &n, Options{}) | |
| 296 | + | must(t, err) | |
| 297 | + | eq(t, n.Outputs, map[string]output{"x": {Scale: 2, Bright: 203}}) | |
| 298 | + | eq(t, n.All, map[string]string{"out.x.nope": "1", "a": "b"}) | |
| 299 | + | }) | |
| 300 | + | } | |
| 301 | + | ||
| 232 | 302 | type plain struct { | |
| 233 | 303 | A int `kv:"a"` | |
| 234 | 304 | } | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "os" | 6 | "os" | ||
| 7 | "path/filepath" | 7 | "path/filepath" | ||
| 8 | "reflect" | 8 | "reflect" | ||
| 9 | + | "strconv" | |||
| 9 | "strings" | 10 | "strings" | ||
| 10 | "testing" | 11 | "testing" | ||
| 11 | "time" | 12 | "time" | ||
| @@ -229,6 +230,75 @@ dir = rel/path | |||||
| 229 | } | 230 | } | ||
| 230 | } | 231 | } | ||
| 231 | 232 | ||||
| 233 | + | type nestedMaps struct { | |||
| 234 | + | All map[string]string `kv:"m.*"` | |||
| 235 | + | Outputs map[string]output `kv:"m.out.*"` | |||
| 236 | + | } | |||
| 237 | + | ||||
| 238 | + | func TestDecodeMaps(t *testing.T) { | |||
| 239 | + | var many strings.Builder | |||
| 240 | + | want := map[string]output{} | |||
| 241 | + | for i := range 40 { // past the linear search, twice per name | |||
| 242 | + | name := "o" + strconv.Itoa(i) | |||
| 243 | + | fmt.Fprintf(&many, "output.%s.scale = 2\noutput.%s.hdr = on\n", name, name) | |||
| 244 | + | want[name] = output{Scale: 2, HDR: true, Bright: 203} | |||
| 245 | + | } | |||
| 246 | + | tests := []struct { | |||
| 247 | + | name string | |||
| 248 | + | in string | |||
| 249 | + | start func(c *testConfig) | |||
| 250 | + | check func(t *testing.T, c *testConfig) | |||
| 251 | + | }{ | |||
| 252 | + | { | |||
| 253 | + | name: "many names", | |||
| 254 | + | in: many.String(), | |||
| 255 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, want) }, | |||
| 256 | + | }, | |||
| 257 | + | { | |||
| 258 | + | name: "existing elements are updated, not reset", | |||
| 259 | + | in: "output.A.hdr = on\noutput.B.scale = 3\nbind.x = y\n", | |||
| 260 | + | start: func(c *testConfig) { | |||
| 261 | + | c.Outputs = map[string]output{"A": {Scale: 2, Bright: 100}, "C": {Scale: 1}} | |||
| 262 | + | c.Binds = map[string]string{"keep": "me"} | |||
| 263 | + | }, | |||
| 264 | + | check: func(t *testing.T, c *testConfig) { | |||
| 265 | + | eq(t, c.Outputs, map[string]output{ | |||
| 266 | + | "A": {Scale: 2, HDR: true, Bright: 100}, | |||
| 267 | + | "B": {Scale: 3, Bright: 203}, | |||
| 268 | + | "C": {Scale: 1}, | |||
| 269 | + | }) | |||
| 270 | + | eq(t, c.Binds, map[string]string{"keep": "me", "x": "y"}) | |||
| 271 | + | }, | |||
| 272 | + | }, | |||
| 273 | + | { | |||
| 274 | + | name: "rejected element leaves an existing map untouched", | |||
| 275 | + | in: "output.A.scale = 9\n", | |||
| 276 | + | start: func(c *testConfig) { c.Outputs = map[string]output{"A": {Scale: 2}} }, | |||
| 277 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{"A": {Scale: 2}}) }, | |||
| 278 | + | }, | |||
| 279 | + | } | |||
| 280 | + | for _, tt := range tests { | |||
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 282 | + | c := defaults() | |||
| 283 | + | if tt.start != nil { | |||
| 284 | + | tt.start(&c) | |||
| 285 | + | } | |||
| 286 | + | _, err := Unmarshal([]byte(tt.in), &c, Options{}) | |||
| 287 | + | must(t, err) | |||
| 288 | + | tt.check(t, &c) | |||
| 289 | + | }) | |||
| 290 | + | } | |||
| 291 | + | ||||
| 292 | + | t.Run("key matching two prefixes", func(t *testing.T) { | |||
| 293 | + | // m.out.x.nope is no output field, so m.* takes it. | |||
| 294 | + | var n nestedMaps | |||
| 295 | + | _, err := Unmarshal([]byte("m.out.x.scale = 2\nm.out.x.nope = 1\nm.a = b\n"), &n, Options{}) | |||
| 296 | + | must(t, err) | |||
| 297 | + | eq(t, n.Outputs, map[string]output{"x": {Scale: 2, Bright: 203}}) | |||
| 298 | + | eq(t, n.All, map[string]string{"out.x.nope": "1", "a": "b"}) | |||
| 299 | + | }) | |||
| 300 | + | } | |||
| 301 | + | ||||
| 232 | type plain struct { | 302 | type plain struct { | ||
| 233 | A int `kv:"a"` | 303 | A int `kv:"a"` | ||
| 234 | } | 304 | } | ||
M marshal.go +56 -14
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | 10 | "strconv" | |
| 11 | 11 | "strings" | |
| 12 | 12 | "time" | |
| 13 | + | "unicode/utf8" | |
| 13 | 14 | "unsafe" | |
| 14 | 15 | ) | |
| 15 | 16 | ||
| @@ -23,21 +24,44 @@ func Quote(v string) string { | |||
| 23 | 24 | } | |
| 24 | 25 | ||
| 25 | 26 | func needsQuote(v string) bool { | |
| 26 | - | if v == "" || v != strings.TrimSpace(v) { | |
| 27 | + | if v == "" { | |
| 27 | 28 | return true | |
| 28 | 29 | } | |
| 29 | - | if v[0] == '"' || v[0] == '\'' { | |
| 30 | + | first, last := v[0], v[len(v)-1] | |
| 31 | + | if first >= utf8.RuneSelf || last >= utf8.RuneSelf { | |
| 32 | + | if v != strings.TrimSpace(v) { | |
| 33 | + | return true | |
| 34 | + | } | |
| 35 | + | } else if asciiSpace(first) || asciiSpace(last) { | |
| 36 | + | return true | |
| 37 | + | } | |
| 38 | + | if first == '"' || first == '\'' { | |
| 30 | 39 | return true // it would be unquoted on reading | |
| 31 | 40 | } | |
| 41 | + | hash := false | |
| 42 | + | for i := 0; i < len(v); i++ { | |
| 43 | + | switch v[i] { | |
| 44 | + | case '\n', '\r': | |
| 45 | + | return true | |
| 46 | + | case '#': | |
| 47 | + | hash = true | |
| 48 | + | } | |
| 49 | + | } | |
| 32 | 50 | // A '#' after a space ends the value unless it is inside a quoted word, | |
| 33 | 51 | // so quote any value where Parse would cut it. | |
| 34 | - | return strings.ContainsAny(v, "\n\r") || stripComment(v, CommentSpaced) != v | |
| 52 | + | return hash && stripComment(v, CommentSpaced) != v | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | // asciiSpace reports the ASCII bytes that strings.TrimSpace removes. | |
| 56 | + | func asciiSpace(c byte) bool { | |
| 57 | + | return c == ' ' || c == '\t' || c == '\n' || c == '\v' || c == '\f' || c == '\r' | |
| 35 | 58 | } | |
| 36 | 59 | ||
| 37 | 60 | // validKey reports whether key can be written and read back as one key. | |
| 61 | + | // A leading U+FEFF would be read as the byte order mark of a first line. | |
| 38 | 62 | func validKey(key string) bool { | |
| 39 | 63 | return key != "" && key[0] != '#' && key == strings.TrimSpace(key) && | |
| 40 | - | !strings.ContainsAny(key, "=\n\r") | |
| 64 | + | !strings.HasPrefix(key, "\ufeff") && !strings.ContainsAny(key, "=\n\r") | |
| 41 | 65 | } | |
| 42 | 66 | ||
| 43 | 67 | func appendQuoted(b []byte, v string) []byte { | |
| @@ -58,7 +82,27 @@ func Marshal(v any) ([]byte, error) { | |||
| 58 | 82 | if err != nil { | |
| 59 | 83 | return nil, err | |
| 60 | 84 | } | |
| 61 | - | m := marshaler{b: make([]byte, 0, 32*len(s.order))} | |
| 85 | + | // Size the output once: about 24 bytes of value per line. | |
| 86 | + | size, maxLen := 0, 0 | |
| 87 | + | for _, f := range s.order { | |
| 88 | + | if f.elem == nil && f.val == nil { | |
| 89 | + | size += len(f.key) + 24 | |
| 90 | + | continue | |
| 91 | + | } | |
| 92 | + | n := rv.FieldByIndex(f.index).Len() | |
| 93 | + | maxLen = max(maxLen, n) | |
| 94 | + | if f.elem == nil { | |
| 95 | + | size += n * (2*len(f.key) + 24) | |
| 96 | + | continue | |
| 97 | + | } | |
| 98 | + | for _, sub := range f.elem.order { | |
| 99 | + | size += n * (len(f.key) + len(sub.key) + 32) | |
| 100 | + | } | |
| 101 | + | } | |
| 102 | + | m := marshaler{b: make([]byte, 0, size), tmp: make([]byte, 0, 64)} | |
| 103 | + | // Map names and their sort order, reused for every map. | |
| 104 | + | names := make([]string, 0, maxLen) | |
| 105 | + | order := make([]int, 0, maxLen) | |
| 62 | 106 | for _, f := range s.order { | |
| 63 | 107 | fv := rv.FieldByIndex(f.index) | |
| 64 | 108 | if f.elem == nil && f.val == nil { | |
| @@ -69,18 +113,19 @@ func Marshal(v any) ([]byte, error) { | |||
| 69 | 113 | } | |
| 70 | 114 | // Copy the map out once, into reused values, then sort by name. | |
| 71 | 115 | n := fv.Len() | |
| 72 | - | names := make([]string, 0, n) | |
| 116 | + | names, order = names[:0], order[:0] | |
| 73 | 117 | vals := reflect.MakeSlice(reflect.SliceOf(f.typ.Elem()), n, n) | |
| 74 | 118 | key := reflect.New(f.typ.Key()).Elem() | |
| 75 | 119 | for it, i := fv.MapRange(), 0; it.Next(); i++ { | |
| 76 | 120 | key.SetIterKey(it) | |
| 77 | - | names = append(names, key.String()) | |
| 121 | + | name := key.String() | |
| 122 | + | if strings.ContainsAny(name, "=\n\r") || name != strings.TrimSpace(name) { | |
| 123 | + | return nil, fmt.Errorf("kvconf: %s: invalid map name %q", f.key, name) | |
| 124 | + | } | |
| 125 | + | names = append(names, name) | |
| 126 | + | order = append(order, i) | |
| 78 | 127 | vals.Index(i).SetIterValue(it) | |
| 79 | 128 | } | |
| 80 | - | order := make([]int, n) | |
| 81 | - | for i := range order { | |
| 82 | - | order[i] = i | |
| 83 | - | } | |
| 84 | 129 | slices.SortFunc(order, func(a, b int) int { return strings.Compare(names[a], names[b]) }) | |
| 85 | 130 | for _, i := range order { | |
| 86 | 131 | name, ev := names[i], vals.Index(i) | |
| @@ -136,9 +181,6 @@ func (m *marshaler) raw(key, value string) error { | |||
| 136 | 181 | // line writes prefix+name[.sub] = value. | |
| 137 | 182 | func (m *marshaler) line(prefix, name, sub string, v reflect.Value, f *field) error { | |
| 138 | 183 | var err error | |
| 139 | - | if name != "" && (strings.ContainsAny(name, "=\n\r") || name != strings.TrimSpace(name)) { | |
| 140 | - | return fmt.Errorf("kvconf: %s: invalid map name %q", prefix, name) | |
| 141 | - | } | |
| 142 | 184 | if m.tmp, err = appendValue(m.tmp[:0], v, f); err != nil { | |
| 143 | 185 | key := prefix + name | |
| 144 | 186 | if sub != "" { | |
| @@ -10,6 +10,7 @@ import ( | |||||
| 10 | "strconv" | 10 | "strconv" | ||
| 11 | "strings" | 11 | "strings" | ||
| 12 | "time" | 12 | "time" | ||
| 13 | + | "unicode/utf8" | |||
| 13 | "unsafe" | 14 | "unsafe" | ||
| 14 | ) | 15 | ) | ||
| 15 | 16 | ||||
| @@ -23,21 +24,44 @@ func Quote(v string) string { | |||||
| 23 | } | 24 | } | ||
| 24 | 25 | ||||
| 25 | func needsQuote(v string) bool { | 26 | func needsQuote(v string) bool { | ||
| 26 | - | if v == "" || v != strings.TrimSpace(v) { | 27 | + | if v == "" { |
| 27 | return true | 28 | return true | ||
| 28 | } | 29 | } | ||
| 29 | - | if v[0] == '"' || v[0] == '\'' { | 30 | + | first, last := v[0], v[len(v)-1] |
| 31 | + | if first >= utf8.RuneSelf || last >= utf8.RuneSelf { | |||
| 32 | + | if v != strings.TrimSpace(v) { | |||
| 33 | + | return true | |||
| 34 | + | } | |||
| 35 | + | } else if asciiSpace(first) || asciiSpace(last) { | |||
| 36 | + | return true | |||
| 37 | + | } | |||
| 38 | + | if first == '"' || first == '\'' { | |||
| 30 | return true // it would be unquoted on reading | 39 | return true // it would be unquoted on reading | ||
| 31 | } | 40 | } | ||
| 41 | + | hash := false | |||
| 42 | + | for i := 0; i < len(v); i++ { | |||
| 43 | + | switch v[i] { | |||
| 44 | + | case '\n', '\r': | |||
| 45 | + | return true | |||
| 46 | + | case '#': | |||
| 47 | + | hash = true | |||
| 48 | + | } | |||
| 49 | + | } | |||
| 32 | // A '#' after a space ends the value unless it is inside a quoted word, | 50 | // A '#' after a space ends the value unless it is inside a quoted word, | ||
| 33 | // so quote any value where Parse would cut it. | 51 | // so quote any value where Parse would cut it. | ||
| 34 | - | return strings.ContainsAny(v, "\n\r") || stripComment(v, CommentSpaced) != v | 52 | + | return hash && stripComment(v, CommentSpaced) != v |
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | // asciiSpace reports the ASCII bytes that strings.TrimSpace removes. | |||
| 56 | + | func asciiSpace(c byte) bool { | |||
| 57 | + | return c == ' ' || c == '\t' || c == '\n' || c == '\v' || c == '\f' || c == '\r' | |||
| 35 | } | 58 | } | ||
| 36 | 59 | ||||
| 37 | // validKey reports whether key can be written and read back as one key. | 60 | // validKey reports whether key can be written and read back as one key. | ||
| 61 | + | // A leading U+FEFF would be read as the byte order mark of a first line. | |||
| 38 | func validKey(key string) bool { | 62 | func validKey(key string) bool { | ||
| 39 | return key != "" && key[0] != '#' && key == strings.TrimSpace(key) && | 63 | return key != "" && key[0] != '#' && key == strings.TrimSpace(key) && | ||
| 40 | - | !strings.ContainsAny(key, "=\n\r") | 64 | + | !strings.HasPrefix(key, "\ufeff") && !strings.ContainsAny(key, "=\n\r") |
| 41 | } | 65 | } | ||
| 42 | 66 | ||||
| 43 | func appendQuoted(b []byte, v string) []byte { | 67 | func appendQuoted(b []byte, v string) []byte { | ||
| @@ -58,7 +82,27 @@ func Marshal(v any) ([]byte, error) { | |||||
| 58 | if err != nil { | 82 | if err != nil { | ||
| 59 | return nil, err | 83 | return nil, err | ||
| 60 | } | 84 | } | ||
| 61 | - | m := marshaler{b: make([]byte, 0, 32*len(s.order))} | 85 | + | // Size the output once: about 24 bytes of value per line. |
| 86 | + | size, maxLen := 0, 0 | |||
| 87 | + | for _, f := range s.order { | |||
| 88 | + | if f.elem == nil && f.val == nil { | |||
| 89 | + | size += len(f.key) + 24 | |||
| 90 | + | continue | |||
| 91 | + | } | |||
| 92 | + | n := rv.FieldByIndex(f.index).Len() | |||
| 93 | + | maxLen = max(maxLen, n) | |||
| 94 | + | if f.elem == nil { | |||
| 95 | + | size += n * (2*len(f.key) + 24) | |||
| 96 | + | continue | |||
| 97 | + | } | |||
| 98 | + | for _, sub := range f.elem.order { | |||
| 99 | + | size += n * (len(f.key) + len(sub.key) + 32) | |||
| 100 | + | } | |||
| 101 | + | } | |||
| 102 | + | m := marshaler{b: make([]byte, 0, size), tmp: make([]byte, 0, 64)} | |||
| 103 | + | // Map names and their sort order, reused for every map. | |||
| 104 | + | names := make([]string, 0, maxLen) | |||
| 105 | + | order := make([]int, 0, maxLen) | |||
| 62 | for _, f := range s.order { | 106 | for _, f := range s.order { | ||
| 63 | fv := rv.FieldByIndex(f.index) | 107 | fv := rv.FieldByIndex(f.index) | ||
| 64 | if f.elem == nil && f.val == nil { | 108 | if f.elem == nil && f.val == nil { | ||
| @@ -69,18 +113,19 @@ func Marshal(v any) ([]byte, error) { | |||||
| 69 | } | 113 | } | ||
| 70 | // Copy the map out once, into reused values, then sort by name. | 114 | // Copy the map out once, into reused values, then sort by name. | ||
| 71 | n := fv.Len() | 115 | n := fv.Len() | ||
| 72 | - | names := make([]string, 0, n) | 116 | + | names, order = names[:0], order[:0] |
| 73 | vals := reflect.MakeSlice(reflect.SliceOf(f.typ.Elem()), n, n) | 117 | vals := reflect.MakeSlice(reflect.SliceOf(f.typ.Elem()), n, n) | ||
| 74 | key := reflect.New(f.typ.Key()).Elem() | 118 | key := reflect.New(f.typ.Key()).Elem() | ||
| 75 | for it, i := fv.MapRange(), 0; it.Next(); i++ { | 119 | for it, i := fv.MapRange(), 0; it.Next(); i++ { | ||
| 76 | key.SetIterKey(it) | 120 | key.SetIterKey(it) | ||
| 77 | - | names = append(names, key.String()) | 121 | + | name := key.String() |
| 122 | + | if strings.ContainsAny(name, "=\n\r") || name != strings.TrimSpace(name) { | |||
| 123 | + | return nil, fmt.Errorf("kvconf: %s: invalid map name %q", f.key, name) | |||
| 124 | + | } | |||
| 125 | + | names = append(names, name) | |||
| 126 | + | order = append(order, i) | |||
| 78 | vals.Index(i).SetIterValue(it) | 127 | vals.Index(i).SetIterValue(it) | ||
| 79 | } | 128 | } | ||
| 80 | - | order := make([]int, n) | |||
| 81 | - | for i := range order { | |||
| 82 | - | order[i] = i | |||
| 83 | - | } | |||
| 84 | slices.SortFunc(order, func(a, b int) int { return strings.Compare(names[a], names[b]) }) | 129 | slices.SortFunc(order, func(a, b int) int { return strings.Compare(names[a], names[b]) }) | ||
| 85 | for _, i := range order { | 130 | for _, i := range order { | ||
| 86 | name, ev := names[i], vals.Index(i) | 131 | name, ev := names[i], vals.Index(i) | ||
| @@ -136,9 +181,6 @@ func (m *marshaler) raw(key, value string) error { | |||||
| 136 | // line writes prefix+name[.sub] = value. | 181 | // line writes prefix+name[.sub] = value. | ||
| 137 | func (m *marshaler) line(prefix, name, sub string, v reflect.Value, f *field) error { | 182 | func (m *marshaler) line(prefix, name, sub string, v reflect.Value, f *field) error { | ||
| 138 | var err error | 183 | var err error | ||
| 139 | - | if name != "" && (strings.ContainsAny(name, "=\n\r") || name != strings.TrimSpace(name)) { | |||
| 140 | - | return fmt.Errorf("kvconf: %s: invalid map name %q", prefix, name) | |||
| 141 | - | } | |||
| 142 | if m.tmp, err = appendValue(m.tmp[:0], v, f); err != nil { | 184 | if m.tmp, err = appendValue(m.tmp[:0], v, f); err != nil { | ||
| 143 | key := prefix + name | 185 | key := prefix + name | ||
| 144 | if sub != "" { | 186 | if sub != "" { | ||
M parse.go +58 -11
| @@ -15,6 +15,7 @@ package kvconf | |||
| 15 | 15 | ||
| 16 | 16 | import ( | |
| 17 | 17 | "errors" | |
| 18 | + | "hash/maphash" | |
| 18 | 19 | "io" | |
| 19 | 20 | "slices" | |
| 20 | 21 | "strconv" | |
| @@ -132,6 +133,57 @@ func (p *problems) add(line int, key, msg string) bool { | |||
| 132 | 133 | return !p.strict | |
| 133 | 134 | } | |
| 134 | 135 | ||
| 136 | + | var dupSeed = maphash.MakeSeed() | |
| 137 | + | ||
| 138 | + | // duplicates reports each assignment of a key already assigned on an | |
| 139 | + | // earlier line, with the line it overrides. An open-addressing table of | |
| 140 | + | // entry indexes, on the stack for a config-sized file, is cheaper than a | |
| 141 | + | // map[string]int. | |
| 142 | + | func (p *problems) duplicates(entries []Entry) { | |
| 143 | + | if len(entries) < 2 { | |
| 144 | + | return | |
| 145 | + | } | |
| 146 | + | size := 1 | |
| 147 | + | for size < 2*len(entries) { | |
| 148 | + | size <<= 1 | |
| 149 | + | } | |
| 150 | + | var buf [512]int32 // 0: empty, else entry index + 1 | |
| 151 | + | var table []int32 | |
| 152 | + | if size <= len(buf) { | |
| 153 | + | table = buf[:size] | |
| 154 | + | } else { | |
| 155 | + | table = make([]int32, size) | |
| 156 | + | } | |
| 157 | + | mask := uint32(size - 1) | |
| 158 | + | var dups []Warning | |
| 159 | + | for i := range entries { | |
| 160 | + | key := entries[i].Key | |
| 161 | + | // Seeded, so a crafted file cannot make every key collide. | |
| 162 | + | h := uint32(maphash.String(dupSeed, key)) | |
| 163 | + | for slot := h & mask; ; slot = (slot + 1) & mask { | |
| 164 | + | k := table[slot] | |
| 165 | + | if k == 0 { | |
| 166 | + | table[slot] = int32(i + 1) | |
| 167 | + | break | |
| 168 | + | } | |
| 169 | + | if prev := &entries[k-1]; prev.Key == key { | |
| 170 | + | dups = append(dups, Warning{Line: entries[i].Line, Key: key, Msg: "overrides line " + strconv.Itoa(prev.Line)}) | |
| 171 | + | table[slot] = int32(i + 1) | |
| 172 | + | break | |
| 173 | + | } | |
| 174 | + | } | |
| 175 | + | } | |
| 176 | + | if len(dups) == 0 { | |
| 177 | + | return | |
| 178 | + | } | |
| 179 | + | // Warnings are reported in line order, merged with the syntax ones. | |
| 180 | + | p.list = append(p.list, dups...) | |
| 181 | + | slices.SortStableFunc(p.list, func(a, b Warning) int { return a.Line - b.Line }) | |
| 182 | + | if p.strict { | |
| 183 | + | p.list = p.list[:1] | |
| 184 | + | } | |
| 185 | + | } | |
| 186 | + | ||
| 135 | 187 | func (p *problems) err() error { | |
| 136 | 188 | if p.strict && len(p.list) > 0 { | |
| 137 | 189 | return p.list[0] | |
| @@ -176,10 +228,6 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||
| 176 | 228 | // One copy: every key and value is a substring of it. | |
| 177 | 229 | s := strings.TrimPrefix(string(data), "\ufeff") | |
| 178 | 230 | entries := make([]Entry, 0, strings.Count(s, "=")) | |
| 179 | - | var seen map[string]int | |
| 180 | - | if !opts.AllowDuplicates { | |
| 181 | - | seen = make(map[string]int, cap(entries)) | |
| 182 | - | } | |
| 183 | 231 | p := problems{strict: opts.Strict} | |
| 184 | 232 | maxLine := opts.maxLine() | |
| 185 | 233 | for n := 1; s != ""; n++ { | |
| @@ -218,14 +266,12 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||
| 218 | 266 | if !opts.RawValues { | |
| 219 | 267 | value = unquote(value) | |
| 220 | 268 | } | |
| 221 | - | if seen != nil { | |
| 222 | - | if prev, dup := seen[key]; dup && !p.add(n, key, "overrides line "+strconv.Itoa(prev)) { | |
| 223 | - | break | |
| 224 | - | } | |
| 225 | - | seen[key] = n | |
| 226 | - | } | |
| 227 | 269 | entries = append(entries, Entry{Key: key, Value: value, Line: n}) | |
| 228 | 270 | } | |
| 271 | + | if !opts.AllowDuplicates { | |
| 272 | + | // entries stops at a strict failure: its duplicates come first. | |
| 273 | + | p.duplicates(entries) | |
| 274 | + | } | |
| 229 | 275 | if err := p.err(); err != nil { | |
| 230 | 276 | return nil, p.list, err | |
| 231 | 277 | } | |
| @@ -271,7 +317,8 @@ func unquote(v string) string { | |||
| 271 | 317 | if strings.IndexByte(inner, '\\') < 0 && strings.IndexByte(inner, '"') < 0 { | |
| 272 | 318 | return inner // the common case, without allocating | |
| 273 | 319 | } | |
| 274 | - | if s, err := strconv.Unquote(v); err == nil { | |
| 320 | + | // An escape such as \x80 could make invalid UTF-8: keep it as written. | |
| 321 | + | if s, err := strconv.Unquote(v); err == nil && utf8.ValidString(s) { | |
| 275 | 322 | return s | |
| 276 | 323 | } | |
| 277 | 324 | case '\'': | |
| @@ -15,6 +15,7 @@ package kvconf | |||||
| 15 | 15 | ||||
| 16 | import ( | 16 | import ( | ||
| 17 | "errors" | 17 | "errors" | ||
| 18 | + | "hash/maphash" | |||
| 18 | "io" | 19 | "io" | ||
| 19 | "slices" | 20 | "slices" | ||
| 20 | "strconv" | 21 | "strconv" | ||
| @@ -132,6 +133,57 @@ func (p *problems) add(line int, key, msg string) bool { | |||||
| 132 | return !p.strict | 133 | return !p.strict | ||
| 133 | } | 134 | } | ||
| 134 | 135 | ||||
| 136 | + | var dupSeed = maphash.MakeSeed() | |||
| 137 | + | ||||
| 138 | + | // duplicates reports each assignment of a key already assigned on an | |||
| 139 | + | // earlier line, with the line it overrides. An open-addressing table of | |||
| 140 | + | // entry indexes, on the stack for a config-sized file, is cheaper than a | |||
| 141 | + | // map[string]int. | |||
| 142 | + | func (p *problems) duplicates(entries []Entry) { | |||
| 143 | + | if len(entries) < 2 { | |||
| 144 | + | return | |||
| 145 | + | } | |||
| 146 | + | size := 1 | |||
| 147 | + | for size < 2*len(entries) { | |||
| 148 | + | size <<= 1 | |||
| 149 | + | } | |||
| 150 | + | var buf [512]int32 // 0: empty, else entry index + 1 | |||
| 151 | + | var table []int32 | |||
| 152 | + | if size <= len(buf) { | |||
| 153 | + | table = buf[:size] | |||
| 154 | + | } else { | |||
| 155 | + | table = make([]int32, size) | |||
| 156 | + | } | |||
| 157 | + | mask := uint32(size - 1) | |||
| 158 | + | var dups []Warning | |||
| 159 | + | for i := range entries { | |||
| 160 | + | key := entries[i].Key | |||
| 161 | + | // Seeded, so a crafted file cannot make every key collide. | |||
| 162 | + | h := uint32(maphash.String(dupSeed, key)) | |||
| 163 | + | for slot := h & mask; ; slot = (slot + 1) & mask { | |||
| 164 | + | k := table[slot] | |||
| 165 | + | if k == 0 { | |||
| 166 | + | table[slot] = int32(i + 1) | |||
| 167 | + | break | |||
| 168 | + | } | |||
| 169 | + | if prev := &entries[k-1]; prev.Key == key { | |||
| 170 | + | dups = append(dups, Warning{Line: entries[i].Line, Key: key, Msg: "overrides line " + strconv.Itoa(prev.Line)}) | |||
| 171 | + | table[slot] = int32(i + 1) | |||
| 172 | + | break | |||
| 173 | + | } | |||
| 174 | + | } | |||
| 175 | + | } | |||
| 176 | + | if len(dups) == 0 { | |||
| 177 | + | return | |||
| 178 | + | } | |||
| 179 | + | // Warnings are reported in line order, merged with the syntax ones. | |||
| 180 | + | p.list = append(p.list, dups...) | |||
| 181 | + | slices.SortStableFunc(p.list, func(a, b Warning) int { return a.Line - b.Line }) | |||
| 182 | + | if p.strict { | |||
| 183 | + | p.list = p.list[:1] | |||
| 184 | + | } | |||
| 185 | + | } | |||
| 186 | + | ||||
| 135 | func (p *problems) err() error { | 187 | func (p *problems) err() error { | ||
| 136 | if p.strict && len(p.list) > 0 { | 188 | if p.strict && len(p.list) > 0 { | ||
| 137 | return p.list[0] | 189 | return p.list[0] | ||
| @@ -176,10 +228,6 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||||
| 176 | // One copy: every key and value is a substring of it. | 228 | // One copy: every key and value is a substring of it. | ||
| 177 | s := strings.TrimPrefix(string(data), "\ufeff") | 229 | s := strings.TrimPrefix(string(data), "\ufeff") | ||
| 178 | entries := make([]Entry, 0, strings.Count(s, "=")) | 230 | entries := make([]Entry, 0, strings.Count(s, "=")) | ||
| 179 | - | var seen map[string]int | |||
| 180 | - | if !opts.AllowDuplicates { | |||
| 181 | - | seen = make(map[string]int, cap(entries)) | |||
| 182 | - | } | |||
| 183 | p := problems{strict: opts.Strict} | 231 | p := problems{strict: opts.Strict} | ||
| 184 | maxLine := opts.maxLine() | 232 | maxLine := opts.maxLine() | ||
| 185 | for n := 1; s != ""; n++ { | 233 | for n := 1; s != ""; n++ { | ||
| @@ -218,14 +266,12 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||||
| 218 | if !opts.RawValues { | 266 | if !opts.RawValues { | ||
| 219 | value = unquote(value) | 267 | value = unquote(value) | ||
| 220 | } | 268 | } | ||
| 221 | - | if seen != nil { | |||
| 222 | - | if prev, dup := seen[key]; dup && !p.add(n, key, "overrides line "+strconv.Itoa(prev)) { | |||
| 223 | - | break | |||
| 224 | - | } | |||
| 225 | - | seen[key] = n | |||
| 226 | - | } | |||
| 227 | entries = append(entries, Entry{Key: key, Value: value, Line: n}) | 269 | entries = append(entries, Entry{Key: key, Value: value, Line: n}) | ||
| 228 | } | 270 | } | ||
| 271 | + | if !opts.AllowDuplicates { | |||
| 272 | + | // entries stops at a strict failure: its duplicates come first. | |||
| 273 | + | p.duplicates(entries) | |||
| 274 | + | } | |||
| 229 | if err := p.err(); err != nil { | 275 | if err := p.err(); err != nil { | ||
| 230 | return nil, p.list, err | 276 | return nil, p.list, err | ||
| 231 | } | 277 | } | ||
| @@ -271,7 +317,8 @@ func unquote(v string) string { | |||||
| 271 | if strings.IndexByte(inner, '\\') < 0 && strings.IndexByte(inner, '"') < 0 { | 317 | if strings.IndexByte(inner, '\\') < 0 && strings.IndexByte(inner, '"') < 0 { | ||
| 272 | return inner // the common case, without allocating | 318 | return inner // the common case, without allocating | ||
| 273 | } | 319 | } | ||
| 274 | - | if s, err := strconv.Unquote(v); err == nil { | 320 | + | // An escape such as \x80 could make invalid UTF-8: keep it as written. |
| 321 | + | if s, err := strconv.Unquote(v); err == nil && utf8.ValidString(s) { | |||
| 275 | return s | 322 | return s | ||
| 276 | } | 323 | } | ||
| 277 | case '\'': | 324 | case '\'': | ||
M parse_test.go +66 -2
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "errors" | |
| 5 | 5 | "reflect" | |
| 6 | 6 | "slices" | |
| 7 | + | "strconv" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "testing" | |
| 9 | 10 | ) | |
| @@ -85,6 +86,11 @@ g = spawn "foo bar" | |||
| 85 | 86 | in: "title = it's fine # a note\nb = say \"x # y\" # c\n", | |
| 86 | 87 | want: []Entry{{"title", "it's fine", 1}, {"b", `say "x # y"`, 2}}, | |
| 87 | 88 | }, | |
| 89 | + | { | |
| 90 | + | name: "escape making invalid UTF-8 is kept as written", | |
| 91 | + | in: `a = "\200"` + "\n", | |
| 92 | + | want: []Entry{{"a", `"\200"`, 1}}, | |
| 93 | + | }, | |
| 88 | 94 | { | |
| 89 | 95 | name: "raw values", | |
| 90 | 96 | in: `a = "x"` + "\n", | |
| @@ -97,6 +103,12 @@ g = spawn "foo bar" | |||
| 97 | 103 | want: []Entry{{"a", "1", 1}, {"a", "2", 2}}, | |
| 98 | 104 | warnings: []string{"line 2: a: overrides line 1"}, | |
| 99 | 105 | }, | |
| 106 | + | { | |
| 107 | + | name: "duplicates and bad lines in line order", | |
| 108 | + | in: "a = 1\nb = 1\nnope\na = 2\n= x\nb = 2\na = 3\n", | |
| 109 | + | want: []Entry{{"a", "1", 1}, {"b", "1", 2}, {"a", "2", 4}, {"b", "2", 6}, {"a", "3", 7}}, | |
| 110 | + | warnings: []string{"line 3: expected key = value", "line 4: a: overrides line 1", "line 5: missing key", "line 6: b: overrides line 2", "line 7: a: overrides line 4"}, | |
| 111 | + | }, | |
| 100 | 112 | { | |
| 101 | 113 | name: "duplicates allowed", | |
| 102 | 114 | in: "a = 1\na = 2\n", | |
| @@ -128,6 +140,20 @@ g = spawn "foo bar" | |||
| 128 | 140 | err: "line 2: a: overrides line 1", | |
| 129 | 141 | warnings: []string{"line 2: a: overrides line 1"}, | |
| 130 | 142 | }, | |
| 143 | + | { | |
| 144 | + | name: "strict duplicate before a bad line", | |
| 145 | + | in: "a = 1\na = 2\nnope\n", | |
| 146 | + | opts: Options{Strict: true}, | |
| 147 | + | err: "line 2: a: overrides line 1", | |
| 148 | + | warnings: []string{"line 2: a: overrides line 1"}, | |
| 149 | + | }, | |
| 150 | + | { | |
| 151 | + | name: "strict bad line before a duplicate", | |
| 152 | + | in: "a = 1\nnope\na = 2\n", | |
| 153 | + | opts: Options{Strict: true}, | |
| 154 | + | err: "line 2: expected key = value", | |
| 155 | + | warnings: []string{"line 2: expected key = value"}, | |
| 156 | + | }, | |
| 131 | 157 | { | |
| 132 | 158 | name: "long line", | |
| 133 | 159 | in: "a = " + strings.Repeat("x", 20) + "\nb = 1\n", | |
| @@ -208,8 +234,42 @@ func TestChanged(t *testing.T) { | |||
| 208 | 234 | } | |
| 209 | 235 | } | |
| 210 | 236 | ||
| 237 | + | // FuzzDuplicates checks the duplicate warnings against a map. | |
| 238 | + | func FuzzDuplicates(f *testing.F) { | |
| 239 | + | for _, s := range []string{"a=1\na=2\nb=1\na=3\n", "x\n=1\na=1\na=1\n"} { | |
| 240 | + | f.Add(s) | |
| 241 | + | } | |
| 242 | + | f.Fuzz(func(t *testing.T, in string) { | |
| 243 | + | all, syntax, err := ParseBytes([]byte(in), Options{AllowDuplicates: true}) | |
| 244 | + | if err != nil { | |
| 245 | + | return | |
| 246 | + | } | |
| 247 | + | var want []string | |
| 248 | + | seen := map[string]int{} | |
| 249 | + | for _, e := range all { | |
| 250 | + | if prev, ok := seen[e.Key]; ok { | |
| 251 | + | want = append(want, Warning{Line: e.Line, Key: e.Key, Msg: "overrides line " + strconv.Itoa(prev)}.Error()) | |
| 252 | + | } | |
| 253 | + | seen[e.Key] = e.Line | |
| 254 | + | } | |
| 255 | + | for _, w := range syntax { | |
| 256 | + | want = append(want, w.Error()) | |
| 257 | + | } | |
| 258 | + | _, got, _ := ParseBytes([]byte(in), Options{}) | |
| 259 | + | gotS := warningStrings(got) | |
| 260 | + | slices.Sort(want) | |
| 261 | + | slices.Sort(gotS) | |
| 262 | + | if !slices.Equal(gotS, want) { | |
| 263 | + | t.Fatalf("warnings = %q, want %q", gotS, want) | |
| 264 | + | } | |
| 265 | + | if !slices.IsSortedFunc(got, func(a, b Warning) int { return a.Line - b.Line }) { | |
| 266 | + | t.Fatalf("warnings out of line order: %q", warningStrings(got)) | |
| 267 | + | } | |
| 268 | + | }) | |
| 269 | + | } | |
| 270 | + | ||
| 211 | 271 | func FuzzParse(f *testing.F) { | |
| 212 | - | for _, s := range []string{"a = 1\n", "a = \"x # y\" # c\n", "\ufeffa=b\r\n", "x\n=\n'"} { | |
| 272 | + | for _, s := range []string{"a = 1\n", "a = \"x # y\" # c\n", "\ufeffa=b\r\n", "x\n=\n'", "\ufeff\ufeff=", `0="\200"`} { | |
| 213 | 273 | f.Add(s) | |
| 214 | 274 | } | |
| 215 | 275 | f.Fuzz(func(t *testing.T, in string) { | |
| @@ -217,7 +277,11 @@ func FuzzParse(f *testing.F) { | |||
| 217 | 277 | if err != nil { | |
| 218 | 278 | return | |
| 219 | 279 | } | |
| 220 | - | // Whatever was read can be written and read back the same. | |
| 280 | + | // Whatever was read can be written and read back the same, except a | |
| 281 | + | // key that Set refuses to write. | |
| 282 | + | if slices.ContainsFunc(entries, func(e Entry) bool { return !validKey(e.Key) }) { | |
| 283 | + | return | |
| 284 | + | } | |
| 221 | 285 | var data []byte | |
| 222 | 286 | for _, e := range entries { | |
| 223 | 287 | data = append(data, e.Key+" = "+Quote(e.Value)+"\n"...) | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "reflect" | 5 | "reflect" | ||
| 6 | "slices" | 6 | "slices" | ||
| 7 | + | "strconv" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "testing" | 9 | "testing" | ||
| 9 | ) | 10 | ) | ||
| @@ -85,6 +86,11 @@ g = spawn "foo bar" | |||||
| 85 | in: "title = it's fine # a note\nb = say \"x # y\" # c\n", | 86 | in: "title = it's fine # a note\nb = say \"x # y\" # c\n", | ||
| 86 | want: []Entry{{"title", "it's fine", 1}, {"b", `say "x # y"`, 2}}, | 87 | want: []Entry{{"title", "it's fine", 1}, {"b", `say "x # y"`, 2}}, | ||
| 87 | }, | 88 | }, | ||
| 89 | + | { | |||
| 90 | + | name: "escape making invalid UTF-8 is kept as written", | |||
| 91 | + | in: `a = "\200"` + "\n", | |||
| 92 | + | want: []Entry{{"a", `"\200"`, 1}}, | |||
| 93 | + | }, | |||
| 88 | { | 94 | { | ||
| 89 | name: "raw values", | 95 | name: "raw values", | ||
| 90 | in: `a = "x"` + "\n", | 96 | in: `a = "x"` + "\n", | ||
| @@ -97,6 +103,12 @@ g = spawn "foo bar" | |||||
| 97 | want: []Entry{{"a", "1", 1}, {"a", "2", 2}}, | 103 | want: []Entry{{"a", "1", 1}, {"a", "2", 2}}, | ||
| 98 | warnings: []string{"line 2: a: overrides line 1"}, | 104 | warnings: []string{"line 2: a: overrides line 1"}, | ||
| 99 | }, | 105 | }, | ||
| 106 | + | { | |||
| 107 | + | name: "duplicates and bad lines in line order", | |||
| 108 | + | in: "a = 1\nb = 1\nnope\na = 2\n= x\nb = 2\na = 3\n", | |||
| 109 | + | want: []Entry{{"a", "1", 1}, {"b", "1", 2}, {"a", "2", 4}, {"b", "2", 6}, {"a", "3", 7}}, | |||
| 110 | + | warnings: []string{"line 3: expected key = value", "line 4: a: overrides line 1", "line 5: missing key", "line 6: b: overrides line 2", "line 7: a: overrides line 4"}, | |||
| 111 | + | }, | |||
| 100 | { | 112 | { | ||
| 101 | name: "duplicates allowed", | 113 | name: "duplicates allowed", | ||
| 102 | in: "a = 1\na = 2\n", | 114 | in: "a = 1\na = 2\n", | ||
| @@ -128,6 +140,20 @@ g = spawn "foo bar" | |||||
| 128 | err: "line 2: a: overrides line 1", | 140 | err: "line 2: a: overrides line 1", | ||
| 129 | warnings: []string{"line 2: a: overrides line 1"}, | 141 | warnings: []string{"line 2: a: overrides line 1"}, | ||
| 130 | }, | 142 | }, | ||
| 143 | + | { | |||
| 144 | + | name: "strict duplicate before a bad line", | |||
| 145 | + | in: "a = 1\na = 2\nnope\n", | |||
| 146 | + | opts: Options{Strict: true}, | |||
| 147 | + | err: "line 2: a: overrides line 1", | |||
| 148 | + | warnings: []string{"line 2: a: overrides line 1"}, | |||
| 149 | + | }, | |||
| 150 | + | { | |||
| 151 | + | name: "strict bad line before a duplicate", | |||
| 152 | + | in: "a = 1\nnope\na = 2\n", | |||
| 153 | + | opts: Options{Strict: true}, | |||
| 154 | + | err: "line 2: expected key = value", | |||
| 155 | + | warnings: []string{"line 2: expected key = value"}, | |||
| 156 | + | }, | |||
| 131 | { | 157 | { | ||
| 132 | name: "long line", | 158 | name: "long line", | ||
| 133 | in: "a = " + strings.Repeat("x", 20) + "\nb = 1\n", | 159 | in: "a = " + strings.Repeat("x", 20) + "\nb = 1\n", | ||
| @@ -208,8 +234,42 @@ func TestChanged(t *testing.T) { | |||||
| 208 | } | 234 | } | ||
| 209 | } | 235 | } | ||
| 210 | 236 | ||||
| 237 | + | // FuzzDuplicates checks the duplicate warnings against a map. | |||
| 238 | + | func FuzzDuplicates(f *testing.F) { | |||
| 239 | + | for _, s := range []string{"a=1\na=2\nb=1\na=3\n", "x\n=1\na=1\na=1\n"} { | |||
| 240 | + | f.Add(s) | |||
| 241 | + | } | |||
| 242 | + | f.Fuzz(func(t *testing.T, in string) { | |||
| 243 | + | all, syntax, err := ParseBytes([]byte(in), Options{AllowDuplicates: true}) | |||
| 244 | + | if err != nil { | |||
| 245 | + | return | |||
| 246 | + | } | |||
| 247 | + | var want []string | |||
| 248 | + | seen := map[string]int{} | |||
| 249 | + | for _, e := range all { | |||
| 250 | + | if prev, ok := seen[e.Key]; ok { | |||
| 251 | + | want = append(want, Warning{Line: e.Line, Key: e.Key, Msg: "overrides line " + strconv.Itoa(prev)}.Error()) | |||
| 252 | + | } | |||
| 253 | + | seen[e.Key] = e.Line | |||
| 254 | + | } | |||
| 255 | + | for _, w := range syntax { | |||
| 256 | + | want = append(want, w.Error()) | |||
| 257 | + | } | |||
| 258 | + | _, got, _ := ParseBytes([]byte(in), Options{}) | |||
| 259 | + | gotS := warningStrings(got) | |||
| 260 | + | slices.Sort(want) | |||
| 261 | + | slices.Sort(gotS) | |||
| 262 | + | if !slices.Equal(gotS, want) { | |||
| 263 | + | t.Fatalf("warnings = %q, want %q", gotS, want) | |||
| 264 | + | } | |||
| 265 | + | if !slices.IsSortedFunc(got, func(a, b Warning) int { return a.Line - b.Line }) { | |||
| 266 | + | t.Fatalf("warnings out of line order: %q", warningStrings(got)) | |||
| 267 | + | } | |||
| 268 | + | }) | |||
| 269 | + | } | |||
| 270 | + | ||||
| 211 | func FuzzParse(f *testing.F) { | 271 | func FuzzParse(f *testing.F) { | ||
| 212 | - | for _, s := range []string{"a = 1\n", "a = \"x # y\" # c\n", "\ufeffa=b\r\n", "x\n=\n'"} { | 272 | + | for _, s := range []string{"a = 1\n", "a = \"x # y\" # c\n", "\ufeffa=b\r\n", "x\n=\n'", "\ufeff\ufeff=", `0="\200"`} { |
| 213 | f.Add(s) | 273 | f.Add(s) | ||
| 214 | } | 274 | } | ||
| 215 | f.Fuzz(func(t *testing.T, in string) { | 275 | f.Fuzz(func(t *testing.T, in string) { | ||
| @@ -217,7 +277,11 @@ func FuzzParse(f *testing.F) { | |||||
| 217 | if err != nil { | 277 | if err != nil { | ||
| 218 | return | 278 | return | ||
| 219 | } | 279 | } | ||
| 220 | - | // Whatever was read can be written and read back the same. | 280 | + | // Whatever was read can be written and read back the same, except a |
| 281 | + | // key that Set refuses to write. | |||
| 282 | + | if slices.ContainsFunc(entries, func(e Entry) bool { return !validKey(e.Key) }) { | |||
| 283 | + | return | |||
| 284 | + | } | |||
| 221 | var data []byte | 285 | var data []byte | ||
| 222 | for _, e := range entries { | 286 | for _, e := range entries { | ||
| 223 | data = append(data, e.Key+" = "+Quote(e.Value)+"\n"...) | 287 | data = append(data, e.Key+" = "+Quote(e.Value)+"\n"...) | ||
M write_test.go +1 -1
| @@ -81,7 +81,7 @@ func TestSet(t *testing.T) { | |||
| 81 | 81 | } | |
| 82 | 82 | ||
| 83 | 83 | func TestSetRejectsInjection(t *testing.T) { | |
| 84 | - | for _, key := range []string{"", "a = b", "a\nadmin", "bind.q = y\nadmin", "# a", " a"} { | |
| 84 | + | for _, key := range []string{"", "a = b", "a\nadmin", "bind.q = y\nadmin", "# a", " a", "\ufeffa"} { | |
| 85 | 85 | t.Run(key, func(t *testing.T) { | |
| 86 | 86 | if _, err := Set(nil, map[string]*string{key: ptr("v")}); err == nil { | |
| 87 | 87 | t.Fatalf("Set accepted key %q", key) | |
| @@ -81,7 +81,7 @@ func TestSet(t *testing.T) { | |||||
| 81 | } | 81 | } | ||
| 82 | 82 | ||||
| 83 | func TestSetRejectsInjection(t *testing.T) { | 83 | func TestSetRejectsInjection(t *testing.T) { | ||
| 84 | - | for _, key := range []string{"", "a = b", "a\nadmin", "bind.q = y\nadmin", "# a", " a"} { | 84 | + | for _, key := range []string{"", "a = b", "a\nadmin", "bind.q = y\nadmin", "# a", " a", "\ufeffa"} { |
| 85 | t.Run(key, func(t *testing.T) { | 85 | t.Run(key, func(t *testing.T) { | ||
| 86 | if _, err := Set(nil, map[string]*string{key: ptr("v")}); err == nil { | 86 | if _, err := Set(nil, map[string]*string{key: ptr("v")}); err == nil { | ||
| 87 | t.Fatalf("Set accepted key %q", key) | 87 | t.Fatalf("Set accepted key %q", key) | ||