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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't marshal empty byte or uint8 slice as null #371

Merged
merged 1 commit into from
Jun 3, 2019

Conversation

nikhita
Copy link
Contributor

@nikhita nikhita commented Jun 2, 2019

Fixes #272

[]byte or []uint8 are encoded as base-64 encoded strings. So, only nil slices of these types should get marshalled as null, and non-nil empty slices should get marshalled as "".

This restores compatibility with the standard library.

I verified that this patch works locally. Even though tests for these exist in https://github.com/json-iterator/go/blob/master/type_tests/slice_test.go, I couldn't reproduce a case where the test case was fuzzed to an empty []byte or []uint8 slice... 馃槙

/cc @taowen @thockin
fyi @liggitt @dims @neolit123

@nikhita
Copy link
Contributor Author

nikhita commented Jun 2, 2019

cc @sttts

@dims
Copy link

dims commented Jun 3, 2019

@nikhita i was playing with this snippet - https://play.golang.org/p/IOXp4klmNvU

before applying your patch, both "case 1" and "case 2" end up printing marshal : "null"
after applying your patch, "case 1" prints marshal : "null" and "case 2" prints marshal : "\"\""

does this help?

reflect_native.go Outdated Show resolved Hide resolved
@nikhita
Copy link
Contributor Author

nikhita commented Jun 3, 2019

Updated with tests. Turned out that there are special tests (apart from the normal fuzzing of types) for encoding byte arrays.

@codecov
Copy link

codecov bot commented Jun 3, 2019

Codecov Report

Merging #371 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #371      +/-   ##
==========================================
+ Coverage   81.69%   81.81%   +0.12%     
==========================================
  Files          41       41              
  Lines        5020     5021       +1     
==========================================
+ Hits         4101     4108       +7     
+ Misses        798      792       -6     
  Partials      121      121
Impacted Files Coverage 螖
reflect_native.go 88.16% <100%> (+0.04%) 猬嗭笍
reflect_struct_decoder.go 49.16% <0%> (+0.83%) 猬嗭笍

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 08047c1...fb5614a. Read the comment docs.

@dims
Copy link

dims commented Jun 3, 2019

@nikhita do we have to add the test for the var mySlice1 []byte // Case 1? where it's marshalled to null?

[]byte or []uint8 are encoded as base-64 encoded string. Per this, non-nil
empty slice should not get marshalled as null, rather as "".

This restores compatibility with the standard library.
@nikhita
Copy link
Contributor Author

nikhita commented Jun 3, 2019

@dims Done.

Copy link

@dims dims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @nikhita

@thockin
Copy link
Collaborator

thockin commented Jun 3, 2019

LGTM. I try to be judicious in my authority on this repo, but this seems like an important edge case.

@thockin thockin merged commit 0039f4a into json-iterator:master Jun 3, 2019
@nikhita nikhita deleted the byte-base64-encode branch June 3, 2019 17:06
@nikhita nikhita mentioned this pull request Jun 3, 2019
zhenzou pushed a commit to zhenzou/jsoniter that referenced this pull request Feb 2, 2022
Don't marshal empty byte or uint8 slice as null
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.

[]byte marshaled to null
4 participants