Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nulls.Int quietly disregards bad data #341

Closed
nscharfe opened this issue Jan 16, 2019 · 1 comment
Closed

nulls.Int quietly disregards bad data #341

nscharfe opened this issue Jan 16, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@nscharfe
Copy link

Description

A couple things I noticed when calling c.Bind in actions handlers:

  • Unexpected data types can cause panics
  • Unexpected data types can cause data to be silently discarded

Steps to Reproduce the Problem

Using buffalo in api mode. A simplified example:
I have a model defined like this:

type MyModel struct {
  ID                      uuid.UUID
  MyFloatField            nulls.Float
  MyIntField              nulls.Int
}

My create handler has the following lines in it:

...
myModel := &models.MyModel{}
if err := c.Bind(myModel); err != nil {
    return errors.WithStack(err)
}
...

Issue 1:

  • I post {"my_float_field": "5.5"} to /create => I get a 500 and it seems that c.Bind is panicking internally (It is expecting a float and gets a string). I would expect c.Bind to return an error so that I can handle it gracefully rather than panic / 500.
  • I post ``{"my_int_field": "5.5"} to /create => binding silently discards `my_int_field` value and the struct field values are all null after binding, no error is returned

Expected Behavior

Expect to get error returned in this case rather than a panic and/or silent loss of data

Info

### Buffalo Version
v0.13.12

### App Information
Pwd=/Users/dev/go/src/github.com/af/institutional-api
Root=/Users/dev/go/src/github.com/af/institutional-api
GoPath=/Users/dev/go
Name=institutional-api
Bin=bin/institutional-api
PackagePkg=github.com/af/institutional-api
ActionsPkg=github.com/af/institutional-api/actions
ModelsPkg=github.com/af/institutional-api/models
GriftsPkg=github.com/af/institutional-api/grifts
VCS=git
WithPop=true
WithSQLite=false
WithDep=true
WithWebpack=false
WithYarn=false
WithDocker=true
WithGrifts=true
WithModules=false

### Go Version
go version go1.11.2 darwin/amd64

### Go Env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/nscharfe/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/nscharfe/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/st/nhlg61pn5_s7m6v7by0wjmtc0000gn/T/go-build752508143=/tmp/go-build -gno-record-gcc-switches -fno-common"

### Node Version
v6.0.0

### NPM Version
3.8.6

### Yarn Version
1.9.4

### PostgreSQL Version
pg_ctl (PostgreSQL) 10.5

### MySQL Version
MySQL Not Found

### SQLite Version
3.24.0 2018-06-04 14:10:15 95fbac39baaab1c3a84fdfc82ccb7f42398b2e92f18a2a57bce1d4a713cbaapl

### Dep Version
dep:
 version     : devel
 build date  : 
 git hash    : 
 go version  : go1.11.2
 go compiler : gc
 platform    : darwin/amd64
 features    : ImportDuringSolve=false
@markbates
Copy link
Member

I'm not able to get a panic when trying to test this in v0.14.0.beta2:

type badActor struct {
	A float64
	B int
}

func Test_Exec_Bad_Actors(t *testing.T) {
	r := require.New(t)
	rd := strings.NewReader(`{"A": "5.5", "B": "adsf"}`)
	req := httptest.NewRequest("POST", "/", rd)
	req.Header.Set("Content-Type", "application/json")
	a := &badActor{}
	err := Exec(req, a)
	r.Error(err)
}

The error returned is json: cannot unmarshal string into Go struct field badActor.A of type float64.

When I try to use nulls.Float64 instead of a standard float64, I get strconv.ParseFloat: parsing "\"5.5\"": invalid syntax

The only thing I can confirm is that nulls.Int seems to be throwing away bad data. I'm going to transfer this issue to that package, as I can't get buffalo to reproduce the panic's, but I can reproduce the issue with nulls.Int.

@markbates markbates transferred this issue from gobuffalo/buffalo Jan 23, 2019
@markbates markbates changed the title Unexpected Bind behavior nulls.Int quietly disregards bad data Jan 23, 2019
stanislas-m added a commit that referenced this issue Feb 14, 2019
@stanislas-m stanislas-m added the bug Something isn't working label Feb 14, 2019
stanislas-m added a commit that referenced this issue Feb 14, 2019
* Handle nulls.Int Unmarshal errors properly

Fixes #341

* Add tests for nulls.Int UnmarshalJSON
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants