This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new 839de5b2 fix: nil field Type causes unrecovered panic in MarshalJSON
(#765)
839de5b2 is described below
commit 839de5b238664fe25306d5005c4c0b32ffb867c7
Author: Shubhendu <[email protected]>
AuthorDate: Tue Mar 3 03:23:17 2026 +0530
fix: nil field Type causes unrecovered panic in MarshalJSON (#765)
### Problem
When a NestedField has a nil Type — which happens when the JSON payload
uses the wrong key (e.g. "field_type" instead of "type") or omits the
"type" key entirely — two failure modes exist:
1. NestedField.UnmarshalJSON silently accepts nil types.
Go's encoding/json leaves fields at their zero value when a key is
absent or unrecognised. Since typeIFace wraps a Type interface, the zero
value is nil. UnmarshalJSON already validates id but applies no
equivalent guard to type, so a payload with a missing or misspelled type
key produces a NestedField with Type == nil and returns no error.
2. typeIFace.MarshalJSON panics on nil type.
```
// before
func (t *typeIFace) MarshalJSON() ([]byte, error) {
if nested, ok := t.Type.(NestedType); ok {
return json.Marshal(nested)
}
return []byte(`"` + t.Type.Type() + `"`), nil // nil pointer
dereference if t.Type == nil
}
```
When a schema containing a nil-type field is later marshalled (e.g.
during CreateTable, CommitTable, or any serialisation path),
t.Type.Type() is a method call on a nil interface — an unrecovered
panic. There is no defer recover() in the json.Marshal call path, so the
process crashes.
```
Example triggering input:
{
"type": "struct",
"fields": [
{"id": 1, "name": "col", "field_type": "long", "required": true}
]
}
```
Before this fix: json.Unmarshal succeeds silently, subsequent
json.Marshal panics. After this fix: json.Unmarshal returns
ErrInvalidSchema immediately.
### Changes
* types.go
NestedField.UnmarshalJSON — add a nil-type guard after decoding,
consistent with the existing id and name guards introduced in PRs #758
and #761: if n.Type == nil {
return fmt.Errorf("%w: field %q is missing required 'type' key in JSON",
ErrInvalidSchema, n.Name)
}
typeIFace.MarshalJSON — add a defensive nil guard so that a NestedField
constructed programmatically with Type == nil returns an error instead
of panicking: if t.Type == nil {
return nil, fmt.Errorf("%w: field type is nil; use the \"type\" JSON key
with a valid Iceberg type such as \"long\", \"string\", or \"boolean\"",
ErrInvalidSchema)
}
* types_test.go — three new tests:
** TestNestedFieldUnmarshalMissingType: absent "type" key →
ErrInvalidSchema
** TestNestedFieldUnmarshalWrongTypeKey: "field_type" instead of "type"
→ ErrInvalidSchema
** TestTypeIFaceMarshalJSONNilType: programmatically constructed
NestedField{Type: nil} → json.Marshal returns ErrInvalidSchema, no panic
### Behaviour after this change
| Scenario | Before | After |
|-----------------------------------------------|-----------------------------------|-------------------------------|
| "type" key absent from JSON | Silent nil type, panic on marshal |
ErrInvalidSchema at unmarshal |
| Wrong key ("field_type") used | Silent nil type, panic on marshal |
ErrInvalidSchema at unmarshal |
| NestedField with nil type marshalled directly | Unrecovered panic |
ErrInvalidSchema returned |
| Valid field with correct "type" key | OK | OK (unchanged) |
Signed-off-by: Shubhendu Ram Tripathi <[email protected]>
---
types.go | 7 +++++++
types_test.go | 31 +++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/types.go b/types.go
index dc8beb2f..5b184ca7 100644
--- a/types.go
+++ b/types.go
@@ -91,6 +91,9 @@ type typeIFace struct {
}
func (t *typeIFace) MarshalJSON() ([]byte, error) {
+ if t.Type == nil {
+ return nil, fmt.Errorf("%w: field type is nil; use the \"type\"
JSON key with a valid Iceberg type such as \"long\", \"string\", or
\"boolean\"", ErrInvalidSchema)
+ }
if nested, ok := t.Type.(NestedType); ok {
return json.Marshal(nested)
}
@@ -254,6 +257,10 @@ func (n *NestedField) UnmarshalJSON(b []byte) error {
return fmt.Errorf("%w: field is missing required 'name' key in
JSON", ErrInvalidSchema)
}
+ if n.Type == nil {
+ return fmt.Errorf("%w: field %q is missing required 'type' key
in JSON", ErrInvalidSchema, n.Name)
+ }
+
return nil
}
diff --git a/types_test.go b/types_test.go
index cc43f61b..02849bca 100644
--- a/types_test.go
+++ b/types_test.go
@@ -434,3 +434,34 @@ func TestNestedFieldUnmarshalZeroIDIsValid(t *testing.T) {
assert.Equal(t, 0, f.ID)
assert.Equal(t, "col", f.Name)
}
+
+func TestNestedFieldUnmarshalMissingType(t *testing.T) {
+ // "type" key is absent — json.Unmarshal should return ErrInvalidSchema
+ // instead of leaving field.Type nil, which would panic in MarshalJSON.
+ data := []byte(`{"id":1,"name":"col","required":false}`)
+
+ var f iceberg.NestedField
+ err := json.Unmarshal(data, &f)
+ assert.ErrorIs(t, err, iceberg.ErrInvalidSchema)
+ assert.ErrorContains(t, err, "missing required 'type'")
+}
+
+func TestNestedFieldUnmarshalWrongTypeKey(t *testing.T) {
+ // "field_type" instead of "type" — same nil-type outcome, same error.
+ data :=
[]byte(`{"id":1,"name":"col","field_type":"long","required":false}`)
+
+ var f iceberg.NestedField
+ err := json.Unmarshal(data, &f)
+ assert.ErrorIs(t, err, iceberg.ErrInvalidSchema)
+ assert.ErrorContains(t, err, "missing required 'type'")
+}
+
+func TestTypeIFaceMarshalJSONNilType(t *testing.T) {
+ // A NestedField with a nil Type must not panic during JSON marshalling.
+ // Prior to this fix, typeIFace.MarshalJSON called t.Type.Type() on a
nil
+ // interface, causing an unrecovered panic.
+ schema := iceberg.NewSchema(0, iceberg.NestedField{ID: 1, Name: "col",
Type: nil})
+ _, err := json.Marshal(schema)
+ require.Error(t, err)
+ assert.ErrorIs(t, err, iceberg.ErrInvalidSchema)
+}