Gin: Panic after merging pull request with jsoniter

[mtfelian]

Copying my comment on commit https://github.com/gin-gonic/gin/commit/ad087650e9881c93a19fd8db75a86968aa998cac

It broke compatibility with rendering some structs to JSON

We have a struct with non-exportable field tagged as json:"-"

All works on last commit c4249f923fc7b0
but after merging this it broke.

2017/07/08 14:52:46 [Recovery] panic recovered:
GET /user?id=106848 HTTP/1.1
Host: 127.0.0.1:41184
Accept-Encoding: gzip
User-Agent: Go-http-client/1.1
User-Token: 106848V2Z1akVxa1NsMVdZUUFIRzNSSlNudGZiaGkyV01nc09sMlltQTlUeW9sWG


[optional]: [user.User]: [optional]: [gorm.DB]: [optional]: [gorm.search]: [optional]: [gorm.DB]: [optional]: [gorm.Callback]: [slice]: [optional]: unsupported type: func(*gorm.Scope)
/usr/local/go/src/runtime/panic.go:443 (0x42db99)
    gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/home/felian/go_code/src/github.com/gin-gonic/gin/render/json.go:33 (0x82fa57)
    JSON.Render: panic(err)
<autogenerated>:10 (0x83191a)
/home/felian/go_code/src/github.com/gin-gonic/gin/context.go:598 (0x66c46a)
    (*Context).Render: if err := r.Render(c.Writer); err != nil {
/home/felian/go_code/src/github.com/gin-gonic/gin/context.go:629 (0x66c7f2)
    (*Context).JSON: c.Render(code, render.JSON{Data: obj})
/home/felian/go_code/src/bitbucket.org/betscase/user-service/api.go:386 (0x4828fe)
    apiGetUser: c.JSON(http.StatusOK, user)
/home/felian/go_code/src/github.com/gin-gonic/gin/context.go:110 (0x66951a)
    (*Context).Next: c.handlers[c.index](c)
/home/felian/go_code/src/github.com/gin-gonic/gin/recovery.go:45 (0x67f491)
    RecoveryWithWriter.func1: c.Next()
/home/felian/go_code/src/github.com/gin-gonic/gin/context.go:110 (0x66951a)
    (*Context).Next: c.handlers[c.index](c)
/home/felian/go_code/src/github.com/gin-gonic/gin/logger.go:82 (0x67e702)
    LoggerWithWriter.func1: c.Next()
/home/felian/go_code/src/github.com/gin-gonic/gin/context.go:110 (0x66951a)
    (*Context).Next: c.handlers[c.index](c)
/home/felian/go_code/src/github.com/gin-gonic/gin/gin.go:337 (0x6735cd)
    (*Engine).handleHTTPRequest: context.Next()
/home/felian/go_code/src/github.com/gin-gonic/gin/gin.go:301 (0x673017)
    (*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/usr/local/go/src/net/http/server.go:2081 (0x621bee)
    serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/local/go/src/net/http/server.go:1472 (0x61e43e)
    (*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/local/go/src/runtime/asm_amd64.s:1998 (0x460b61)
    goexit: BYTE    $0x90   // NOP

[javierprovecho]

@mtfelian i did tried successfully the example below, could you clarify a bit more how is your struct?

package main

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

type test struct {
    Msg        string `json:"message"`
    NotVisible string `json:"-"`
}

func main() {
    r := gin.Default()
    r.GET("/ping", func(c *gin.Context) {
        c.JSON(200, &test{Msg: "hello", NotVisible: "world"})
    })
    r.Run()
}

[mtfelian]

I tested on following code:

package main

import (
    "fmt"
    "github.com/gin-gonic/gin"
    "github.com/jinzhu/gorm"
    "net/http"
)

type a struct {
    Field        int      `json:"field"`
    dbConnection *gorm.DB `json:"-"`
}

func apiTest(c *gin.Context) {
    v := a{Field:        7}
    c.JSON(http.StatusOK, v)
}

// Gin запускает роутер запросов
func Gin() *gin.Engine {
    router := gin.Default()
    router.GET("/", apiTest)
    return router
}

func main() {
    router := Gin()
    router.Run(fmt.Sprintf(":%d", 8000))
}

[javierprovecho]

@mtfelian got it!, discovered a bug in github.com/json-iterator/go, it parses the type before the flags on a supposedly discarded variable with json:"-"

package main

import json1 "encoding/json"
import json2 "github.com/json-iterator/go"
import "fmt"

type test struct {
    Msg            string `json:"message"`
    NotVisible     string `json:"-"`
    FuncNotVisible func() `json:"-"`
}

func main() {
    result1, err1 := json1.Marshal(&test{Msg: "hello", NotVisible: "world", FuncNotVisible: func() {}})
    fmt.Println(string(result1), err1)

    result2, err2 := json2.Marshal(&test{Msg: "hello", NotVisible: "world", FuncNotVisible: func() {}})
    fmt.Println(string(result2), err2)
}

[javierprovecho]

cc @appleboy

[javierprovecho]

@mtfelian already filled an issue, I'll revert the pr and put it on hold until that is fixed

[appleboy]

@javierprovecho maybe we can recommend user download the stable version until that is fixed.

[mtfelian]

@javierprovecho what about reverting the PR from master until that is fixed? It brokes CI due to go get from master branch by default.

[javierprovecho]

@appleboy yes 👍

@mtfelian maybe you want to use govendor for sticking on a stable version.

I already pull requested a fix for github.com/json-iterator/go, see https://github.com/json-iterator/go/pull/121.

[taowen]

fix merged into upstream

[afiune]

@javierprovecho should we be covering also json:"-,omitempty"?

[appleboy]

@afiune We will use encoding/json as default JSON package but you can change to jsoniter by build from other tags.

$ go build -tags=jsoniter .

see #1026

[afiune]

Interesting. Thanks for pointing that out @appleboy - I'll take a quick look.