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)
+}

Reply via email to