AlexStocks commented on code in PR #3550:
URL: https://github.com/apache/dubbo-go/pull/3550#discussion_r3699002080
##########
protocol/triple/triple_protocol/codec.go:
##########
@@ -619,6 +581,103 @@ func copySlice(inSlice, outSlice reflect.Value) error {
return nil
}
+// tripleServerCodecSession is a per-request Codec for the triple server that
+// handles both IDL and Non-IDL formats.
+//
+// SerializeType is request-scoped state that the Codec interface
+// (Marshal/Unmarshal) has no channel to surface. The session object IS that
+// channel: Unmarshal captures SerializeType from the TripleRequestWrapper in a
+// single decode, and Marshal reads it to wrap the response in a
+// TripleResponseWrapper.
+type tripleServerCodecSession struct {
+ delegate Codec // IDL path codec, resolved from
Content-Type
+ serializeType string // captured by Unmarshal when Non-IDL; read
by Marshal
+ allowedSerializeType string // provider "serialization" param;
effective allowlist = {hessian2} ∪ {this}. TODO: support Java's multi-valued
prefer-serialization
+}
+
+var _ Codec = (*tripleServerCodecSession)(nil)
+
+func (s *tripleServerCodecSession) Name() string { return s.delegate.Name() }
+
+// checkAllowed enforces the provider-side serialization allowlist.
+// hessian2 is always allowed (Non-IDL interop default); any other name must
+// match the provider's configured serialization.
+func (s *tripleServerCodecSession) checkAllowed(codecName string) error {
+ if codecName == codecNameHessian2 || codecName ==
s.allowedSerializeType {
+ return nil
+ }
+ return fmt.Errorf("serialize type %q not allowed by provider (allowed:
%s, %s)",
+ codecName, codecNameHessian2, s.allowedSerializeType)
+}
+
+func (s *tripleServerCodecSession) Unmarshal(data []byte, message any) error {
+ if _, isProto := message.(proto.Message); isProto {
+ // IDL: standard proto message.
+ return s.delegate.Unmarshal(data, message)
+ }
+ // Non-IDL: decode the TripleRequestWrapper once, capturing
SerializeType
+ // for the subsequent response Marshal and decoding the inner args in
the
+ // same pass.
+ var reqWrapper interoperability.TripleRequestWrapper
+ if err := proto.Unmarshal(data, &reqWrapper); err != nil {
+ return fmt.Errorf("unmarshal triple wrapper request: %w", err)
+ }
+ s.serializeType = reqWrapper.SerializeType
+ inner, err := resolveInnerCodec(reqWrapper.SerializeType)
+ if err != nil {
+ return fmt.Errorf("unmarshal triple wrapper request: %w", err)
+ }
+ if err := s.checkAllowed(inner.Name()); err != nil {
+ return fmt.Errorf("unmarshal triple wrapper request: %w", err)
+ }
+ return unmarshalWrapperRequestArgs(&reqWrapper, inner, message)
+}
+
+func (s *tripleServerCodecSession) Marshal(message any) ([]byte, error) {
+ if _, isProto := message.(proto.Message); isProto {
+ // IDL: standard proto message.
+ return s.delegate.Marshal(message)
+ }
+ // Non-IDL: wrap the response in a TripleResponseWrapper whose Data is
+ // serialized with the inner codec resolved from the request's
SerializeType.
+ inner, err := resolveInnerCodec(s.serializeType)
+ if err != nil {
+ return nil, fmt.Errorf("marshal triple wrapper response: %w",
err)
+ }
+ data, err := inner.Marshal(message)
Review Comment:
[P0] 非 IDL 响应把内部 `[]any` 容器写进了 wrapper
生产链路的 `wrapTripleResponse` 固定构造 `[]any{result}`，handler 再通过
`conn.Send(response.Any())` 把这个切片传到这里；当前实现却把整个 `message` 交给 inner
codec。使用相同生产形状的 WSL/Go 1.25.1 overlay 已稳定复现：MsgPack 将 array 解码到 string reply
时返回类型错误，Hessian2 在 `copySlice` 对 string 调用 `reflect.MakeSlice` 并 panic。新增
round-trip 测试直接向 session 传标量，绕过了真实形状。
请在编码前严格解包单元素 response container，并明确定义 0/多元素和 nil 返回；补从 Triple
Invoker/handler 公共入口覆盖 Hessian2、MsgPack、具体类型指针和 `*any` 的端到端测试。
##########
protocol/triple/triple_protocol/codec.go:
##########
@@ -351,7 +305,11 @@ func (c *protoWrapperCodec) Unmarshal(binary []byte,
message any) error {
if err := proto.Unmarshal(binary, &wrapperResp); err == nil {
// Check if it's a valid response wrapper (has serializeType or
non-empty data)
if len(wrapperResp.Data) > 0 {
- return c.innerCodec.Unmarshal(wrapperResp.Data, message)
+ inner, err :=
resolveInnerCodec(wrapperResp.SerializeType)
Review Comment:
[P1] 空 Data 也必须先校验 `SerializeType`
当前只在 `Data` 非空时调用 `resolveInnerCodec`；`TripleResponseWrapper{SerializeType:
"unknown", Data: nil}` 会命中后续分支直接返回成功。WSL overlay 已复现 `Unmarshal` 返回 nil，违反
Issue #3496 对 unknown/disabled type 明确报错的验收条件，调用方也无法区分真实 void 与损坏协议响应。
请在判断空 Data 前先解析并校验非空 `SerializeType`，再按已知类型处理 void/null；补
unknown、disabled、`hessian4`、空字符串以及合法 Hessian2/MsgPack 的空 Data 测试。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
