Merge pull request 'perf: cut Parse, Unmarshal and Marshal allocations' (#4) from perf/decode-marshal into main
Reviewed-on: https://git.bnema.dev/bnema/kvconf/pulls/4
6 files · +422 -91
M decode.go +137 -64
| @@ -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,137 @@ 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 | + | const ( | |
| 348 | + | // linearNames is how many names are searched linearly before index | |
| 349 | + | // is built. | |
| 350 | + | linearNames = 16 | |
| 351 | + | // maxElemHint caps the elements of a map of structs allocated up front. | |
| 352 | + | maxElemHint = 64 | |
| 353 | + | ) | |
| 354 | + | ||
| 355 | + | // mapFor returns the state of the map field f. The first call sets every | |
| 356 | + | // map's hint from the entries left, so a config without map keys never | |
| 357 | + | // counts. A key is counted for every prefix it starts with, so a key that | |
| 358 | + | // apply passes on to a shorter prefix is counted there. | |
| 359 | + | func (st *decodeState) mapFor(s *schema, f *field) *mapState { | |
| 360 | + | if st.maps == nil { | |
| 361 | + | st.maps = make([]mapState, len(s.prefixes)) | |
| 362 | + | for i := st.next; i < len(st.entries); i++ { | |
| 363 | + | key := st.entries[i].Key | |
| 364 | + | for _, p := range s.prefixes { | |
| 365 | + | if len(key) > len(p.key) && key[:len(p.key)] == p.key { | |
| 366 | + | st.maps[p.slot].hint++ | |
| 367 | + | } | |
| 368 | + | } | |
| 369 | + | } | |
| 333 | 370 | } | |
| 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 | |
| 371 | + | ms := &st.maps[f.slot] | |
| 372 | + | if !ms.m.IsValid() { | |
| 373 | + | ms.m = st.root.FieldByIndex(f.index) | |
| 374 | + | if f.elem != nil { | |
| 375 | + | // A name sets some of its fields: guess half of them per name, | |
| 376 | + | // capped, as elements are whole structs. elems grows if needed. | |
| 377 | + | ms.hint = min(2*ms.hint/max(len(f.elem.order), 1)+1, maxElemHint) | |
| 378 | + | } | |
| 379 | + | if ms.m.IsNil() { | |
| 380 | + | // Created even if every key is rejected, as before a strict error. | |
| 381 | + | // A map of structs gets its names in flush: hint is only a guess. | |
| 382 | + | size := ms.hint | |
| 383 | + | if f.elem != nil { | |
| 384 | + | size = 0 | |
| 385 | + | } | |
| 386 | + | ms.m.Set(reflect.MakeMapWithSize(f.typ, size)) | |
| 387 | + | } | |
| 388 | + | if f.elem == nil { | |
| 389 | + | if f.typ == stringMapType && f.val.plain() { | |
| 390 | + | ms.plain = ms.m.Interface().(map[string]string) | |
| 391 | + | } else { | |
| 392 | + | ms.val = reflect.New(f.typ.Elem()).Elem() | |
| 393 | + | } | |
| 394 | + | } | |
| 395 | + | if ms.plain == nil { | |
| 396 | + | ms.key = reflect.New(f.typ.Key()).Elem() | |
| 397 | + | } | |
| 339 | 398 | } | |
| 340 | - | return key, st.vals[f] | |
| 399 | + | return ms | |
| 341 | 400 | } | |
| 342 | 401 | ||
| 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 | |
| 402 | + | // element returns the index in ms.elems of the pending element name. | |
| 403 | + | func (ms *mapState) element(f *field, name string) int { | |
| 404 | + | if ms.index != nil { | |
| 405 | + | if i, ok := ms.index[name]; ok { | |
| 406 | + | return i | |
| 407 | + | } | |
| 408 | + | } else { | |
| 409 | + | for i := range ms.names { | |
| 410 | + | if ms.names[i].name == name { | |
| 411 | + | return i | |
| 412 | + | } | |
| 413 | + | } | |
| 352 | 414 | } | |
| 353 | - | if p, ok := byName[name]; ok { | |
| 354 | - | return p | |
| 415 | + | i := len(ms.names) | |
| 416 | + | if !ms.elems.IsValid() { | |
| 417 | + | ms.elems = reflect.MakeSlice(f.elems, ms.hint, ms.hint) | |
| 418 | + | ms.names = make([]pendingName, 0, ms.hint) | |
| 419 | + | } else if i == ms.elems.Len() { | |
| 420 | + | grown := reflect.MakeSlice(f.elems, 2*i, 2*i) | |
| 421 | + | reflect.Copy(grown, ms.elems) | |
| 422 | + | ms.elems = grown | |
| 423 | + | } | |
| 424 | + | ms.names = append(ms.names, pendingName{name: name}) | |
| 425 | + | if ms.index != nil { | |
| 426 | + | ms.index[name] = i | |
| 427 | + | } else if len(ms.names) > linearNames { | |
| 428 | + | ms.index = make(map[string]int, ms.hint) | |
| 429 | + | for j, p := range ms.names { | |
| 430 | + | ms.index[p.name] = j | |
| 431 | + | } | |
| 355 | 432 | } | |
| 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) | |
| 433 | + | ev := ms.elems.Index(i) | |
| 434 | + | ms.key.SetString(name) | |
| 435 | + | if cur := ms.m.MapIndex(ms.key); cur.IsValid() { | |
| 436 | + | ev.Set(cur) | |
| 361 | 437 | } else if f.defaults { | |
| 362 | - | p.val.Addr().Interface().(Defaulter).SetDefaults() | |
| 438 | + | ev.Addr().Interface().(Defaulter).SetDefaults() | |
| 363 | 439 | } | |
| 364 | - | byName[name] = p | |
| 365 | - | return p | |
| 440 | + | return i | |
| 366 | 441 | } | |
| 367 | 442 | ||
| 368 | 443 | // 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 { | |
| 444 | + | func (st *decodeState) flush(s *schema) { | |
| 445 | + | for slot := range st.maps { | |
| 446 | + | ms, f := &st.maps[slot], s.prefixes[slot] | |
| 447 | + | if f.elem == nil || !ms.m.IsValid() { | |
| 448 | + | continue | |
| 449 | + | } | |
| 450 | + | for i, p := range ms.names { | |
| 374 | 451 | if p.set { | |
| 375 | - | key.SetString(name) | |
| 376 | - | m.SetMapIndex(key, p.val) | |
| 452 | + | ms.key.SetString(p.name) | |
| 453 | + | ms.m.SetMapIndex(ms.key, ms.elems.Index(i)) | |
| 377 | 454 | } | |
| 378 | 455 | } | |
| 379 | 456 | } | |
| @@ -404,26 +481,22 @@ func (s *schema) apply(st *decodeState, e *Entry) (msg string, known bool) { | |||
| 404 | 481 | continue | |
| 405 | 482 | } | |
| 406 | 483 | } | |
| 407 | - | m := root.FieldByIndex(f.index) | |
| 408 | - | if m.IsNil() { | |
| 409 | - | m.Set(reflect.MakeMap(f.typ)) | |
| 410 | - | } | |
| 484 | + | ms := st.mapFor(s, f) | |
| 411 | 485 | if sub != nil { | |
| 412 | 486 | // 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 | |
| 487 | + | i := ms.element(f, name) | |
| 488 | + | if msg = sub.set(ms.elems.Index(i).FieldByIndex(sub.index), e.Value); msg == "" { | |
| 489 | + | ms.names[i].set = true | |
| 416 | 490 | } | |
| 417 | 491 | return msg, true | |
| 418 | 492 | } | |
| 419 | - | if f.typ == stringMapType && f.val.plain() { | |
| 420 | - | m.Interface().(map[string]string)[name] = e.Value | |
| 493 | + | if ms.plain != nil { | |
| 494 | + | ms.plain[name] = e.Value | |
| 421 | 495 | return "", true | |
| 422 | 496 | } | |
| 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) | |
| 497 | + | if msg = f.val.set(ms.val, e.Value); msg == "" { | |
| 498 | + | ms.key.SetString(name) | |
| 499 | + | ms.m.SetMapIndex(ms.key, ms.val) | |
| 427 | 500 | } | |
| 428 | 501 | return msg, true | |
| 429 | 502 | } | |
| @@ -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,137 @@ 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 | + | const ( |
| 331 | - | if st.keys == nil { | 348 | + | // linearNames is how many names are searched linearly before index |
| 332 | - | st.keys, st.vals = map[*field]reflect.Value{}, map[*field]reflect.Value{} | 349 | + | // is built. |
| 350 | + | linearNames = 16 | |||
| 351 | + | // maxElemHint caps the elements of a map of structs allocated up front. | |||
| 352 | + | maxElemHint = 64 | |||
| 353 | + | ) | |||
| 354 | + | ||||
| 355 | + | // mapFor returns the state of the map field f. The first call sets every | |||
| 356 | + | // map's hint from the entries left, so a config without map keys never | |||
| 357 | + | // counts. A key is counted for every prefix it starts with, so a key that | |||
| 358 | + | // apply passes on to a shorter prefix is counted there. | |||
| 359 | + | func (st *decodeState) mapFor(s *schema, f *field) *mapState { | |||
| 360 | + | if st.maps == nil { | |||
| 361 | + | st.maps = make([]mapState, len(s.prefixes)) | |||
| 362 | + | for i := st.next; i < len(st.entries); i++ { | |||
| 363 | + | key := st.entries[i].Key | |||
| 364 | + | for _, p := range s.prefixes { | |||
| 365 | + | if len(key) > len(p.key) && key[:len(p.key)] == p.key { | |||
| 366 | + | st.maps[p.slot].hint++ | |||
| 367 | + | } | |||
| 368 | + | } | |||
| 369 | + | } | |||
| 333 | } | 370 | } | ||
| 334 | - | key, ok := st.keys[f] | 371 | + | ms := &st.maps[f.slot] |
| 335 | - | if !ok { | 372 | + | if !ms.m.IsValid() { |
| 336 | - | key = reflect.New(f.typ.Key()).Elem() | 373 | + | ms.m = st.root.FieldByIndex(f.index) |
| 337 | - | val = reflect.New(f.typ.Elem()).Elem() | 374 | + | if f.elem != nil { |
| 338 | - | st.keys[f], st.vals[f] = key, val | 375 | + | // A name sets some of its fields: guess half of them per name, |
| 376 | + | // capped, as elements are whole structs. elems grows if needed. | |||
| 377 | + | ms.hint = min(2*ms.hint/max(len(f.elem.order), 1)+1, maxElemHint) | |||
| 378 | + | } | |||
| 379 | + | if ms.m.IsNil() { | |||
| 380 | + | // Created even if every key is rejected, as before a strict error. | |||
| 381 | + | // A map of structs gets its names in flush: hint is only a guess. | |||
| 382 | + | size := ms.hint | |||
| 383 | + | if f.elem != nil { | |||
| 384 | + | size = 0 | |||
| 385 | + | } | |||
| 386 | + | ms.m.Set(reflect.MakeMapWithSize(f.typ, size)) | |||
| 387 | + | } | |||
| 388 | + | if f.elem == nil { | |||
| 389 | + | if f.typ == stringMapType && f.val.plain() { | |||
| 390 | + | ms.plain = ms.m.Interface().(map[string]string) | |||
| 391 | + | } else { | |||
| 392 | + | ms.val = reflect.New(f.typ.Elem()).Elem() | |||
| 393 | + | } | |||
| 394 | + | } | |||
| 395 | + | if ms.plain == nil { | |||
| 396 | + | ms.key = reflect.New(f.typ.Key()).Elem() | |||
| 397 | + | } | |||
| 339 | } | 398 | } | ||
| 340 | - | return key, st.vals[f] | 399 | + | return ms |
| 341 | } | 400 | } | ||
| 342 | 401 | ||||
| 343 | - | // element returns the addressable pending element name of the map field f. | 402 | + | // 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 { | 403 | + | func (ms *mapState) element(f *field, name string) int { |
| 345 | - | if st.pending == nil { | 404 | + | if ms.index != nil { |
| 346 | - | st.pending = map[*field]map[string]*pendingElem{} | 405 | + | if i, ok := ms.index[name]; ok { |
| 347 | - | } | 406 | + | return i |
| 348 | - | byName := st.pending[f] | 407 | + | } |
| 349 | - | if byName == nil { | 408 | + | } else { |
| 350 | - | byName = map[string]*pendingElem{} | 409 | + | for i := range ms.names { |
| 351 | - | st.pending[f] = byName | 410 | + | if ms.names[i].name == name { |
| 411 | + | return i | |||
| 412 | + | } | |||
| 413 | + | } | |||
| 352 | } | 414 | } | ||
| 353 | - | if p, ok := byName[name]; ok { | 415 | + | i := len(ms.names) |
| 354 | - | return p | 416 | + | if !ms.elems.IsValid() { |
| 417 | + | ms.elems = reflect.MakeSlice(f.elems, ms.hint, ms.hint) | |||
| 418 | + | ms.names = make([]pendingName, 0, ms.hint) | |||
| 419 | + | } else if i == ms.elems.Len() { | |||
| 420 | + | grown := reflect.MakeSlice(f.elems, 2*i, 2*i) | |||
| 421 | + | reflect.Copy(grown, ms.elems) | |||
| 422 | + | ms.elems = grown | |||
| 423 | + | } | |||
| 424 | + | ms.names = append(ms.names, pendingName{name: name}) | |||
| 425 | + | if ms.index != nil { | |||
| 426 | + | ms.index[name] = i | |||
| 427 | + | } else if len(ms.names) > linearNames { | |||
| 428 | + | ms.index = make(map[string]int, ms.hint) | |||
| 429 | + | for j, p := range ms.names { | |||
| 430 | + | ms.index[p.name] = j | |||
| 431 | + | } | |||
| 355 | } | 432 | } | ||
| 356 | - | p := &pendingElem{val: reflect.New(f.typ.Elem()).Elem()} | 433 | + | ev := ms.elems.Index(i) |
| 357 | - | key, _ := st.scratch(f) | 434 | + | ms.key.SetString(name) |
| 358 | - | key.SetString(name) | 435 | + | if cur := ms.m.MapIndex(ms.key); cur.IsValid() { |
| 359 | - | if cur := m.MapIndex(key); cur.IsValid() { | 436 | + | ev.Set(cur) |
| 360 | - | p.val.Set(cur) | |||
| 361 | } else if f.defaults { | 437 | } else if f.defaults { | ||
| 362 | - | p.val.Addr().Interface().(Defaulter).SetDefaults() | 438 | + | ev.Addr().Interface().(Defaulter).SetDefaults() |
| 363 | } | 439 | } | ||
| 364 | - | byName[name] = p | 440 | + | return i |
| 365 | - | return p | |||
| 366 | } | 441 | } | ||
| 367 | 442 | ||||
| 368 | // flush writes the pending struct elements back to their maps. | 443 | // flush writes the pending struct elements back to their maps. | ||
| 369 | - | func (st *decodeState) flush() { | 444 | + | func (st *decodeState) flush(s *schema) { |
| 370 | - | for f, byName := range st.pending { | 445 | + | for slot := range st.maps { |
| 371 | - | m := st.root.FieldByIndex(f.index) | 446 | + | ms, f := &st.maps[slot], s.prefixes[slot] |
| 372 | - | key, _ := st.scratch(f) | 447 | + | if f.elem == nil || !ms.m.IsValid() { |
| 373 | - | for name, p := range byName { | 448 | + | continue |
| 449 | + | } | |||
| 450 | + | for i, p := range ms.names { | |||
| 374 | if p.set { | 451 | if p.set { | ||
| 375 | - | key.SetString(name) | 452 | + | ms.key.SetString(p.name) |
| 376 | - | m.SetMapIndex(key, p.val) | 453 | + | ms.m.SetMapIndex(ms.key, ms.elems.Index(i)) |
| 377 | } | 454 | } | ||
| 378 | } | 455 | } | ||
| 379 | } | 456 | } | ||
| @@ -404,26 +481,22 @@ func (s *schema) apply(st *decodeState, e *Entry) (msg string, known bool) { | |||||
| 404 | continue | 481 | continue | ||
| 405 | } | 482 | } | ||
| 406 | } | 483 | } | ||
| 407 | - | m := root.FieldByIndex(f.index) | 484 | + | ms := st.mapFor(s, f) |
| 408 | - | if m.IsNil() { | |||
| 409 | - | m.Set(reflect.MakeMap(f.typ)) | |||
| 410 | - | } | |||
| 411 | if sub != nil { | 485 | if sub != nil { | ||
| 412 | // A name whose every key is rejected is not created. | 486 | // A name whose every key is rejected is not created. | ||
| 413 | - | p := st.element(f, m, name) | 487 | + | i := ms.element(f, name) |
| 414 | - | if msg = sub.set(p.val.FieldByIndex(sub.index), e.Value); msg == "" { | 488 | + | if msg = sub.set(ms.elems.Index(i).FieldByIndex(sub.index), e.Value); msg == "" { |
| 415 | - | p.set = true | 489 | + | ms.names[i].set = true |
| 416 | } | 490 | } | ||
| 417 | return msg, true | 491 | return msg, true | ||
| 418 | } | 492 | } | ||
| 419 | - | if f.typ == stringMapType && f.val.plain() { | 493 | + | if ms.plain != nil { |
| 420 | - | m.Interface().(map[string]string)[name] = e.Value | 494 | + | ms.plain[name] = e.Value |
| 421 | return "", true | 495 | return "", true | ||
| 422 | } | 496 | } | ||
| 423 | - | key, val := st.scratch(f) | 497 | + | if msg = f.val.set(ms.val, e.Value); msg == "" { |
| 424 | - | if msg = f.val.set(val, e.Value); msg == "" { | 498 | + | ms.key.SetString(name) |
| 425 | - | key.SetString(name) | 499 | + | ms.m.SetMapIndex(ms.key, ms.val) |
| 426 | - | m.SetMapIndex(key, val) | |||
| 427 | } | 500 | } | ||
| 428 | return msg, true | 501 | return msg, true | ||
| 429 | } | 502 | } | ||
M decode_test.go +89 -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,94 @@ 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 | + | // One key per name: more names than the element hint guesses, past the | |
| 240 | + | // linear search. | |
| 241 | + | var many strings.Builder | |
| 242 | + | want := map[string]output{} | |
| 243 | + | for i := range 3 * maxElemHint { | |
| 244 | + | name := "o" + strconv.Itoa(i) | |
| 245 | + | fmt.Fprintf(&many, "output.%s.hdr = on\n", name) | |
| 246 | + | want[name] = output{Scale: 1, HDR: true, Bright: 203} | |
| 247 | + | } | |
| 248 | + | tests := []struct { | |
| 249 | + | name string | |
| 250 | + | in string | |
| 251 | + | opts Options | |
| 252 | + | start func(c *testConfig) | |
| 253 | + | check func(t *testing.T, c *testConfig) | |
| 254 | + | err bool | |
| 255 | + | }{ | |
| 256 | + | { | |
| 257 | + | name: "many names", | |
| 258 | + | in: many.String(), | |
| 259 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, want) }, | |
| 260 | + | }, | |
| 261 | + | { | |
| 262 | + | name: "one name, many keys", | |
| 263 | + | in: strings.Repeat("output.x.scale = 2\n", 200), | |
| 264 | + | opts: Options{AllowDuplicates: true}, | |
| 265 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{"x": {Scale: 2, Bright: 203}}) }, | |
| 266 | + | }, | |
| 267 | + | { | |
| 268 | + | name: "map reached before a strict error exists", | |
| 269 | + | in: "output.x.scale = 2\ncount = bad\n", | |
| 270 | + | opts: Options{Strict: true}, | |
| 271 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{}) }, | |
| 272 | + | err: true, | |
| 273 | + | }, | |
| 274 | + | { | |
| 275 | + | name: "existing elements are updated, not reset", | |
| 276 | + | in: "output.A.hdr = on\noutput.B.scale = 3\nbind.x = y\n", | |
| 277 | + | start: func(c *testConfig) { | |
| 278 | + | c.Outputs = map[string]output{"A": {Scale: 2, Bright: 100}, "C": {Scale: 1}} | |
| 279 | + | c.Binds = map[string]string{"keep": "me"} | |
| 280 | + | }, | |
| 281 | + | check: func(t *testing.T, c *testConfig) { | |
| 282 | + | eq(t, c.Outputs, map[string]output{ | |
| 283 | + | "A": {Scale: 2, HDR: true, Bright: 100}, | |
| 284 | + | "B": {Scale: 3, Bright: 203}, | |
| 285 | + | "C": {Scale: 1}, | |
| 286 | + | }) | |
| 287 | + | eq(t, c.Binds, map[string]string{"keep": "me", "x": "y"}) | |
| 288 | + | }, | |
| 289 | + | }, | |
| 290 | + | { | |
| 291 | + | name: "rejected element leaves an existing map untouched", | |
| 292 | + | in: "output.A.scale = 9\n", | |
| 293 | + | start: func(c *testConfig) { c.Outputs = map[string]output{"A": {Scale: 2}} }, | |
| 294 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{"A": {Scale: 2}}) }, | |
| 295 | + | }, | |
| 296 | + | } | |
| 297 | + | for _, tt := range tests { | |
| 298 | + | t.Run(tt.name, func(t *testing.T) { | |
| 299 | + | c := defaults() | |
| 300 | + | if tt.start != nil { | |
| 301 | + | tt.start(&c) | |
| 302 | + | } | |
| 303 | + | _, err := Unmarshal([]byte(tt.in), &c, tt.opts) | |
| 304 | + | if (err != nil) != tt.err { | |
| 305 | + | t.Fatalf("err = %v", err) | |
| 306 | + | } | |
| 307 | + | tt.check(t, &c) | |
| 308 | + | }) | |
| 309 | + | } | |
| 310 | + | ||
| 311 | + | t.Run("key matching two prefixes", func(t *testing.T) { | |
| 312 | + | // m.out.x.nope is no output field, so m.* takes it. | |
| 313 | + | var n nestedMaps | |
| 314 | + | _, err := Unmarshal([]byte("m.out.x.scale = 2\nm.out.x.nope = 1\nm.a = b\n"), &n, Options{}) | |
| 315 | + | must(t, err) | |
| 316 | + | eq(t, n.Outputs, map[string]output{"x": {Scale: 2, Bright: 203}}) | |
| 317 | + | eq(t, n.All, map[string]string{"out.x.nope": "1", "a": "b"}) | |
| 318 | + | }) | |
| 319 | + | } | |
| 320 | + | ||
| 232 | 321 | type plain struct { | |
| 233 | 322 | A int `kv:"a"` | |
| 234 | 323 | } | |
| @@ -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,94 @@ 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 | + | // One key per name: more names than the element hint guesses, past the | |||
| 240 | + | // linear search. | |||
| 241 | + | var many strings.Builder | |||
| 242 | + | want := map[string]output{} | |||
| 243 | + | for i := range 3 * maxElemHint { | |||
| 244 | + | name := "o" + strconv.Itoa(i) | |||
| 245 | + | fmt.Fprintf(&many, "output.%s.hdr = on\n", name) | |||
| 246 | + | want[name] = output{Scale: 1, HDR: true, Bright: 203} | |||
| 247 | + | } | |||
| 248 | + | tests := []struct { | |||
| 249 | + | name string | |||
| 250 | + | in string | |||
| 251 | + | opts Options | |||
| 252 | + | start func(c *testConfig) | |||
| 253 | + | check func(t *testing.T, c *testConfig) | |||
| 254 | + | err bool | |||
| 255 | + | }{ | |||
| 256 | + | { | |||
| 257 | + | name: "many names", | |||
| 258 | + | in: many.String(), | |||
| 259 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, want) }, | |||
| 260 | + | }, | |||
| 261 | + | { | |||
| 262 | + | name: "one name, many keys", | |||
| 263 | + | in: strings.Repeat("output.x.scale = 2\n", 200), | |||
| 264 | + | opts: Options{AllowDuplicates: true}, | |||
| 265 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{"x": {Scale: 2, Bright: 203}}) }, | |||
| 266 | + | }, | |||
| 267 | + | { | |||
| 268 | + | name: "map reached before a strict error exists", | |||
| 269 | + | in: "output.x.scale = 2\ncount = bad\n", | |||
| 270 | + | opts: Options{Strict: true}, | |||
| 271 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{}) }, | |||
| 272 | + | err: true, | |||
| 273 | + | }, | |||
| 274 | + | { | |||
| 275 | + | name: "existing elements are updated, not reset", | |||
| 276 | + | in: "output.A.hdr = on\noutput.B.scale = 3\nbind.x = y\n", | |||
| 277 | + | start: func(c *testConfig) { | |||
| 278 | + | c.Outputs = map[string]output{"A": {Scale: 2, Bright: 100}, "C": {Scale: 1}} | |||
| 279 | + | c.Binds = map[string]string{"keep": "me"} | |||
| 280 | + | }, | |||
| 281 | + | check: func(t *testing.T, c *testConfig) { | |||
| 282 | + | eq(t, c.Outputs, map[string]output{ | |||
| 283 | + | "A": {Scale: 2, HDR: true, Bright: 100}, | |||
| 284 | + | "B": {Scale: 3, Bright: 203}, | |||
| 285 | + | "C": {Scale: 1}, | |||
| 286 | + | }) | |||
| 287 | + | eq(t, c.Binds, map[string]string{"keep": "me", "x": "y"}) | |||
| 288 | + | }, | |||
| 289 | + | }, | |||
| 290 | + | { | |||
| 291 | + | name: "rejected element leaves an existing map untouched", | |||
| 292 | + | in: "output.A.scale = 9\n", | |||
| 293 | + | start: func(c *testConfig) { c.Outputs = map[string]output{"A": {Scale: 2}} }, | |||
| 294 | + | check: func(t *testing.T, c *testConfig) { eq(t, c.Outputs, map[string]output{"A": {Scale: 2}}) }, | |||
| 295 | + | }, | |||
| 296 | + | } | |||
| 297 | + | for _, tt := range tests { | |||
| 298 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 299 | + | c := defaults() | |||
| 300 | + | if tt.start != nil { | |||
| 301 | + | tt.start(&c) | |||
| 302 | + | } | |||
| 303 | + | _, err := Unmarshal([]byte(tt.in), &c, tt.opts) | |||
| 304 | + | if (err != nil) != tt.err { | |||
| 305 | + | t.Fatalf("err = %v", err) | |||
| 306 | + | } | |||
| 307 | + | tt.check(t, &c) | |||
| 308 | + | }) | |||
| 309 | + | } | |||
| 310 | + | ||||
| 311 | + | t.Run("key matching two prefixes", func(t *testing.T) { | |||
| 312 | + | // m.out.x.nope is no output field, so m.* takes it. | |||
| 313 | + | var n nestedMaps | |||
| 314 | + | _, err := Unmarshal([]byte("m.out.x.scale = 2\nm.out.x.nope = 1\nm.a = b\n"), &n, Options{}) | |||
| 315 | + | must(t, err) | |||
| 316 | + | eq(t, n.Outputs, map[string]output{"x": {Scale: 2, Bright: 203}}) | |||
| 317 | + | eq(t, n.All, map[string]string{"out.x.nope": "1", "a": "b"}) | |||
| 318 | + | }) | |||
| 319 | + | } | |||
| 320 | + | ||||
| 232 | type plain struct { | 321 | type plain struct { | ||
| 233 | A int `kv:"a"` | 322 | A int `kv:"a"` | ||
| 234 | } | 323 | } | ||
M marshal.go +55 -13
| @@ -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,21 +113,22 @@ 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 | 121 | names = append(names, key.String()) | |
| 122 | + | order = append(order, i) | |
| 78 | 123 | vals.Index(i).SetIterValue(it) | |
| 79 | 124 | } | |
| 80 | - | order := make([]int, n) | |
| 81 | - | for i := range order { | |
| 82 | - | order[i] = i | |
| 83 | - | } | |
| 84 | 125 | slices.SortFunc(order, func(a, b int) int { return strings.Compare(names[a], names[b]) }) | |
| 85 | 126 | for _, i := range order { | |
| 86 | 127 | name, ev := names[i], vals.Index(i) | |
| 128 | + | // Checked in name order, so the error is the same on every run. | |
| 129 | + | if strings.ContainsAny(name, "=\n\r") || name != strings.TrimSpace(name) { | |
| 130 | + | return nil, fmt.Errorf("kvconf: %s: invalid map name %q", f.key, name) | |
| 131 | + | } | |
| 87 | 132 | if f.elem == nil { | |
| 88 | 133 | if err := m.line(f.key, name, "", ev, f.val); err != nil { | |
| 89 | 134 | return nil, err | |
| @@ -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,21 +113,22 @@ 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 | names = append(names, key.String()) | ||
| 122 | + | order = append(order, i) | |||
| 78 | vals.Index(i).SetIterValue(it) | 123 | vals.Index(i).SetIterValue(it) | ||
| 79 | } | 124 | } | ||
| 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]) }) | 125 | slices.SortFunc(order, func(a, b int) int { return strings.Compare(names[a], names[b]) }) | ||
| 85 | for _, i := range order { | 126 | for _, i := range order { | ||
| 86 | name, ev := names[i], vals.Index(i) | 127 | name, ev := names[i], vals.Index(i) | ||
| 128 | + | // Checked in name order, so the error is the same on every run. | |||
| 129 | + | if strings.ContainsAny(name, "=\n\r") || name != strings.TrimSpace(name) { | |||
| 130 | + | return nil, fmt.Errorf("kvconf: %s: invalid map name %q", f.key, name) | |||
| 131 | + | } | |||
| 87 | if f.elem == nil { | 132 | if f.elem == nil { | ||
| 88 | if err := m.line(f.key, name, "", ev, f.val); err != nil { | 133 | if err := m.line(f.key, name, "", ev, f.val); err != nil { | ||
| 89 | return nil, err | 134 | return nil, err | ||
| @@ -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 +57 -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,56 @@ 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 | + | syntax := len(p.list) | |
| 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 | + | p.list = append(p.list, 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(p.list) == syntax { | |
| 177 | + | return | |
| 178 | + | } | |
| 179 | + | // Warnings are reported in line order, merged with the syntax ones. | |
| 180 | + | slices.SortStableFunc(p.list, func(a, b Warning) int { return a.Line - b.Line }) | |
| 181 | + | if p.strict { | |
| 182 | + | p.list = p.list[:1] | |
| 183 | + | } | |
| 184 | + | } | |
| 185 | + | ||
| 135 | 186 | func (p *problems) err() error { | |
| 136 | 187 | if p.strict && len(p.list) > 0 { | |
| 137 | 188 | return p.list[0] | |
| @@ -176,10 +227,6 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||
| 176 | 227 | // One copy: every key and value is a substring of it. | |
| 177 | 228 | s := strings.TrimPrefix(string(data), "\ufeff") | |
| 178 | 229 | 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 | 230 | p := problems{strict: opts.Strict} | |
| 184 | 231 | maxLine := opts.maxLine() | |
| 185 | 232 | for n := 1; s != ""; n++ { | |
| @@ -218,14 +265,12 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||
| 218 | 265 | if !opts.RawValues { | |
| 219 | 266 | value = unquote(value) | |
| 220 | 267 | } | |
| 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 | 268 | entries = append(entries, Entry{Key: key, Value: value, Line: n}) | |
| 228 | 269 | } | |
| 270 | + | if !opts.AllowDuplicates { | |
| 271 | + | // entries stops at a strict failure: its duplicates come first. | |
| 272 | + | p.duplicates(entries) | |
| 273 | + | } | |
| 229 | 274 | if err := p.err(); err != nil { | |
| 230 | 275 | return nil, p.list, err | |
| 231 | 276 | } | |
| @@ -271,7 +316,8 @@ func unquote(v string) string { | |||
| 271 | 316 | if strings.IndexByte(inner, '\\') < 0 && strings.IndexByte(inner, '"') < 0 { | |
| 272 | 317 | return inner // the common case, without allocating | |
| 273 | 318 | } | |
| 274 | - | if s, err := strconv.Unquote(v); err == nil { | |
| 319 | + | // An escape such as \x80 could make invalid UTF-8: keep it as written. | |
| 320 | + | if s, err := strconv.Unquote(v); err == nil && utf8.ValidString(s) { | |
| 275 | 321 | return s | |
| 276 | 322 | } | |
| 277 | 323 | 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,56 @@ 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 | + | syntax := len(p.list) | |||
| 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 | + | p.list = append(p.list, 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(p.list) == syntax { | |||
| 177 | + | return | |||
| 178 | + | } | |||
| 179 | + | // Warnings are reported in line order, merged with the syntax ones. | |||
| 180 | + | slices.SortStableFunc(p.list, func(a, b Warning) int { return a.Line - b.Line }) | |||
| 181 | + | if p.strict { | |||
| 182 | + | p.list = p.list[:1] | |||
| 183 | + | } | |||
| 184 | + | } | |||
| 185 | + | ||||
| 135 | func (p *problems) err() error { | 186 | func (p *problems) err() error { | ||
| 136 | if p.strict && len(p.list) > 0 { | 187 | if p.strict && len(p.list) > 0 { | ||
| 137 | return p.list[0] | 188 | return p.list[0] | ||
| @@ -176,10 +227,6 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||||
| 176 | // One copy: every key and value is a substring of it. | 227 | // One copy: every key and value is a substring of it. | ||
| 177 | s := strings.TrimPrefix(string(data), "\ufeff") | 228 | s := strings.TrimPrefix(string(data), "\ufeff") | ||
| 178 | entries := make([]Entry, 0, strings.Count(s, "=")) | 229 | 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} | 230 | p := problems{strict: opts.Strict} | ||
| 184 | maxLine := opts.maxLine() | 231 | maxLine := opts.maxLine() | ||
| 185 | for n := 1; s != ""; n++ { | 232 | for n := 1; s != ""; n++ { | ||
| @@ -218,14 +265,12 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) { | |||||
| 218 | if !opts.RawValues { | 265 | if !opts.RawValues { | ||
| 219 | value = unquote(value) | 266 | value = unquote(value) | ||
| 220 | } | 267 | } | ||
| 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}) | 268 | entries = append(entries, Entry{Key: key, Value: value, Line: n}) | ||
| 228 | } | 269 | } | ||
| 270 | + | if !opts.AllowDuplicates { | |||
| 271 | + | // entries stops at a strict failure: its duplicates come first. | |||
| 272 | + | p.duplicates(entries) | |||
| 273 | + | } | |||
| 229 | if err := p.err(); err != nil { | 274 | if err := p.err(); err != nil { | ||
| 230 | return nil, p.list, err | 275 | return nil, p.list, err | ||
| 231 | } | 276 | } | ||
| @@ -271,7 +316,8 @@ func unquote(v string) string { | |||||
| 271 | if strings.IndexByte(inner, '\\') < 0 && strings.IndexByte(inner, '"') < 0 { | 316 | if strings.IndexByte(inner, '\\') < 0 && strings.IndexByte(inner, '"') < 0 { | ||
| 272 | return inner // the common case, without allocating | 317 | return inner // the common case, without allocating | ||
| 273 | } | 318 | } | ||
| 274 | - | if s, err := strconv.Unquote(v); err == nil { | 319 | + | // An escape such as \x80 could make invalid UTF-8: keep it as written. |
| 320 | + | if s, err := strconv.Unquote(v); err == nil && utf8.ValidString(s) { | |||
| 275 | return s | 321 | return s | ||
| 276 | } | 322 | } | ||
| 277 | case '\'': | 323 | case '\'': | ||
M parse_test.go +74 -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,50 @@ 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 | + | if !slices.IsSortedFunc(got, func(a, b Warning) int { return a.Line - b.Line }) { | |
| 260 | + | t.Fatalf("warnings out of line order: %q", warningStrings(got)) | |
| 261 | + | } | |
| 262 | + | // Strict fails on the first problem in line order. | |
| 263 | + | _, _, err = ParseBytes([]byte(in), Options{Strict: true}) | |
| 264 | + | switch { | |
| 265 | + | case len(got) == 0 && err != nil: | |
| 266 | + | t.Fatalf("strict err = %v, lenient had no warning", err) | |
| 267 | + | case len(got) > 0 && (err == nil || err.Error() != got[0].Error()): | |
| 268 | + | t.Fatalf("strict err = %v, want %q", err, got[0].Error()) | |
| 269 | + | } | |
| 270 | + | gotS := warningStrings(got) | |
| 271 | + | slices.Sort(want) | |
| 272 | + | slices.Sort(gotS) | |
| 273 | + | if !slices.Equal(gotS, want) { | |
| 274 | + | t.Fatalf("warnings = %q, want %q", gotS, want) | |
| 275 | + | } | |
| 276 | + | }) | |
| 277 | + | } | |
| 278 | + | ||
| 211 | 279 | 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'"} { | |
| 280 | + | for _, s := range []string{"a = 1\n", "a = \"x # y\" # c\n", "\ufeffa=b\r\n", "x\n=\n'", "\ufeff\ufeff=", `0="\200"`} { | |
| 213 | 281 | f.Add(s) | |
| 214 | 282 | } | |
| 215 | 283 | f.Fuzz(func(t *testing.T, in string) { | |
| @@ -217,7 +285,11 @@ func FuzzParse(f *testing.F) { | |||
| 217 | 285 | if err != nil { | |
| 218 | 286 | return | |
| 219 | 287 | } | |
| 220 | - | // Whatever was read can be written and read back the same. | |
| 288 | + | // Whatever was read can be written and read back the same, except a | |
| 289 | + | // key that Set refuses to write. | |
| 290 | + | if slices.ContainsFunc(entries, func(e Entry) bool { return !validKey(e.Key) }) { | |
| 291 | + | return | |
| 292 | + | } | |
| 221 | 293 | var data []byte | |
| 222 | 294 | for _, e := range entries { | |
| 223 | 295 | 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,50 @@ 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 | + | if !slices.IsSortedFunc(got, func(a, b Warning) int { return a.Line - b.Line }) { | |||
| 260 | + | t.Fatalf("warnings out of line order: %q", warningStrings(got)) | |||
| 261 | + | } | |||
| 262 | + | // Strict fails on the first problem in line order. | |||
| 263 | + | _, _, err = ParseBytes([]byte(in), Options{Strict: true}) | |||
| 264 | + | switch { | |||
| 265 | + | case len(got) == 0 && err != nil: | |||
| 266 | + | t.Fatalf("strict err = %v, lenient had no warning", err) | |||
| 267 | + | case len(got) > 0 && (err == nil || err.Error() != got[0].Error()): | |||
| 268 | + | t.Fatalf("strict err = %v, want %q", err, got[0].Error()) | |||
| 269 | + | } | |||
| 270 | + | gotS := warningStrings(got) | |||
| 271 | + | slices.Sort(want) | |||
| 272 | + | slices.Sort(gotS) | |||
| 273 | + | if !slices.Equal(gotS, want) { | |||
| 274 | + | t.Fatalf("warnings = %q, want %q", gotS, want) | |||
| 275 | + | } | |||
| 276 | + | }) | |||
| 277 | + | } | |||
| 278 | + | ||||
| 211 | func FuzzParse(f *testing.F) { | 279 | 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'"} { | 280 | + | 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) | 281 | f.Add(s) | ||
| 214 | } | 282 | } | ||
| 215 | f.Fuzz(func(t *testing.T, in string) { | 283 | f.Fuzz(func(t *testing.T, in string) { | ||
| @@ -217,7 +285,11 @@ func FuzzParse(f *testing.F) { | |||||
| 217 | if err != nil { | 285 | if err != nil { | ||
| 218 | return | 286 | return | ||
| 219 | } | 287 | } | ||
| 220 | - | // Whatever was read can be written and read back the same. | 288 | + | // Whatever was read can be written and read back the same, except a |
| 289 | + | // key that Set refuses to write. | |||
| 290 | + | if slices.ContainsFunc(entries, func(e Entry) bool { return !validKey(e.Key) }) { | |||
| 291 | + | return | |||
| 292 | + | } | |||
| 221 | var data []byte | 293 | var data []byte | ||
| 222 | for _, e := range entries { | 294 | for _, e := range entries { | ||
| 223 | data = append(data, e.Key+" = "+Quote(e.Value)+"\n"...) | 295 | data = append(data, e.Key+" = "+Quote(e.Value)+"\n"...) | ||
M write_test.go +10 -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) | |
| @@ -106,6 +106,15 @@ func TestMarshalRejectsInjection(t *testing.T) { | |||
| 106 | 106 | } | |
| 107 | 107 | }) | |
| 108 | 108 | } | |
| 109 | + | t.Run("first invalid name in order", func(t *testing.T) { | |
| 110 | + | v := &marshalConfig{Binds: map[string]string{" d": "", " b": "", " a": "", " c": ""}} | |
| 111 | + | for range 20 { // map order is random | |
| 112 | + | _, err := Marshal(v) | |
| 113 | + | if err == nil || err.Error() != `kvconf: bind.: invalid map name " a"` { | |
| 114 | + | t.Fatalf("err = %v", err) | |
| 115 | + | } | |
| 116 | + | } | |
| 117 | + | }) | |
| 109 | 118 | } | |
| 110 | 119 | ||
| 111 | 120 | // upper marshals with a pointer receiver, like many TextMarshalers. | |
| @@ -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) | ||
| @@ -106,6 +106,15 @@ func TestMarshalRejectsInjection(t *testing.T) { | |||||
| 106 | } | 106 | } | ||
| 107 | }) | 107 | }) | ||
| 108 | } | 108 | } | ||
| 109 | + | t.Run("first invalid name in order", func(t *testing.T) { | |||
| 110 | + | v := &marshalConfig{Binds: map[string]string{" d": "", " b": "", " a": "", " c": ""}} | |||
| 111 | + | for range 20 { // map order is random | |||
| 112 | + | _, err := Marshal(v) | |||
| 113 | + | if err == nil || err.Error() != `kvconf: bind.: invalid map name " a"` { | |||
| 114 | + | t.Fatalf("err = %v", err) | |||
| 115 | + | } | |||
| 116 | + | } | |||
| 117 | + | }) | |||
| 109 | } | 118 | } | ||
| 110 | 119 | ||||
| 111 | // upper marshals with a pointer receiver, like many TextMarshalers. | 120 | // upper marshals with a pointer receiver, like many TextMarshalers. | ||