handle typed nil pointers in Encode method (apache/dubbo-go#2517)#376
Open
Nexusrex18 wants to merge 1 commit intoapache:masterfrom
Open
handle typed nil pointers in Encode method (apache/dubbo-go#2517)#376Nexusrex18 wants to merge 1 commit intoapache:masterfrom
Nexusrex18 wants to merge 1 commit intoapache:masterfrom
Conversation
tiltwind
reviewed
Apr 1, 2025
| cmd := exec.Command("java", cmdArgs...) | ||
| out, err := cmd.Output() | ||
| if err != nil { | ||
| if exitErr, ok := err.(*exec.ExitError); ok { |
Contributor
There was a problem hiding this comment.
why ignore the the exec.ExitError?
tiltwind
reviewed
Apr 1, 2025
|
|
||
| func TestEncodeTypedNilPointer(t *testing.T) { | ||
| encoder := NewEncoder() | ||
| var val *int32 = nil |
Contributor
There was a problem hiding this comment.
we define *int32 as java.lang.Integer,
dubbo-go-hessian2/java_lang_test.go
Line 60 in e93635a
1 task
Contributor
AlexStocks
reviewed
Mar 29, 2026
Contributor
AlexStocks
left a comment
There was a problem hiding this comment.
Code Review: handle typed nil pointers in Encode method (apache/dubbo-go#2517)
评价
问题定位准确,修复方案正确。解决了 (*int32)(nil) 这类 typed nil pointer 无法正确编码为 Hessian null 的问题。
[P1] 规范级问题
1. encode.go:102-109 - 性能开销
使用 reflect.ValueOf(v).Kind() 和 IsNil() 会增加性能开销。对于高频调用的 Encode 方法,建议使用类型断言优化:
// 优化方案:优先使用类型断言
switch v.(type) {
case nil:
e.buffer = EncNull(e.buffer)
return nil
default:
vVal := reflect.ValueOf(v)
if vVal.Kind() == reflect.Ptr && vVal.IsNil() {
e.buffer = EncNull(e.buffer)
return nil
}
}
// 继续原有类型判断...2. 与 PR #384 的关系
本 PR 与 #384 解决同一问题且修改相同文件,建议:
- 合并两个 PR
- 保留更完整的测试覆盖
3. 测试覆盖
encode_test.go 的测试覆盖了主要类型,但建议补充:
- 嵌套指针类型(
**int32) - 接口类型的 nil 判断
- 结构体指针的 nil 判断
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the issue where the Encode method in encode.go did not properly handle typed nil pointers (e.g., (*int32)(nil)). The changes ensure that such pointers are encoded as Hessian NULL ('N').
Changes:
Fixes apache/dubbo-go#2517 (Ref: apache/dubbo-go#2517)
This PR is a prerequisite for the corresponding changes in apache/dubbo-go.