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

Client hangs due to unexpected character parsing JSON (wrong type: string instead of int) #1086

Closed
murrekatt opened this issue Aug 27, 2017 · 13 comments
Labels
Milestone

Comments

@murrekatt
Copy link

murrekatt commented Aug 27, 2017

I ran into an issue when an int in a JSON message was accidentally passed in as a string. The caller e.g. curl hangs and the error is "unexpected character...".

I have a small gin server like so:

package main

import (
	"fmt"

	"github.com/gin-gonic/gin"
)

func main() {
	r := gin.New()
	r.Use(gin.ErrorLogger())
	r.POST("/messages", handleMessage)
	r.Run(":8080")
}

type Message struct {
	Number int `json:"number"`
}

func handleMessage(c *gin.Context) {
	p := &Message{}
	if err := c.BindJSON(p); err != nil {
		fmt.Println("ERROR", err)
		return
	}
}

Build and run this and then make a call with curl:

curl -v -X POST -d '{"number":"5"}' localhost:8080/messages

Curl hangs and there is an error printed in the server console:

ERROR main.Message: Number: readUint64: unexpected character: �, parsing 11 ..."number":"... at {"number":"5"}

I've seen this now on both Linux and OSX.

Go version: go version go1.7.4 darwin/amd64
Gin hash: 65a6dd46a50bd435597b5bcababd1b2a811c9073

@murrekatt
Copy link
Author

I forgot to mention that removing r.Use(gin.ErrorLogger()) the client no longer hangs, so there is some issue with the error.

I'll continue to investigate.

@murrekatt
Copy link
Author

The issue is in a project where I use Vendetta to manage deps and that is the version of gin I have there. In my GOPATH I have a slightly older gin version which is not showing the issue. This gin version is on hash d5b353c5d5a560322e6d96121c814115562501f7.

@murrekatt
Copy link
Author

Also works with current latest 26c3f42095b8a1378617eb4756fbf8282cb1ae89

Seems the dep json-iterator was removed and that sounds to likely have been the breaking point. I'll not investigate more for now.

@javierprovecho
Copy link
Member

@murrekatt interesting, thanks for reporting, jsonite was not removed but made not default (through build flags, see https://github.com/gin-gonic/gin/blob/master/README.md#build-with-jsoniter), reopening and pinging @taowen

@javierprovecho javierprovecho added this to the 1.x milestone Aug 27, 2017
@taowen
Copy link

taowen commented Aug 28, 2017

it is a int field, and the input is a string. what is the expected behavior?

@javierprovecho
Copy link
Member

@taowen that's not an issue, was my fault reading it. but I did found a worse issue, when @murrekatt says curl hangs, I find that cpu goes crazy 100% (1 core per request). I suspect it is some kind of loop in a goroutine. Any idea? (to test it, just run the example at top)

@taowen
Copy link

taowen commented Aug 29, 2017

jsoniter already returned a error which means the goroutine is no longer running jsoniter code. there is 0 goroutine started by jsoniter. Add some profiling to find out what is being executed when cpu goes crazy might be a easier path to resolution.

@cch123
Copy link

cch123 commented Aug 29, 2017

@javierprovecho @taowen
on linux, use sudo perf top to locate who is consuming cpu resource

  55.53%  a                        [.] github.com/json-iterator/go.writeStringSlowPathWithHTMLEscaped
  43.71%  a                        [.] unicode/utf8.DecodeRuneInString

I think the key point is writeStringSlowPathWithHTMLEscaped or DecodeRuneInString

maybe this case causes some infinite loop

@taowen
Copy link

taowen commented Aug 29, 2017

@cch123 pull request?

@taowen
Copy link

taowen commented Aug 29, 2017

@murrekatt @javierprovecho we have re-produced the bug

func Test_invalid_number(t *testing.T) {
	type Message struct {
		Number int `json:"number"`
	}
	obj := Message{}
	decoder := ConfigCompatibleWithStandardLibrary.NewDecoder(bytes.NewBufferString(`{"number":"5"}`))
	err := decoder.Decode(&obj)
	fmt.Println(err)
	ConfigCompatibleWithStandardLibrary.Marshal(err.Error())
}

There is a infinite loop in the Marshal when string is invalid utf8

@taowen
Copy link

taowen commented Aug 29, 2017

json-iterator/go#153 should be fixed now

@javierprovecho
Copy link
Member

@taowen @cch123 awesome, 👏 😄. thanks!

I'll keep this open until I merge a pr for updating gin vendor.json

easonlin404 added a commit to easonlin404/gin that referenced this issue Aug 30, 2017
@appleboy
Copy link
Member

closed via #1090

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants