This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new ddf066214 fix(go): return nil serializer on getTypeInfo err (#3719)
ddf066214 is described below
commit ddf06621410674532440a87b278e726ad3bdc492
Author: Ayush Kumar <[email protected]>
AuthorDate: Sat May 30 17:50:46 2026 +0530
fix(go): return nil serializer on getTypeInfo err (#3719)
## Why?
## What does this PR do?
## Related issues
Closes #3718
## AI Contribution Checklist
Issue was flagged by claude
```
Confirmed bug
1. type_resolver.go:1018 — serializer-creation error is silently swallowed
Confidence: high (confirmed by go vet and by reading the code). Severity:
medium–high.
serializer, err := r.createSerializer(value.Type(), false)
if err != nil {
fmt.Errorf("failed to create serializer: %w", err) // <-- result
discarded
}
info.Serializer = serializer // serializer is nil here on error
// ...
r.typePointerCache[typePtr] = info // nil serializer gets cached
return info, nil // returns nil error despite failure
The error is built with fmt.Errorf but never returned or assigned. On the
lazy-init path, when createSerializer fails, serializer is nil, that nil is
stored into info.Serializer, cached, and the function returns (info, nil) —
pretending success. A later serialize/deserialize then uses a nil serializer →
wrong behavior or a nil-deref panic, far from the real cause.
Fix: return nil, fmt.Errorf("failed to create serializer: %w", err)
go vet flags this directly:
type_resolver.go:1018:5: result of fmt.Errorf call not used
```
## Does this PR introduce any user-facing change?
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
---
go/fory/type_resolver.go | 2 +-
go/fory/type_resolver_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/go/fory/type_resolver.go b/go/fory/type_resolver.go
index 82bf185f8..cc9d01765 100644
--- a/go/fory/type_resolver.go
+++ b/go/fory/type_resolver.go
@@ -1015,7 +1015,7 @@ func (r *TypeResolver) getTypeInfo(value reflect.Value,
create bool) (*TypeInfo,
*/
serializer, err := r.createSerializer(value.Type(),
false)
if err != nil {
- fmt.Errorf("failed to create serializer: %w",
err)
+ return nil, fmt.Errorf("failed to create
serializer: %w", err)
}
info.Serializer = serializer
}
diff --git a/go/fory/type_resolver_test.go b/go/fory/type_resolver_test.go
new file mode 100644
index 000000000..e911b21e2
--- /dev/null
+++ b/go/fory/type_resolver_test.go
@@ -0,0 +1,64 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "reflect"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+// Regression test for the lazy-serializer-init path in getTypeInfo.
+//
+// Before the fix, a createSerializer failure on this path silently discarded
+// the error (fmt.Errorf result unused), stored a nil Serializer in the cache,
+// and returned (info, nil) — hiding the failure from callers.
+//
+// After the fix the error is propagated and no nil serializer is cached.
+func TestGetTypeInfo_LazyInitPropagatesError(t *testing.T) {
+ f := New()
+ resolver := f.typeResolver
+
+ // **int is explicitly rejected by createSerializer ("pointer to
pointer is not
+ // supported"), so it is a reliable trigger for the error path.
+ ptrPtrInt := reflect.TypeOf((**int)(nil)) // **int
+
+ // Seed typesInfo with an entry whose Serializer is nil, mimicking a
type
+ // that was registered but whose serializer has not been created yet.
+ resolver.typesInfo[ptrPtrInt] = &TypeInfo{
+ Type: ptrPtrInt,
+ Serializer: nil, // nil triggers the lazy-init branch
+ }
+
+ value := reflect.New(ptrPtrInt).Elem() // zero Value of type **int
+ info, err := resolver.getTypeInfo(value, true)
+
+ // The bug: before the fix, err was nil and info had a nil Serializer.
+ // The fix: err must be non-nil; info should be nil.
+ require.Error(t, err, "expected error when createSerializer fails on
lazy init, got nil")
+ require.Nil(t, info, "expected nil TypeInfo when createSerializer
fails, got non-nil")
+
+ // Also verify the cache was NOT poisoned with a nil-serializer entry.
+ typePtr := typePointer(ptrPtrInt)
+ cached, ok := resolver.typePointerCache[typePtr]
+ if ok {
+ require.NotNil(t, cached.Serializer,
+ "typePointerCache must not hold an entry with a nil
Serializer")
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
