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

fixed null in number json tag string #480

Merged
merged 1 commit into from
Aug 3, 2020
Merged

fixed null in number json tag string #480

merged 1 commit into from
Aug 3, 2020

Conversation

nikolaydubina
Copy link
Contributor

@nikolaydubina nikolaydubina commented Jul 24, 2020

This fixes recently reported bug, when null is passed to number with json tag set to string. As highlighted in issue, standard library will place zero value in such case (GoPlayground). However, json-iterator previously crashed with error like this:


--- FAIL: Test_unmarshal (0.00s)
    --- FAIL: Test_unmarshal/[41]{"field":_null} (0.00s)
        require.go:794: 
            	Error Trace:	value_test.go:52
            	Error:      	Received unexpected error:
            	            	Field: stringModeNumberDecoder: expect ", but found n, error found in #10 byte of ...|"field": null}|..., bigger context ...|{"field": null}|...
            	Test:       	Test_unmarshal/[41]{"field":_null}
            	Messages:   	jsoniter

Test plan:

  • added test that fails without this modification
  • all other tests pass

@codecov
Copy link

codecov bot commented Jul 24, 2020

Codecov Report

Merging #480 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #480   +/-   ##
=======================================
  Coverage   86.32%   86.33%           
=======================================
  Files          41       41           
  Lines        5112     5115    +3     
=======================================
+ Hits         4413     4416    +3     
  Misses        558      558           
  Partials      141      141           
Impacted Files Coverage Δ
reflect_struct_decoder.go 82.75% <100.00%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bce16d...35c32cc. Read the comment docs.

@nikolaydubina
Copy link
Contributor Author

cc: @taowen @AllenX2018 @thockin

@nikolaydubina
Copy link
Contributor Author

cc: @cch123 @olegshaldybin

@AllenX2018
Copy link
Collaborator

Thanks for the pr. I'm sorry that I've been busy these days. I will check it in this week.

@AllenX2018 AllenX2018 merged commit 9461257 into json-iterator:master Aug 3, 2020
@AllenX2018
Copy link
Collaborator

Merged. Thanks for fixing this again.

zhenzou pushed a commit to zhenzou/jsoniter that referenced this pull request Feb 2, 2022
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.

2 participants