francoispqt / gojay

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

10-decimal float32 parse error #112

Closed grmorozov closed 5 years ago

grmorozov commented 5 years ago

There is a bug in parsing numbers with 10 decimal digits. To reproduce, add next test case to TestDecoderFloat32

{
    name:           "10-decimal-digit-float",
    json:           "0.9833984375",
    expectedResult: 0.9833984,
},

output:

--- FAIL: TestDecoderFloat32 (0.00s)
    --- FAIL: TestDecoderFloat32/10-digit-float (0.00s)
        decode_number_float_test.go:925:
                Error Trace:    decode_number_float_test.go:925
                Error:          Expected nil, but got: "Cannot unmarshal JSON to type 'int32'"
                Test:           TestDecoderFloat32/10-decimal-digit-float
                Messages:       Err must be nil
        decode_number_float_test.go:928:
                Error Trace:    decode_number_float_test.go:928
                Error:          Not equal:
                                expected: 983398
                                actual  : 0
                Test:           TestDecoderFloat32/10-decimal-digit-float
                Messages:       v must be equal to 0.983398

First problem, it shouldn't fail. Second - error message "Cannot unmarshal JSON to type 'int32'" when parsing float32 is misleading (should it be moved to a separate issue?).

This particular error can be fixed by changing line 383 in _decode_numberfloat.go (expI >= 11 instead of expI >= 12). I wanted to submit a pull request with the fix, however there is similar logic with numbers in exponential format (line 347) which probably should be changed too. IMO, there is not enough test cases for this part of the code to make such changes with confidence.

francoispqt commented 5 years ago

Hi, thanks for raising the issue.

I'm working on it.

francoispqt commented 5 years ago

The bug has been fixed by #116

I'm closing the issue. Sorry for the delay.