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

fix null encoding for nil pointer #368

Merged
merged 2 commits into from
Nov 27, 2023
Merged

Conversation

ahaostudy
Copy link
Contributor

What this PR does:

In the previous implementation, dubbo-go-hessian2 would encode nil values of a specified type as empty bytes.

For example:

var null *int = nil

e := NewEncoder()
e.Encode(null)

len(e.buffer) == 0  // true

However, the expected encoding for nil values should be []byte("N"), regardless of the type of nil value.

This PR addresses the above issue. It encodes nil values of any type as []byte("N").

To avoid impacting existing code, this feature is designed as an optional option.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6c40744) 69.23% compared to head (1e5ed7c) 69.32%.

❗ Current head 1e5ed7c differs from pull request most recent head ebf53db. Consider uploading reports for the commit ebf53db to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
+ Coverage   69.23%   69.32%   +0.09%     
==========================================
  Files          28       28              
  Lines        3146     3149       +3     
==========================================
+ Hits         2178     2183       +5     
+ Misses        745      744       -1     
+ Partials      223      222       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wongoo
Copy link
Contributor

wongoo commented Nov 26, 2023

@ahaostudy the checking nil is not implemented completely, I think we can fix it, and not need to add a special config for it.

I change the code as follow, and it works. You can update this PR, and then I will approve it.
screenshot-20231126-122925

@ahaostudy
Copy link
Contributor Author

ahaostudy commented Nov 26, 2023

@ahaostudy the checking nil is not implemented completely, I think we can fix it, and not need to add a special config for it.

I change the code as follow, and it works. You can update this PR, and then I will approve it. screenshot-20231126-122925

In this implementation, support for encoding nil values of types *bool and *int32 is still not available.

image

@tiltwind
Copy link
Contributor

ref #369

Copy link
Contributor

@wongoo wongoo left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexStocks AlexStocks merged commit 333dcbc into apache:master Nov 27, 2023
2 checks passed
@wongoo wongoo changed the title support java null encoding fix null encoding for nil pointer Nov 29, 2023
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.

5 participants