Skip to content

Commit 6b988ba

Browse files
author
Dean Karn
authored
Merge pull request #32 from go-playground/fix-array-handling
Fix Array Handling + mixed index and non-index values
2 parents 7fecd71 + cdb4dc8 commit 6b988ba

File tree

4 files changed

+192
-101
lines changed

4 files changed

+192
-101
lines changed

‎README.md‎

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Package form
22
============
3-
<img align="right" src="https://raw.githubusercontent.com/go-playground/form/master/logo.jpg">![Project status](https://img.shields.io/badge/version-3.1.0-green.svg)
3+
<img align="right" src="https://raw.githubusercontent.com/go-playground/form/master/logo.jpg">![Project status](https://img.shields.io/badge/version-3.1.1-green.svg)
44
[![Build Status](https://semaphoreci.com/api/v1/joeybloggs/form/branches/master/badge.svg)](https://semaphoreci.com/joeybloggs/form)
55
[![Coverage Status](https://coveralls.io/repos/github/go-playground/form/badge.svg?branch=master)](https://coveralls.io/github/go-playground/form?branch=master)
66
[![Go Report Card](https://goreportcard.com/badge/github.com/go-playground/form)](https://goreportcard.com/report/github.com/go-playground/form)
@@ -12,13 +12,14 @@ Package form Decodes url.Values into Go value(s) and Encodes Go value(s) into ur
1212

1313
It has the following features:
1414

15-
- Supports map of almost all types.
16-
- Supports both Numbered and Normal arrays eg. `"Array[0]"` and just `"Array"` with multiple values passed.
17-
- Array honours the specified index. eg. if `"Array[2]"` is the only Array value passed down, it will be put at index 2; if array isn't big enough it will be expanded.
18-
- Only creates objects as necessary eg. if no `array` or `map` values are passed down, the `array` and `map` are left as their default values in the struct.
19-
- Allows for Custom Type registration.
20-
- Handles time.Time using RFC3339 time format by default, but can easily be changed by registering a Custom Type, see below.
21-
- Handles Encoding & Decoding of almost all Go types eg. can Decode into struct, array, map, int... and Encode a struct, array, map, int...
15+
- Supports map of almost all types.
16+
- Supports both Numbered and Normal arrays eg. `"Array[0]"` and just `"Array"` with multiple values passed.
17+
- Slice honours the specified index. eg. if "Slice[2]" is the only Slice value passed down, it will be put at index 2; if slice isn't big enough it will be expanded.
18+
- Array honours the specified index. eg. if "Array[2]" is the only Array value passed down, it will be put at index 2; if array isn't big enough a warning will be printed and value ignored.
19+
- Only creates objects as necessary eg. if no `array` or `map` values are passed down, the `array` and `map` are left as their default values in the struct.
20+
- Allows for Custom Type registration.
21+
- Handles time.Time using RFC3339 time format by default, but can easily be changed by registering a Custom Type, see below.
22+
- Handles Encoding & Decoding of almost all Go types eg. can Decode into struct, array, map, int... and Encode a struct, array, map, int...
2223

2324
Common Questions
2425

@@ -48,7 +49,7 @@ Installation
4849

4950
Use go get.
5051

51-
go get github.com/go-playground/form
52+
go get -u github.com/go-playground/form
5253

5354
Then import the form package into your own code.
5455

@@ -275,7 +276,7 @@ Field []*string{nil, nil, &i}
275276

276277
Benchmarks
277278
------
278-
###### Run on MacBook Pro (15-inch, 2017) using go version go1.9 darwin/amd64
279+
###### Run on MacBook Pro (15-inch, 2017) using go version go1.10.1 darwin/amd64
279280

280281
NOTE: the 1 allocation and B/op in the first 4 decodes is actually the struct allocating when passing it in, so primitives are actually zero allocation.
281282

@@ -285,26 +286,26 @@ goos: darwin
285286
goarch: amd64
286287
pkg: github.com/go-playground/form/benchmarks
287288

288-
BenchmarkSimpleUserDecodeStruct-8 5000000 243 ns/op 64 B/op 1 allocs/op
289-
BenchmarkSimpleUserDecodeStructParallel-8 20000000 72.4 ns/op 64 B/op 1 allocs/op
290-
BenchmarkSimpleUserEncodeStruct-8 2000000 683 ns/op 485 B/op 10 allocs/op
291-
BenchmarkSimpleUserEncodeStructParallel-8 10000000 205 ns/op 485 B/op 10 allocs/op
292-
BenchmarkPrimitivesDecodeStructAllPrimitivesTypes-8 2000000 739 ns/op 96 B/op 1 allocs/op
293-
BenchmarkPrimitivesDecodeStructAllPrimitivesTypesParallel-8 10000000 214 ns/op 96 B/op 1 allocs/op
294-
BenchmarkPrimitivesEncodeStructAllPrimitivesTypes-8 500000 3608 ns/op 2977 B/op 36 allocs/op
295-
BenchmarkPrimitivesEncodeStructAllPrimitivesTypesParallel-8 1000000 1013 ns/op 2978 B/op 36 allocs/op
296-
BenchmarkComplexArrayDecodeStructAllTypes-8 100000 13358 ns/op 2249 B/op 121 allocs/op
297-
BenchmarkComplexArrayDecodeStructAllTypesParallel-8 500000 3656 ns/op 2249 B/op 121 allocs/op
298-
BenchmarkComplexArrayEncodeStructAllTypes-8 100000 11574 ns/op 7112 B/op 104 allocs/op
299-
BenchmarkComplexArrayEncodeStructAllTypesParallel-8 500000 3278 ns/op 7112 B/op 104 allocs/op
300-
BenchmarkComplexMapDecodeStructAllTypes-8 100000 18945 ns/op 5305 B/op 130 allocs/op
301-
BenchmarkComplexMapDecodeStructAllTypesParallel-8 300000 5213 ns/op 5308 B/op 130 allocs/op
302-
BenchmarkComplexMapEncodeStructAllTypes-8 100000 12177 ns/op 6972 B/op 129 allocs/op
303-
BenchmarkComplexMapEncodeStructAllTypesParallel-8 500000 3409 ns/op 6971 B/op 129 allocs/op
304-
BenchmarkDecodeNestedStruct-8 500000 2642 ns/op 384 B/op 14 allocs/op
305-
BenchmarkDecodeNestedStructParallel-8 2000000 760 ns/op 384 B/op 14 allocs/op
306-
BenchmarkEncodeNestedStruct-8 1000000 1686 ns/op 693 B/op 16 allocs/op
307-
BenchmarkEncodeNestedStructParallel-8 3000000 468 ns/op 693 B/op 16 allocs/op
289+
BenchmarkSimpleUserDecodeStruct-8 5000000 236 ns/op 64 B/op 1 allocs/op
290+
BenchmarkSimpleUserDecodeStructParallel-8 20000000 82.1 ns/op 64 B/op 1 allocs/op
291+
BenchmarkSimpleUserEncodeStruct-8 2000000 627 ns/op 485 B/op 10 allocs/op
292+
BenchmarkSimpleUserEncodeStructParallel-8 10000000 223 ns/op 485 B/op 10 allocs/op
293+
BenchmarkPrimitivesDecodeStructAllPrimitivesTypes-8 2000000 724 ns/op 96 B/op 1 allocs/op
294+
BenchmarkPrimitivesDecodeStructAllPrimitivesTypesParallel-8 10000000 246 ns/op 96 B/op 1 allocs/op
295+
BenchmarkPrimitivesEncodeStructAllPrimitivesTypes-8 500000 3187 ns/op 2977 B/op 36 allocs/op
296+
BenchmarkPrimitivesEncodeStructAllPrimitivesTypesParallel-8 1000000 1106 ns/op 2977 B/op 36 allocs/op
297+
BenchmarkComplexArrayDecodeStructAllTypes-8 100000 13748 ns/op 2248 B/op 121 allocs/op
298+
BenchmarkComplexArrayDecodeStructAllTypesParallel-8 500000 4313 ns/op 2249 B/op 121 allocs/op
299+
BenchmarkComplexArrayEncodeStructAllTypes-8 200000 10758 ns/op 7113 B/op 104 allocs/op
300+
BenchmarkComplexArrayEncodeStructAllTypesParallel-8 500000 3532 ns/op 7113 B/op 104 allocs/op
301+
BenchmarkComplexMapDecodeStructAllTypes-8 100000 17644 ns/op 5305 B/op 130 allocs/op
302+
BenchmarkComplexMapDecodeStructAllTypesParallel-8 300000 5470 ns/op 5308 B/op 130 allocs/op
303+
BenchmarkComplexMapEncodeStructAllTypes-8 200000 11155 ns/op 6971 B/op 129 allocs/op
304+
BenchmarkComplexMapEncodeStructAllTypesParallel-8 500000 3768 ns/op 6971 B/op 129 allocs/op
305+
BenchmarkDecodeNestedStruct-8 500000 2462 ns/op 384 B/op 14 allocs/op
306+
BenchmarkDecodeNestedStructParallel-8 2000000 814 ns/op 384 B/op 14 allocs/op
307+
BenchmarkEncodeNestedStruct-8 1000000 1483 ns/op 693 B/op 16 allocs/op
308+
BenchmarkEncodeNestedStructParallel-8 3000000 525 ns/op 693 B/op 16 allocs/op
308309
```
309310

310311
Competitor benchmarks can be found [here](https://github.com/go-playground/form/blob/master/benchmarks/benchmarks.md)

‎decoder.go‎

Lines changed: 129 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -354,113 +354,174 @@ func (d *decoder) setFieldByType(current reflect.Value, namespace []byte, idx in
354354
v.SetBool(b)
355355
set = true
356356

357-
case reflect.Slice, reflect.Array:
358-
if !ok {
357+
case reflect.Slice:
359358

360-
d.parseMapData()
359+
d.parseMapData()
360+
// slice elements could be mixed eg. number and non-numbers Value[0]=[]string{"10"} and Value=[]string{"10","20"}
361361

362-
// maybe it's an numbered array i.e. Phone[0].Number
363-
if rd := d.findAlias(string(namespace)); rd != nil {
362+
if ok && len(arr) > 0 {
363+
var varr reflect.Value
364364

365-
var varr reflect.Value
366-
var kv key
365+
var ol int
366+
l := len(arr)
367367

368-
sl := rd.sliceLen + 1
368+
if v.IsNil() {
369+
varr = reflect.MakeSlice(v.Type(), len(arr), len(arr))
370+
} else {
369371

370-
// checking below for maxArraySize, but if array exists and already
371-
// has sufficient capacity allocated then we do not check as the code
372-
// obviously allows a capacity greater than the maxArraySize.
372+
ol = v.Len()
373+
l += ol
373374

374-
if v.IsNil() {
375+
if v.Cap() <= l {
376+
varr = reflect.MakeSlice(v.Type(), l, l)
377+
} else {
378+
// preserve predefined capacity, possibly for reuse after decoding
379+
varr = reflect.MakeSlice(v.Type(), l, v.Cap())
380+
}
381+
reflect.Copy(varr, v)
382+
}
375383

376-
if sl > d.d.maxArraySize {
377-
d.setError(namespace, fmt.Errorf(errArraySize, sl, d.d.maxArraySize))
378-
return
379-
}
384+
for i := ol; i < l; i++ {
385+
newVal := reflect.New(v.Type().Elem()).Elem()
380386

381-
varr = reflect.MakeSlice(v.Type(), sl, sl)
387+
if d.setFieldByType(newVal, namespace, i-ol) {
388+
set = true
389+
varr.Index(i).Set(newVal)
390+
}
391+
}
382392

383-
} else if v.Len() < sl {
393+
v.Set(varr)
394+
}
384395

385-
if v.Cap() <= sl {
396+
// maybe it's an numbered array i.e. Phone[0].Number
397+
if rd := d.findAlias(string(namespace)); rd != nil {
386398

387-
if sl > d.d.maxArraySize {
388-
d.setError(namespace, fmt.Errorf(errArraySize, sl, d.d.maxArraySize))
389-
return
390-
}
399+
var varr reflect.Value
400+
var kv key
391401

392-
varr = reflect.MakeSlice(v.Type(), sl, sl)
393-
} else {
394-
varr = reflect.MakeSlice(v.Type(), sl, v.Cap())
395-
}
402+
sl := rd.sliceLen + 1
396403

397-
reflect.Copy(varr, v)
404+
// checking below for maxArraySize, but if array exists and already
405+
// has sufficient capacity allocated then we do not check as the code
406+
// obviously allows a capacity greater than the maxArraySize.
398407

399-
} else {
400-
varr = v
408+
if v.IsNil() {
409+
410+
if sl > d.d.maxArraySize {
411+
d.setError(namespace, fmt.Errorf(errArraySize, sl, d.d.maxArraySize))
412+
return
401413
}
402414

403-
for i := 0; i < len(rd.keys); i++ {
415+
varr = reflect.MakeSlice(v.Type(), sl, sl)
404416

405-
kv = rd.keys[i]
406-
newVal := reflect.New(varr.Type().Elem()).Elem()
417+
} else if v.Len() < sl {
407418

408-
if kv.ivalue == -1 {
409-
d.setError(namespace, fmt.Errorf("Invalid Array index '%s'", kv.value))
410-
continue
411-
}
419+
if v.Cap() <= sl {
412420

413-
if d.setFieldByType(newVal, append(namespace, kv.searchValue...), 0) {
414-
set = true
415-
varr.Index(kv.ivalue).Set(newVal)
421+
if sl > d.d.maxArraySize {
422+
d.setError(namespace, fmt.Errorf(errArraySize, sl, d.d.maxArraySize))
423+
return
416424
}
425+
426+
varr = reflect.MakeSlice(v.Type(), sl, sl)
427+
} else {
428+
varr = reflect.MakeSlice(v.Type(), sl, v.Cap())
417429
}
418430

419-
if !set {
420-
return
431+
reflect.Copy(varr, v)
432+
433+
} else {
434+
varr = v
435+
}
436+
437+
for i := 0; i < len(rd.keys); i++ {
438+
439+
kv = rd.keys[i]
440+
newVal := reflect.New(varr.Type().Elem()).Elem()
441+
442+
if kv.ivalue == -1 {
443+
d.setError(namespace, fmt.Errorf("invalid slice index '%s'", kv.value))
444+
continue
421445
}
422446

423-
v.Set(varr)
447+
if d.setFieldByType(newVal, append(namespace, kv.searchValue...), 0) {
448+
set = true
449+
varr.Index(kv.ivalue).Set(newVal)
450+
}
424451
}
425452

426-
return
427-
}
453+
if !set {
454+
return
455+
}
428456

429-
if len(arr) == 0 {
430-
return
457+
v.Set(varr)
431458
}
432459

433-
var varr reflect.Value
434-
435-
var ol int
436-
l := len(arr)
460+
case reflect.Array:
461+
d.parseMapData()
437462

438-
if v.IsNil() {
439-
varr = reflect.MakeSlice(v.Type(), len(arr), len(arr))
440-
} else {
463+
// array elements could be mixed eg. number and non-numbers Value[0]=[]string{"10"} and Value=[]string{"10","20"}
441464

442-
ol = v.Len()
443-
l += ol
465+
if ok && len(arr) > 0 {
466+
var varr reflect.Value
444467

445-
if v.Cap() <= l {
446-
varr = reflect.MakeSlice(v.Type(), l, l)
447-
} else {
448-
// preserve predefined capacity, possibly for reuse after decoding
449-
varr = reflect.MakeSlice(v.Type(), l, v.Cap())
468+
overCapacity := v.Len() < len(arr)
469+
if overCapacity {
470+
// more values than array capacity, ignore values over capacity as it's possible some would just want
471+
// to grab the first x number of elements; in the future strict mode logic should return an error
472+
fmt.Println("warning number of post form array values is larger than array capacity, ignoring overflow values")
450473
}
474+
varr = reflect.Indirect(reflect.New(reflect.ArrayOf(v.Len(), v.Type().Elem())))
451475
reflect.Copy(varr, v)
452-
}
453476

454-
for i := ol; i < l; i++ {
455-
newVal := reflect.New(v.Type().Elem()).Elem()
477+
for i := 0; i < v.Len(); i++ {
478+
newVal := reflect.New(v.Type().Elem()).Elem()
456479

457-
if d.setFieldByType(newVal, namespace, i-ol) {
458-
set = true
459-
varr.Index(i).Set(newVal)
480+
if d.setFieldByType(newVal, namespace, 0) {
481+
set = true
482+
varr.Index(i).Set(newVal)
483+
}
460484
}
485+
v.Set(varr)
461486
}
462487

463-
v.Set(varr)
488+
// maybe it's an numbered array i.e. Phone[0].Number
489+
if rd := d.findAlias(string(namespace)); rd != nil {
490+
var varr reflect.Value
491+
var kv key
492+
493+
overCapacity := rd.sliceLen >= v.Len()
494+
if overCapacity {
495+
// more values than array capacity, ignore values over capacity as it's possible some would just want
496+
// to grab the first x number of elements; in the future strict mode logic should return an error
497+
fmt.Println("warning number of post form array values is larger than array capacity, ignoring overflow values")
498+
}
499+
varr = reflect.Indirect(reflect.New(reflect.ArrayOf(v.Len(), v.Type().Elem())))
500+
reflect.Copy(varr, v)
501+
502+
for i := 0; i < len(rd.keys); i++ {
503+
kv = rd.keys[i]
504+
if kv.ivalue >= v.Len() {
505+
continue
506+
}
507+
newVal := reflect.New(varr.Type().Elem()).Elem()
508+
509+
if kv.ivalue == -1 {
510+
d.setError(namespace, fmt.Errorf("invalid array index '%s'", kv.value))
511+
continue
512+
}
513+
514+
if d.setFieldByType(newVal, append(namespace, kv.searchValue...), 0) {
515+
set = true
516+
varr.Index(kv.ivalue).Set(newVal)
517+
}
518+
}
519+
520+
if !set {
521+
return
522+
}
523+
v.Set(varr)
524+
}
464525

465526
case reflect.Map:
466527
var rd *recursiveData

‎decoder_test.go‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ func TestDecoderErrors(t *testing.T) {
785785
}
786786

787787
test := TestError{
788-
OverFlowExistingArray: make([]int, 2, 2),
788+
OverFlowExistingArray: make([]int, 2),
789789
}
790790

791791
decoder := NewDecoder()
@@ -888,7 +888,7 @@ func TestDecoderErrors(t *testing.T) {
888888
Equal(t, k.Error(), "Array size of '1000' is larger than the maximum currently set on the decoder of '4'. To increase this limit please see, SetMaxArraySize(size uint)")
889889

890890
k = err["BadArrayIndex"]
891-
Equal(t, k.Error(), "Invalid Array index 'bad index'")
891+
Equal(t, k.Error(), "invalid slice index 'bad index'")
892892

893893
type TestError2 struct {
894894
BadMapKey map[time.Time]string
@@ -1565,3 +1565,29 @@ func TestInterfaceDecoding(t *testing.T) {
15651565
Equal(t, err, nil)
15661566
Equal(t, b.Iface, "1")
15671567
}
1568+
1569+
func TestDecodeArrayBug(t *testing.T) {
1570+
var data struct {
1571+
A [2]string
1572+
B [2]string
1573+
C [2]string
1574+
}
1575+
decoder := NewDecoder()
1576+
err := decoder.Decode(&data, url.Values{
1577+
// Mixed types
1578+
"A": {"10"},
1579+
"A[1]": {"20"},
1580+
// overflow
1581+
"B": {"10", "20", "30"},
1582+
"B[1]": {"31", "10", "20"},
1583+
"B[2]": {"40"},
1584+
// invalid array index
1585+
"C[q]": {""},
1586+
})
1587+
NotEqual(t, err, nil)
1588+
Equal(t, err.Error(), "Field Namespace:C ERROR:invalid array index 'q'")
1589+
Equal(t, data.A[0], "10")
1590+
Equal(t, data.A[1], "20")
1591+
Equal(t, data.B[0], "10")
1592+
Equal(t, data.B[1], "31")
1593+
}

‎doc.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ It has the following features:
88
- Supports map of almost all types.
99
- Supports both Numbered and Normal arrays eg. "Array[0]" and just "Array"
1010
with multiple values passed.
11+
- Slice honours the specified index. eg. if "Slice[2]" is the only Slice
12+
value passed down, it will be put at index 2; if slice isn't big enough
13+
it will be expanded.
1114
- Array honours the specified index. eg. if "Array[2]" is the only Array
1215
value passed down, it will be put at index 2; if array isn't big enough
13-
it will be expanded.
16+
a warning will be printed and value ignored.
1417
- Only creates objects as necessary eg. if no `array` or `map` values are
1518
passed down, the `array` and `map` are left as their default values in
1619
the struct.

0 commit comments

Comments
 (0)