monoculum / formam

a package for decode form's values into struct in Go
Apache License 2.0
190 stars 19 forks source link

panic on reflect #4

Closed DavidSatimeWallin closed 8 years ago

DavidSatimeWallin commented 8 years ago

Just updated (removed repo and did go get -u on this repo) and im getting a panic on reflection

18:6:37 app         | 2016/05/15 18:06:37 panic: reflect: slice index out of range
Stack trace:
goroutine 42 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
    /home/david/.gvm/gos/go1.6/src/runtime/debug/stack.go:24 +0x80
gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp.(*workerPool).workerFunc.func1(0xc82018c680, 0xc820199ed0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp/workerpool.go:207 +0x53
panic(0x99b7c0, 0xc820075c10)
    /home/david/.gvm/gos/go1.6/src/runtime/panic.go:426 +0x4e9
reflect.Value.Index(0x9877a0, 0xc8200a41d0, 0x197, 0xffffffffffffffff, 0x0, 0x0, 0x0)
    /home/david/.gvm/gos/go1.6/src/reflect/value.go:854 +0x151
gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam.(*decoder).decode(0xc820199250, 0x0, 0x0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam/formam.go:209 +0x10ad
gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam.(*decoder).end(0xc820199250, 0x0, 0x0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam/formam.go:189 +0xb5
gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam.(*decoder).begin(0xc820199250, 0x0, 0x0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam/formam.go:132 +0x562
gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam.Decode(0xc820199350, 0x970ee0, 0xc8200a41b0, 0x0, 0x0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/monoculum/formam/formam.go:68 +0x3bf
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*Context).ReadForm(0xc820098780, 0x970ee0, 0xc8200a41b0, 0x0, 0x0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/context_binder.go:98 +0x26b
gitlab.com/dvwallin/1spot/libs/handlerslib.AddEventPostHandler(0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/libs/handlerslib/eventsRouteHandler.go:45 +0x9b
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.HandlerFunc.Serve(0xc6e788, 0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/handler.go:62 +0x26
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*Context).Next(0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/context.go:154 +0x80
main.IsLord(0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/main.go:37 +0x1b8
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.HandlerFunc.Serve(0xc6eed0, 0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/handler.go:62 +0x26
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*Context).Next(0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/context.go:154 +0x80
main.IsLoggedIn(0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/main.go:19 +0x1b8
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.HandlerFunc.Serve(0xc6eec8, 0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/handler.go:62 +0x26
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*Context).Next(0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/context.go:154 +0x80
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/middleware/pongo2.(*pongo2Middleware).Serve(0x10e2380, 0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/middleware/pongo2/pongo2.go:39 +0x33
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*Context).Do(0xc820098780)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/context.go:144 +0x5d
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*tree).serve(0xc820162d20, 0xc820152480, 0xc8200df380, 0x11, 0x1)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/tree.go:140 +0x1be
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*router).serveFunc(0xc82017e190, 0xc820152480)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/router.go:159 +0x256
gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.(*router).(gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris.serveFunc)-fm(0xc820152480)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/kataras/iris/router.go:74 +0x2a
gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp.(*Server).serveConn(0xc82018a8c0, 0x7fc6450a4668, 0xc82016a2a8, 0x0, 0x0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp/server.go:1452 +0x767
gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp.(*Server).(gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp.serveConn)-fm(0x7fc6450a4668, 0xc82016a2a8, 0x0, 0x0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp/server.go:1183 +0x42
gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp.(*workerPool).workerFunc(0xc82018c680, 0xc8201d65e0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp/workerpool.go:224 +0x11c
gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp.(*workerPool).getCh.func1(0xc82018c680, 0xc8201d65e0, 0x973220, 0xc8201d65e0)
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp/workerpool.go:183 +0x2b
created by gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp.(*workerPool).getCh
    /home/david/.gvm/pkgsets/go1.6/global/src/gitlab.com/dvwallin/1spot/vendor/github.com/valyala/fasthttp/workerpool.go:185 +0x1e9
ghost commented 8 years ago

@dashaus I think the last changes you did broke it, please fix it, Iris framework uses your library and I hope that this will not change.

Edit 2: @dvwallin the iris' version is working now, @dashaus the problem was the nil slice, you're late to answer so I forced to make a fork of your code and edit it.

emilgpa commented 8 years ago

@dvwallin can you pass me the code that you are using? @kataras In the tests, the slices aren't initialize and are passed successfully.

ghost commented 8 years ago

Create a real example and you will see, this was the fix for our version of your package

emilgpa commented 8 years ago

I fixed a bug related with unmarshalText. I don't know if the bug is related with your problem @dvwallin. Try it again, please!

@kataras I use formam strongly in my project, and I use a lot slices. No problem I've found...

ghost commented 8 years ago

We had a problem with nil slices, I fixed because ,obviously, you will not xD anyway thanks for your package, is very good and simple, continue this!

emilgpa commented 8 years ago
if d.curr.IsNil() {
      _newSlice := reflect.MakeSlice(d.curr.Type(), len(d.values), len(d.values))
      reflect.Copy(_newSlice, d.curr)
      d.curr.Set(_newSlice)
}

It is your fix? Because it is redundant. The expandSlice function does this!

emilgpa commented 8 years ago

And the d.values is only used when the field is slice/array without proving an index! Be careful...

emilgpa commented 8 years ago

And I have never had to do make([]string, 0), it is not necessary. Please, create a example without to use your framework and you can see that the error is no produced

ghost commented 8 years ago

And the d.values is only used when the field is slice/array without proving an index! Be careful..

Yes, because of this I'm using this , if it's 0 then no problem (golang no-setted int types are zero) , make an empty slice, and after expand it as you say, this is not something to worry about it.

It is your fix? Because it is redundant. The expandSlice function does this!

expandSlice doesn't expand the nil slices.

And I have never had to do make([]string, 0), it is not necessary. Please, create a example without to use your framework and you can see that the error is no produced

I did with pure fasthttp and html/template. Did you try to do an example with fasthttp or you use only net/http for your tests? Please do it and after answer me correctly. Thanks!

emilgpa commented 8 years ago

expandSlice doesn't expand the nil slices.

expandSlice expands nil slices. Your fix is not a fix. Is an unnecessary redundancy. It not should to do any effect positive neither negative in your framework. Your fix is a placebo: https://play.golang.org/p/p47CqZY7A9

I did with pure fasthttp and html/template. Did you try to do an example with fasthttp or you use only net/http for your tests? Please do it and after answer me correctly. Thanks!

If fasthttp encode the form's values as do net/http, then there should not be any problem. If fasthttp encode the form's values in another way, formam is not the package to use, formam only allow url.Values for decode in a struct or slice.

ghost commented 8 years ago

Well written answer, you've right.