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

encoding/json: ",string" bool field accepts "terrible" as true #15146

Closed
chanxuehong opened this issue Apr 6, 2016 · 14 comments

Comments

Projects
None yet
10 participants
@chanxuehong
Copy link
Contributor

commented Apr 6, 2016

EDIT by @bradfitz: see below in comment #15146 (comment) ; Original bug report follows...

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
go version go1.6 windows/amd64
  1. What operating system and processor architecture are you using (go env)?
set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=d:\gopath
set GORACE=
set GOROOT=d:\go
set GOTOOLDIR=d:\go\pkg\tool\windows_amd64
set GO15VENDOREXPERIMENT=1
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1
  1. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.

http://play.golang.org/p/Jmzo5N4kHV

package main

import (
    "encoding/json"
    "fmt"
)

type X struct {
    A string `json:"a,string"`
    B bool   `json:"b,string"`
    C int    `json:"c,string"`
}

func main() {
    b := []byte(`{"a":"n","b":"t","c":"100"}`)

    var x X
    if err := json.Unmarshal(b, &x); err != nil {
        fmt.Println(err)
        return
    }
    fmt.Printf("%+v\n", x)
}
  1. What did you expect to see?

like

json: invalid use of ,string struct tag, trying to unmarshal ...
  1. What did you see instead?
{A: B:true C:100}
@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 7, 2016

The error message is correct: it's an invalid use of the ,string option. After removing the quotes on "n", what's left is n, which isn't a valid JSON string.

@bradfitz bradfitz closed this Apr 7, 2016

@chanxuehong

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2016

@bradfitz

yes,

After removing the quotes on  "n" , what's left is  n , which isn't a valid JSON string.

but now encoding/json does not report the error message, and we got

{A: B:true C:100}

is it right???

@chanxuehong

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2016

@bradfitz

After removing the quotes on  "n" , what's left is  n , which isn't a valid JSON string.
After removing the quotes on  "t" , what's left is  t , which isn't a valid JSON string.

but now the result is

{A: B:true C:100}

A is "" and B is true

I think it is a bug!

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 8, 2016

Let's move this discussion to a forum. See https://golang.org/wiki/Questions

You're obviously looking something, but whatever it is, I don't think it's the ,string option.

@chanxuehong

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2016

@bradfitz

I think it's the ,string option
since it has ,string option, it should unquoteBytes(

func unquoteBytes(s []byte) (t []byte, ok bool) {
)

golang-nuts topic is:
https://groups.google.com/forum/#!topic/golang-nuts/Z3BauUVklWM

@yunyet

This comment has been minimized.

Copy link

commented Apr 8, 2016

It is definitely a bug. Why it be closed?

@transtone

This comment has been minimized.

Copy link

commented Apr 8, 2016

@bradfitz

After removing the quotes on "n", what's left is n, which isn't a valid JSON string.

so it should get a err, not a struct.

@kostya-sh

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2016

This looks like a bug.

Also decoding bool field with ",string" tag doesn't looks correct: tra-la-la is decoded to true.

http://play.golang.org/p/upVyInopWQ

package main

import (
    "encoding/json"
    "fmt"
)

type X struct {
    B bool   `json:"b,string"`
}

func main() {
    b := []byte(`{"b":"tra-la-la"}`)
    var x X
    if err := json.Unmarshal(b, &x); err != nil {
        fmt.Println(err)
        return
    }
    fmt.Printf("%+v\n", x)
}

@bradfitz bradfitz added this to the Go1.7 milestone Apr 8, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 8, 2016

@kostya-sh, thanks! I've reopened this bug and edited the original bug report's header to link to your example.

@bradfitz bradfitz reopened this Apr 8, 2016

@sergeylanzman

This comment has been minimized.

Copy link

commented Apr 8, 2016

This is not bug(maybe?)
if value started from 't' json decoder return true (example "tra-la-la")
if value started from 'f' json decoder return false(example "foo")
in code this
line in code
and
line in code

case 't', 'f': // true, false
        return c == 't'

@bradfitz If you think this is bug?

@kostya-sh

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2016

This example shows the difference in error handling while decoding fields with and without ",string" tag: http://play.golang.org/p/RatENpHcvP

There are 3 problems as far as I can see (with ",string" tag):

  • any value starting with "t" is interpreted as true for bool field
  • any value starting with "f" is interpreted as false for bool filed
  • any value starting with "n" is interpreted as null for bool, string and int fileds
@rsc

This comment has been minimized.

Copy link
Contributor

commented May 18, 2016

The single letter check is not meant to intentionally accept "terrible" as true and "factual" as false. It is simply a mistake introduced when ,string was added. Originally the JSON parser was only letting true, false, and null through to that case, so looking at the first letter was sufficient. But the ,string code started passing arbitrary strings to that code and didn't first check to see if that was appropriate. Just a bug, not intended, nothing more.

Should be easy to fix.

Thanks for the simple case @kostya-sh.

@rsc rsc changed the title encoding/json: a bug with "string" tag option encoding/json: ",string" bool field accepts "terrible" as true May 18, 2016

@gopherbot

This comment has been minimized.

Copy link

commented May 21, 2016

CL https://golang.org/cl/23315 mentions this issue.

@quentinmit quentinmit added the NeedsFix label May 26, 2016

@rsc rsc modified the milestones: Go1.8, Go1.7 May 27, 2016

@odeke-em odeke-em self-assigned this Aug 27, 2016

@gopherbot

This comment has been minimized.

Copy link

commented Oct 12, 2016

CL https://golang.org/cl/30943 mentions this issue.

@gopherbot gopherbot closed this in 0da30d5 Oct 13, 2016

@golang golang locked and limited conversation to collaborators Oct 13, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.