From 8785d3ce031b7f042da48a63a4ea5b03cae12f6f Mon Sep 17 00:00:00 2001 From: Dean Karn Date: Thu, 13 Jul 2023 09:35:54 -0700 Subject: [PATCH] Handle invalid Slice/Array indexes better (#61) --- .github/workflows/workflow.yml | 16 ++++----- .travis.yml | 26 --------------- Makefile | 13 ++++++++ README.md | 60 ++++++++++++++-------------------- decoder.go | 6 +++- decoder_test.go | 27 +++++++++++++++ 6 files changed, 78 insertions(+), 70 deletions(-) delete mode 100644 .travis.yml create mode 100644 Makefile diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 970d1de..43bb70f 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -8,20 +8,20 @@ jobs: test: strategy: matrix: - go-version: [1.15.x, 1.16.x] + go-version: [1.17.x, 1.20.x] os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} steps: - name: Install Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v3 with: go-version: ${{ matrix.go-version }} - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Restore Cache - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-v1-go-${{ hashFiles('**/go.sum') }} @@ -32,7 +32,7 @@ jobs: run: go test -race -covermode=atomic -coverprofile="profile.cov" ./... - name: Send Coverage - if: matrix.os == 'ubuntu-latest' && matrix.go-version == '1.16.x' + if: matrix.os == 'ubuntu-latest' && matrix.go-version == '1.20.x' uses: shogo82148/actions-goveralls@v1 with: path-to-profile: profile.cov @@ -41,8 +41,8 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: golangci-lint - uses: golangci/golangci-lint-action@v2 + uses: golangci/golangci-lint-action@v3 with: - version: v1.37.1 + version: latest diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index fbc61d1..0000000 --- a/.travis.yml +++ /dev/null @@ -1,26 +0,0 @@ -language: go -go: - - 1.13.4 - - tip -matrix: - allow_failures: - - go: tip - -notifications: - email: - recipients: dean.karn@gmail.com - on_success: change - on_failure: always - -before_install: - - go install github.com/mattn/goveralls - -# Only clone the most recent commit. -git: - depth: 1 - -script: - - go test -v -race -covermode=atomic -coverprofile=coverage.coverprofile ./... - -after_success: | - goveralls -coverprofile=coverage.coverprofile -service travis-ci -repotoken $COVERALLS_TOKEN \ No newline at end of file diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..0ff37bc --- /dev/null +++ b/Makefile @@ -0,0 +1,13 @@ +all: lint test bench + +lint: + golangci-lint run --timeout 5m + +test: + go test -covermode=atomic -race ./... + +bench: + go test -bench=. -benchmem ./... + +.PHONY: test lint bench +.DEFAULT_GOAL := all diff --git a/README.md b/README.md index 647100f..2e76aee 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ Package form ============ -![Project status](https://img.shields.io/badge/version-4.2.0-green.svg) +![Project status](https://img.shields.io/badge/version-4.2.1-green.svg) [![Build Status](https://github.com/go-playground/form/actions/workflows/workflow.yml/badge.svg)](https://github.com/go-playground/form/actions/workflows/workflow.yml) [![Coverage Status](https://coveralls.io/repos/github/go-playground/form/badge.svg?branch=master)](https://coveralls.io/github/go-playground/form?branch=master) [![Go Report Card](https://goreportcard.com/badge/github.com/go-playground/form)](https://goreportcard.com/report/github.com/go-playground/form) @@ -277,36 +277,35 @@ Field []*string{nil, nil, &i} Benchmarks ------ -###### Run on MacBook Pro (15-inch, 2017) using go version go1.10.1 darwin/amd64 +###### Run on M1 MacBook Pro using go version go1.20.6 darwin/amd64 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. ```go -go test -run=NONE -bench=. -benchmem=true +go test -run=NONE -bench=. -benchmem=true ./... goos: darwin -goarch: amd64 -pkg: github.com/go-playground/form/benchmarks - -BenchmarkSimpleUserDecodeStruct-8 5000000 236 ns/op 64 B/op 1 allocs/op -BenchmarkSimpleUserDecodeStructParallel-8 20000000 82.1 ns/op 64 B/op 1 allocs/op -BenchmarkSimpleUserEncodeStruct-8 2000000 627 ns/op 485 B/op 10 allocs/op -BenchmarkSimpleUserEncodeStructParallel-8 10000000 223 ns/op 485 B/op 10 allocs/op -BenchmarkPrimitivesDecodeStructAllPrimitivesTypes-8 2000000 724 ns/op 96 B/op 1 allocs/op -BenchmarkPrimitivesDecodeStructAllPrimitivesTypesParallel-8 10000000 246 ns/op 96 B/op 1 allocs/op -BenchmarkPrimitivesEncodeStructAllPrimitivesTypes-8 500000 3187 ns/op 2977 B/op 36 allocs/op -BenchmarkPrimitivesEncodeStructAllPrimitivesTypesParallel-8 1000000 1106 ns/op 2977 B/op 36 allocs/op -BenchmarkComplexArrayDecodeStructAllTypes-8 100000 13748 ns/op 2248 B/op 121 allocs/op -BenchmarkComplexArrayDecodeStructAllTypesParallel-8 500000 4313 ns/op 2249 B/op 121 allocs/op -BenchmarkComplexArrayEncodeStructAllTypes-8 200000 10758 ns/op 7113 B/op 104 allocs/op -BenchmarkComplexArrayEncodeStructAllTypesParallel-8 500000 3532 ns/op 7113 B/op 104 allocs/op -BenchmarkComplexMapDecodeStructAllTypes-8 100000 17644 ns/op 5305 B/op 130 allocs/op -BenchmarkComplexMapDecodeStructAllTypesParallel-8 300000 5470 ns/op 5308 B/op 130 allocs/op -BenchmarkComplexMapEncodeStructAllTypes-8 200000 11155 ns/op 6971 B/op 129 allocs/op -BenchmarkComplexMapEncodeStructAllTypesParallel-8 500000 3768 ns/op 6971 B/op 129 allocs/op -BenchmarkDecodeNestedStruct-8 500000 2462 ns/op 384 B/op 14 allocs/op -BenchmarkDecodeNestedStructParallel-8 2000000 814 ns/op 384 B/op 14 allocs/op -BenchmarkEncodeNestedStruct-8 1000000 1483 ns/op 693 B/op 16 allocs/op -BenchmarkEncodeNestedStructParallel-8 3000000 525 ns/op 693 B/op 16 allocs/op +goarch: arm64 +pkg: github.com/go-playground/form/v4/benchmarks +BenchmarkSimpleUserDecodeStruct-8 8704111 121.1 ns/op 64 B/op 1 allocs/op +BenchmarkSimpleUserDecodeStructParallel-8 35916134 32.89 ns/op 64 B/op 1 allocs/op +BenchmarkSimpleUserEncodeStruct-8 3746173 320.7 ns/op 485 B/op 10 allocs/op +BenchmarkSimpleUserEncodeStructParallel-8 7293147 180.0 ns/op 485 B/op 10 allocs/op +BenchmarkPrimitivesDecodeStructAllPrimitivesTypes-8 2993259 400.5 ns/op 96 B/op 1 allocs/op +BenchmarkPrimitivesDecodeStructAllPrimitivesTypesParallel-8 13023300 97.70 ns/op 96 B/op 1 allocs/op +BenchmarkPrimitivesEncodeStructAllPrimitivesTypes-8 643202 1767 ns/op 2977 B/op 35 allocs/op +BenchmarkPrimitivesEncodeStructAllPrimitivesTypesParallel-8 1000000 1202 ns/op 2978 B/op 35 allocs/op +BenchmarkComplexArrayDecodeStructAllTypes-8 172630 6822 ns/op 2008 B/op 121 allocs/op +BenchmarkComplexArrayDecodeStructAllTypesParallel-8 719788 1735 ns/op 2009 B/op 121 allocs/op +BenchmarkComplexArrayEncodeStructAllTypes-8 197052 5839 ns/op 7087 B/op 104 allocs/op +BenchmarkComplexArrayEncodeStructAllTypesParallel-8 348039 3247 ns/op 7089 B/op 104 allocs/op +BenchmarkComplexMapDecodeStructAllTypes-8 139246 8550 ns/op 5313 B/op 130 allocs/op +BenchmarkComplexMapDecodeStructAllTypesParallel-8 409018 3143 ns/op 5317 B/op 130 allocs/op +BenchmarkComplexMapEncodeStructAllTypes-8 208833 5515 ns/op 4257 B/op 103 allocs/op +BenchmarkComplexMapEncodeStructAllTypesParallel-8 523833 2182 ns/op 4258 B/op 103 allocs/op +BenchmarkDecodeNestedStruct-8 807690 1408 ns/op 344 B/op 14 allocs/op +BenchmarkDecodeNestedStructParallel-8 3409441 359.6 ns/op 344 B/op 14 allocs/op +BenchmarkEncodeNestedStruct-8 1488520 803.6 ns/op 653 B/op 16 allocs/op +BenchmarkEncodeNestedStructParallel-8 3570204 346.6 ns/op 653 B/op 16 allocs/op ``` Competitor benchmarks can be found [here](https://github.com/go-playground/form/blob/master/benchmarks/benchmarks.md) @@ -319,15 +318,6 @@ Here is a list of software that compliments using this library post decoding. * [Validator](https://github.com/go-playground/validator) - Go Struct and Field validation, including Cross Field, Cross Struct, Map, Slice and Array diving. * [mold](https://github.com/go-playground/mold) - Is a general library to help modify or set data within data structures and other objects. -Package Versioning ----------- -I'm jumping on the vendoring bandwagon, you should vendor this package as I will not -be creating different version with gopkg.in like allot of my other libraries. - -Why? because my time is spread pretty thin maintaining all of the libraries I have + LIFE, -it is so freeing not to worry about it and will help me keep pouring out bigger and better -things for you the community. - License ------ Distributed under MIT License, please see license file in code for more details. diff --git a/decoder.go b/decoder.go index e71c977..e212422 100644 --- a/decoder.go +++ b/decoder.go @@ -109,7 +109,11 @@ func (d *decoder) parseMapData() { // no need to check for error, it will always pass // as we have done the checking to ensure // the value is a number ahead of time. - ke.ivalue, _ = strconv.Atoi(ke.value) + var err error + ke.ivalue, err = strconv.Atoi(ke.value) + if err != nil { + ke.ivalue = -1 + } if ke.ivalue > rd.sliceLen { rd.sliceLen = ke.ivalue diff --git a/decoder_test.go b/decoder_test.go index 8a54f04..99897ed 100644 --- a/decoder_test.go +++ b/decoder_test.go @@ -1909,3 +1909,30 @@ func TestDecoder_EmptyArrayBool(t *testing.T) { err := d.Decode(v, in) Equal(t, err, nil) } + +func TestDecoder_InvalidSliceIndex(t *testing.T) { + type PostsRequest struct { + PostIds []string + } + in := url.Values{ + "PostIds[]": []string{"1", "2"}, + } + + v := new(PostsRequest) + d := NewDecoder() + err := d.Decode(v, in) + NotEqual(t, err, nil) + Equal(t, err.Error(), "Field Namespace:PostIds ERROR:invalid slice index ''") + + // No error with proper name + type PostsRequest2 struct { + PostIds []string `form:"PostIds[]"` + } + + v2 := new(PostsRequest2) + err = d.Decode(v2, in) + Equal(t, err, nil) + Equal(t, len(v2.PostIds), 2) + Equal(t, v2.PostIds[0], "1") + Equal(t, v2.PostIds[1], "2") +}