(iceberg-go) branch main updated: fix: validate format-version in metadata constructors (#1387)

[at...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

laskoviymishka 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 d93d43b9 fix: validate format-version in metadata constructors (#1387)
d93d43b9 is described below

commit d93d43b9dfa94d46b18001b5c6fe0bf851d69137
Author: Minh Vu <vu...@gmail.com>
AuthorDate: Thu Jul 9 19:02:44 2026 +0200

    fix: validate format-version in metadata constructors (#1387)
    
    ## Why
    
    Table and view metadata creation accepted invalid `format-version`
    values silently. A bad property like `format-version: banana` currently
    fell back to the default version and also mutated the caller's props
    map, which can hide user mistakes and corrupt input ownership.
    
    ## What changed
    
    - table metadata (`table/metadata.go`):
    - In `NewMetadataWithUUID`, `format-version` is now parsed with strict
    `Atoi` validation.
    - Invalid values now return `iceberg.ErrInvalidFormatVersion` instead of
    silently defaulting.
    - `format-version` is removed from a **cloned** props map before passing
    into metadata builder, so caller-provided maps are not mutated.
    
    - view metadata (`view/metadata.go`):
      - Mirrored the same strict validation in view metadata creation.
      - Invalid values now return `iceberg.ErrInvalidFormatVersion`.
      - Uses a cloned props map to avoid external mutation.
    
    ## Testing
    
    - `go test ./table -run
    'TestNewMetadataRejectInvalidFormatVersion|TestNewMetadataWithExplicitV1Format|TestNewMetadataV2Format'
    -count=1`
    - `go test ./view -run
    'TestNewMetadataRejectInvalidFormatVersion|TestNewMetadata|TestUnmarshalViewMetadata'
    -count=1`
---
 table/metadata.go               | 10 ++++++----
 table/metadata_internal_test.go | 16 ++++++++++++++++
 view/metadata.go                | 14 ++++++++++----
 view/metadata_test.go           | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/table/metadata.go b/table/metadata.go
index 9cf7481a..81b44b04 100644
--- a/table/metadata.go
+++ b/table/metadata.go
@@ -2205,15 +2205,17 @@ func NewMetadataWithUUID(sc *iceberg.Schema, partitions *iceberg.PartitionSpec,
 	if tableUuid == uuid.Nil {
 		tableUuid = uuid.New()
 	}
+	var inputProps iceberg.Properties
 	var err error
 	formatVersion := DefaultFormatVersion
 	if props != nil {
-		verStr, ok := props[PropertyFormatVersion]
+		inputProps = maps.Clone(props)
+		verStr, ok := inputProps[PropertyFormatVersion]
 		if ok {
 			if formatVersion, err = strconv.Atoi(verStr); err != nil {
-				formatVersion = DefaultFormatVersion
+				return nil, fmt.Errorf("%w: %s", iceberg.ErrInvalidFormatVersion, verStr)
 			}
-			delete(props, PropertyFormatVersion)
+			delete(inputProps, PropertyFormatVersion)
 		}
 	}
 
@@ -2259,7 +2261,7 @@ func NewMetadataWithUUID(sc *iceberg.Schema, partitions *iceberg.PartitionSpec,
 		return nil, err
 	}
 
-	if err = builder.SetProperties(props); err != nil {
+	if err = builder.SetProperties(inputProps); err != nil {
 		return nil, err
 	}
 
diff --git a/table/metadata_internal_test.go b/table/metadata_internal_test.go
index b890201c..1a36d612 100644
--- a/table/metadata_internal_test.go
+++ b/table/metadata_internal_test.go
@@ -780,6 +780,22 @@ func TestNewMetadataWithExplicitV1Format(t *testing.T) {
 	assert.Truef(t, expected.Equals(actual), "expected: %s\ngot: %s", expected, actual)
 }
 
+func TestNewMetadataRejectInvalidFormatVersion(t *testing.T) {
+	props := iceberg.Properties{
+		"format-version": "banana",
+		"foo":            "bar",
+	}
+	schema := iceberg.NewSchema(1, iceberg.NestedField{ID: 1, Name: "x", Type: iceberg.PrimitiveTypes.Int64})
+
+	metadata, err := NewMetadata(schema, iceberg.UnpartitionedSpec, UnsortedSortOrder, "s3://bucket/path", props)
+	require.Error(t, err)
+	require.ErrorIs(t, err, iceberg.ErrInvalidFormatVersion)
+	assert.Nil(t, metadata)
+	assert.Equal(t, "banana", props["format-version"])
+	assert.Equal(t, "bar", props["foo"])
+	assert.Len(t, props, 2)
+}
+
 func TestNewMetadataV2Format(t *testing.T) {
 	schema := iceberg.NewSchemaWithIdentifiers(10,
 		[]int{22},
diff --git a/view/metadata.go b/view/metadata.go
index 06590c71..188a4186 100644
--- a/view/metadata.go
+++ b/view/metadata.go
@@ -518,18 +518,24 @@ func NewMetadataWithUUID(version *Version, sc *iceberg.Schema, location string,
 		viewUUID = uuid.New()
 	}
 
+	var inputProps iceberg.Properties
 	formatVersion := DefaultViewFormatVersion
 	if props != nil {
-		verStr, ok := props["format-version"]
+		inputProps = maps.Clone(props)
+		verStr, ok := inputProps[table.PropertyFormatVersion]
 		if ok {
 			var err error
 			if formatVersion, err = strconv.Atoi(verStr); err != nil {
-				formatVersion = DefaultViewFormatVersion
+				return nil, fmt.Errorf("%w: %s", iceberg.ErrInvalidFormatVersion, verStr)
 			}
-			delete(props, "format-version")
+			delete(inputProps, table.PropertyFormatVersion)
 		}
 	}
 
+	if formatVersion > SupportedViewFormatVersion {
+		return nil, fmt.Errorf("%w: %d", iceberg.ErrInvalidFormatVersion, formatVersion)
+	}
+
 	// We assume that this constructor is used for building metadata for a new view.
 	// Thus, we enforce that the VersionID is the initial one defined in the spec.
 	if version.VersionID != InitialVersionID {
@@ -547,7 +553,7 @@ func NewMetadataWithUUID(version *Version, sc *iceberg.Schema, location string,
 		SetFormatVersion(formatVersion).
 		SetUUID(viewUUID).
 		SetLoc(location).
-		SetProperties(props).
+		SetProperties(inputProps).
 		SetCurrentVersion(version, sc).
 		Build()
 }
diff --git a/view/metadata_test.go b/view/metadata_test.go
index 7561ac74..c85f9114 100644
--- a/view/metadata_test.go
+++ b/view/metadata_test.go
@@ -25,6 +25,7 @@ import (
 	"testing"
 
 	"github.com/apache/iceberg-go"
+	"github.com/apache/iceberg-go/table"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -88,6 +89,43 @@ func TestNewMetadata(t *testing.T) {
 	assert.Equal(t, []VersionLogEntry{{TimestampMS: 1000, VersionID: 1}}, md.VersionLog())
 }
 
+func TestNewMetadataRejectInvalidFormatVersion(t *testing.T) {
+	tests := []struct {
+		name      string
+		formatVer string
+	}{
+		{
+			name:      "non-numeric format-version",
+			formatVer: "banana",
+		},
+		{
+			name:      "unsupported format-version",
+			formatVer: "2",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			version := newTestVersion(1, LastAddedID,
+				WithVersionSummary(VersionSummary{"summary-key": "summary-val"}),
+				WithTimestampMS(1000))
+			schema := newTestSchema(0)
+			props := iceberg.Properties{
+				table.PropertyFormatVersion: tc.formatVer,
+				"foo":                       "bar",
+			}
+
+			md, err := NewMetadata(version, schema, "location", props)
+			require.Error(t, err)
+			require.ErrorIs(t, err, iceberg.ErrInvalidFormatVersion)
+			require.Nil(t, md)
+			assert.Equal(t, tc.formatVer, props[table.PropertyFormatVersion])
+			assert.Equal(t, "bar", props["foo"])
+			assert.Len(t, props, 2)
+		})
+	}
+}
+
 func TestUnmarshalViewMetadata(t *testing.T) {
 	md, err := ParseMetadataString(exampleViewJSON)
 	require.NoError(t, err)