francoispqt / gojay

high performance JSON encoder/decoder with stream API for Golang
MIT License
2.11k stars 113 forks source link

Decoder.{{Type}}Null does not behave as documented when encountering incorrect typed json elements. #158

Open Foxcapades opened 3 years ago

Foxcapades commented 3 years ago

This was initially detected with Decoder.StringNull, but it appears to affect all Decoder.{{Type}}Null methods.

As per the StringNull method documentation, when the next value is not a string or null, the method should return an InvalidUnmarshalError, however the method returns no such error.

Version in use: github.com/francoispqt/gojay v1.2.13

Method docs:

// StringNull decodes the JSON value within an object or an array to a **string.
// If next key is not a JSON string nor null, InvalidUnmarshalError will be returned.
// If a `null` is encountered, gojay does not change the value of the pointer.

Demonstration:

package main

import (
    "bytes"
    "fmt"
    "github.com/francoispqt/gojay"
)

func main() {
    var tmp *string

    dec := gojay.NewDecoder(bytes.NewBufferString("13245"))

    // This should _not_ return nil.  It should return an InvalidUnmarshalError.
    fmt.Println(dec.StringNull(&tmp))
}

Which results with:

<nil>

It appears that the Decoder method decodeStringNull is calling skipData after encountering an invalid character (and assigning the correct error to dec.err), which just skips over the bad data and then returns nil, if skipData returns nil, and completely ignores the initial error.

Starting from https://github.com/francoispqt/gojay/blob/master/decode_string.go#L82-L88

        default:
            dec.err = dec.makeInvalidUnmarshalErr(v)
            err := dec.skipData()
            if err != nil {
                return err
            }
            return nil