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

handle decoding true/false as strings #203

Closed

Conversation

juanvallejo
Copy link

Addresses cases such as the following:

v := struct{
    Selector map[string]string `json:"selector"`
}{}
dataToDecode := `{
    "selector": {
        "field": false
    }
}`
dataToDecodeAsYAML := `
selector:
  field: no
`

where the output struct expects a map of string keys and values, and the input contains non-quoted boolean values.

Particularly in cases where the input is yaml and is later converted into json before being decoded, this patch prevents a potentially confusing error where a user might use a yaml-specific boolean keyword (such as yes or no), and then is presented with a decoder error similar to [pos 36]: json: expect char '"' but got char 'f'

Related downstream bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1458204#c1

cc @fabianofranz

@ugorji
Copy link
Owner

ugorji commented Aug 25, 2017

I need a reproducer that I can run. Those are much better than reading prose and understanding what you are trying to say.

@juanvallejo
Copy link
Author

@ugorji thanks, the program below replicates the specific case we are running into:

package main

import (
	"fmt"

	"github.com/ugorji/go/codec"
)

var jsonData = `{
    "selector":  {
        "key1": false
    }
}`

type SampleSchema struct {
	Selector map[string]string `json:"selector"`
}

func main() {
	obj := &SampleSchema{
		Selector: make(map[string]string),
	}

	err := codec.NewDecoderBytes([]byte(jsonData), new(codec.JsonHandle)).Decode(obj)
	if err != nil {
		// attempt to reproduce error:
		// error: [pos 38]: json: expect char '"' but got char 'f'
		fmt.Printf("error: %v\n", err)
	}
}

@ugorji
Copy link
Owner

ugorji commented Aug 25, 2017

Got it. Let me think about it over the weekend. Can you update the original bug request with this reproducer, so I can focus on that and respond to that? I will then re-open it back up.

@ugorji
Copy link
Owner

ugorji commented Aug 26, 2017

Fix incorporated upstream.

@ugorji ugorji closed this Aug 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants