From c84a31a02145650981b198c807220a3dab21c1b9 Mon Sep 17 00:00:00 2001 From: Alessandro Nori Date: Tue, 29 Sep 2026 10:42:21 +0200 Subject: [PATCH 1/3] Encode table names in REST paths --- catalog/rest/metrics_reporter_test.go | 6 +++--- catalog/rest/rest.go | 23 +++++++++++++++-------- catalog/rest/rest_internal_test.go | 9 +++++++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/catalog/rest/metrics_reporter_test.go b/catalog/rest/metrics_reporter_test.go index b901e7459..f9eb6dfb1 100644 --- a/catalog/rest/metrics_reporter_test.go +++ b/catalog/rest/metrics_reporter_test.go @@ -573,9 +573,9 @@ func TestRESTMetricsReporterBuildsPathThroughProduction(t *testing.T) { select { case req := <-received: - // The namespace levels are percent-encoded and joined by the encoded - // separator (%1F); the table segment is escaped by the URL builder. - assert.Equal(t, "/v1/my-prefix/namespaces/a%20b%1Fd%20e/tables/t%20x/metrics", req.escapedPath) + // Namespace levels use path encoding and the encoded separator (%1F); + // the table name uses form-style URL encoding. + assert.Equal(t, "/v1/my-prefix/namespaces/a%20b%1Fd%20e/tables/t+x/metrics", req.escapedPath) case <-time.After(3 * time.Second): t.Fatal("timed out waiting for the async metrics POST") } diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 2f5af16df..39de8adfb 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -1421,9 +1421,16 @@ func (r *Catalog) nsSeparator() string { return r.namespaceSeparator } +func encodeString(value string) string { + encoded := url.QueryEscape(value) + encoded = strings.ReplaceAll(encoded, "%2A", "*") + + return strings.ReplaceAll(encoded, "~", "%7E") +} + // encodeNamespace URL-encodes each namespace level and joins them with the // server-advertised, URL-encoded namespace separator for use as a REST path -// segment. Mirrors RESTUtil.encodeNamespace in the Java implementation. +// segment. func (r *Catalog) encodeNamespace(namespace table.Identifier) string { encoded := make([]string, len(namespace)) for i, level := range namespace { @@ -1454,7 +1461,7 @@ func (r *Catalog) splitIdentForPath(ident table.Identifier) (string, string, err return "", "", err } - return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), catalog.ObjectNameFromIdent(ident), nil + return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), encodeString(catalog.ObjectNameFromIdent(ident)), nil } func (r *Catalog) splitViewIdentForPath(ident table.Identifier) (string, string, error) { @@ -1478,7 +1485,7 @@ func (r *Catalog) CreateTable(ctx context.Context, identifier table.Identifier, return nil, err } - ns, tbl, err := r.splitIdentForPath(identifier) + ns, _, err := r.splitIdentForPath(identifier) if err != nil { return nil, err } @@ -1505,7 +1512,7 @@ func (r *Catalog) CreateTable(ctx context.Context, identifier table.Identifier, stagedCreate := len(cfg.StagedUpdates) > 0 payload := createTableRequest{ - Name: tbl, + Name: catalog.ObjectNameFromIdent(identifier), Schema: schema, Location: cfg.Location, PartitionSpec: cfg.PartitionSpec, @@ -1607,7 +1614,7 @@ func (r *Catalog) CommitTable(ctx context.Context, ident table.Identifier, requi restIdentifier := identifier{ Namespace: catalog.NamespaceFromIdent(ident), - Name: tblName, + Name: catalog.ObjectNameFromIdent(ident), } type payload struct { @@ -1730,7 +1737,7 @@ func (r *Catalog) RegisterTable(ctx context.Context, identifier table.Identifier return nil, err } - ns, tbl, err := r.splitIdentForPath(identifier) + ns, _, err := r.splitIdentForPath(identifier) if err != nil { return nil, err } @@ -1755,7 +1762,7 @@ func (r *Catalog) RegisterTable(ctx context.Context, identifier table.Identifier } ret, err := doPost[payload, loadTableResponse](ctx, r.baseURI, path, - payload{Name: tbl, MetadataLoc: metadataLoc}, r.cl, map[int]error{ + payload{Name: catalog.ObjectNameFromIdent(identifier), MetadataLoc: metadataLoc}, r.cl, map[int]error{ http.StatusNotFound: catalog.ErrNoSuchNamespace, http.StatusConflict: catalog.ErrTableAlreadyExists, }) if err != nil { @@ -1829,7 +1836,7 @@ func (r *Catalog) UpdateTable(ctx context.Context, ident table.Identifier, requi restIdentifier := identifier{ Namespace: catalog.NamespaceFromIdent(ident), - Name: tbl, + Name: catalog.ObjectNameFromIdent(ident), } type payload struct { Identifier identifier `json:"identifier"` diff --git a/catalog/rest/rest_internal_test.go b/catalog/rest/rest_internal_test.go index 880fa1e65..1ec63ed8f 100644 --- a/catalog/rest/rest_internal_test.go +++ b/catalog/rest/rest_internal_test.go @@ -81,6 +81,15 @@ func TestSplitIdentForPathRequiresNamespaceAndName(t *testing.T) { require.NoError(t, err) assert.Equal(t, "parent%1Fnamespace", ns) assert.Equal(t, "table", tbl) + + ns, tbl, err = cat.splitIdentForPath(table.Identifier{"namespace+name", "table+name"}) + require.NoError(t, err) + assert.Equal(t, "namespace+name", ns) + assert.Equal(t, "table%2Bname", tbl) +} + +func TestEncodeString(t *testing.T) { + assert.Equal(t, "+%25%26%2B%C2%A3%E2%82%AC", encodeString(" %&+£€")) } func TestLoadRegisteredCatalogRejectsInvalidAuthURL(t *testing.T) { From 141d7b255c2d862f35e8e74bcb14962ade1d2ac3 Mon Sep 17 00:00:00 2001 From: Alessandro Nori Date: Thu, 1 Oct 2026 10:57:15 +0200 Subject: [PATCH 2/3] Fix REST path segment encoding --- catalog/rest/metrics_reporter_test.go | 6 ++--- catalog/rest/rest.go | 26 ++++++++++---------- catalog/rest/rest_internal_test.go | 34 ++++++++++++++++++++++++--- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/catalog/rest/metrics_reporter_test.go b/catalog/rest/metrics_reporter_test.go index f9eb6dfb1..7485135b7 100644 --- a/catalog/rest/metrics_reporter_test.go +++ b/catalog/rest/metrics_reporter_test.go @@ -573,9 +573,9 @@ func TestRESTMetricsReporterBuildsPathThroughProduction(t *testing.T) { select { case req := <-received: - // Namespace levels use path encoding and the encoded separator (%1F); - // the table name uses form-style URL encoding. - assert.Equal(t, "/v1/my-prefix/namespaces/a%20b%1Fd%20e/tables/t+x/metrics", req.escapedPath) + // Namespace levels and the table name use path encoding; namespace + // levels are joined by the encoded separator (%1F). + assert.Equal(t, "/v1/my-prefix/namespaces/a%20b%1Fd%20e/tables/t%20x/metrics", req.escapedPath) case <-time.After(3 * time.Second): t.Fatal("timed out waiting for the async metrics POST") } diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 39de8adfb..633b2e41c 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -1421,11 +1421,11 @@ func (r *Catalog) nsSeparator() string { return r.namespaceSeparator } -func encodeString(value string) string { - encoded := url.QueryEscape(value) - encoded = strings.ReplaceAll(encoded, "%2A", "*") - - return strings.ReplaceAll(encoded, "~", "%7E") +// encodePathSegment escapes a REST path segment per RFC 3986. PathEscape +// leaves plus signs literal, so encode them explicitly to avoid form decoders +// interpreting them as spaces. +func encodePathSegment(value string) string { + return strings.ReplaceAll(url.PathEscape(value), "+", "%2B") } // encodeNamespace URL-encodes each namespace level and joins them with the @@ -1434,7 +1434,7 @@ func encodeString(value string) string { func (r *Catalog) encodeNamespace(namespace table.Identifier) string { encoded := make([]string, len(namespace)) for i, level := range namespace { - encoded[i] = url.PathEscape(level) + encoded[i] = encodePathSegment(level) } return strings.Join(encoded, r.nsSeparator()) @@ -1461,7 +1461,7 @@ func (r *Catalog) splitIdentForPath(ident table.Identifier) (string, string, err return "", "", err } - return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), encodeString(catalog.ObjectNameFromIdent(ident)), nil + return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), encodePathSegment(catalog.ObjectNameFromIdent(ident)), nil } func (r *Catalog) splitViewIdentForPath(ident table.Identifier) (string, string, error) { @@ -1469,7 +1469,7 @@ func (r *Catalog) splitViewIdentForPath(ident table.Identifier) (string, string, return "", "", err } - return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), catalog.ObjectNameFromIdent(ident), nil + return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), encodePathSegment(catalog.ObjectNameFromIdent(ident)), nil } func (r *Catalog) splitFunctionIdentForPath(ident table.Identifier) (string, string, error) { @@ -1477,7 +1477,7 @@ func (r *Catalog) splitFunctionIdentForPath(ident table.Identifier) (string, str return "", "", err } - return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), catalog.ObjectNameFromIdent(ident), nil + return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), encodePathSegment(catalog.ObjectNameFromIdent(ident)), nil } func (r *Catalog) CreateTable(ctx context.Context, identifier table.Identifier, schema *iceberg.Schema, opts ...catalog.CreateTableOpt) (*table.Table, error) { @@ -1607,7 +1607,7 @@ func (r *Catalog) CommitTable(ctx context.Context, ident table.Identifier, requi return nil, "", err } - ns, tblName, err := r.splitIdentForPath(ident) + ns, encodedTbl, err := r.splitIdentForPath(ident) if err != nil { return nil, "", err } @@ -1623,7 +1623,7 @@ func (r *Catalog) CommitTable(ctx context.Context, ident table.Identifier, requi Updates []table.Update `json:"updates"` } - path, err := endpointUpdateTable.reqPath(ns, tblName) + path, err := endpointUpdateTable.reqPath(ns, encodedTbl) if err != nil { return nil, "", err } @@ -1829,7 +1829,7 @@ func (r *Catalog) UpdateTable(ctx context.Context, ident table.Identifier, requi return nil, err } - ns, tbl, err := r.splitIdentForPath(ident) + ns, encodedTbl, err := r.splitIdentForPath(ident) if err != nil { return nil, err } @@ -1843,7 +1843,7 @@ func (r *Catalog) UpdateTable(ctx context.Context, ident table.Identifier, requi Requirements []table.Requirement `json:"requirements"` Updates []table.Update `json:"updates"` } - path, err := endpointUpdateTable.reqPath(ns, tbl) + path, err := endpointUpdateTable.reqPath(ns, encodedTbl) if err != nil { return nil, err } diff --git a/catalog/rest/rest_internal_test.go b/catalog/rest/rest_internal_test.go index 1ec63ed8f..7b4534270 100644 --- a/catalog/rest/rest_internal_test.go +++ b/catalog/rest/rest_internal_test.go @@ -84,12 +84,40 @@ func TestSplitIdentForPathRequiresNamespaceAndName(t *testing.T) { ns, tbl, err = cat.splitIdentForPath(table.Identifier{"namespace+name", "table+name"}) require.NoError(t, err) - assert.Equal(t, "namespace+name", ns) + assert.Equal(t, "namespace%2Bname", ns) assert.Equal(t, "table%2Bname", tbl) + + for name, split := range map[string]func(table.Identifier) (string, string, error){ + "view": cat.splitViewIdentForPath, + "function": cat.splitFunctionIdentForPath, + } { + t.Run(name, func(t *testing.T) { + ns, object, err := split(table.Identifier{"namespace+name", name + "+name"}) + require.NoError(t, err) + assert.Equal(t, "namespace%2Bname", ns) + assert.Equal(t, name+"%2Bname", object) + }) + } } -func TestEncodeString(t *testing.T) { - assert.Equal(t, "+%25%26%2B%C2%A3%E2%82%AC", encodeString(" %&+£€")) +func TestEncodePathSegment(t *testing.T) { + tests := []struct { + name string + value string + want string + }{ + {name: "space", value: " ", want: "%20"}, + {name: "plus", value: "+", want: "%2B"}, + {name: "percent", value: "%", want: "%25"}, + {name: "slash", value: "/", want: "%2F"}, + {name: "unicode", value: "£€", want: "%C2%A3%E2%82%AC"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, encodePathSegment(tt.value)) + }) + } } func TestLoadRegisteredCatalogRejectsInvalidAuthURL(t *testing.T) { From 4c580879091b4a9c83bbc24696649e1152c2e340 Mon Sep 17 00:00:00 2001 From: Alessandro Nori Date: Fri, 2 Oct 2026 15:13:00 +0200 Subject: [PATCH 3/3] Keep view payload names unencoded --- catalog/rest/rest.go | 14 ++++++------ catalog/rest/rest_test.go | 45 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 633b2e41c..1c7a34184 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -2369,7 +2369,7 @@ func (r *Catalog) CreateView(ctx context.Context, identifier table.Identifier, v return nil, fmt.Errorf("%w: view version cannot be nil", iceberg.ErrInvalidArgument) } - ns, viewName, err := r.splitViewIdentForPath(identifier) + ns, _, err := r.splitViewIdentForPath(identifier) if err != nil { return nil, err } @@ -2400,7 +2400,7 @@ func (r *Catalog) CreateView(ctx context.Context, identifier table.Identifier, v } payload := createViewRequest{ - Name: viewName, + Name: catalog.ObjectNameFromIdent(identifier), Location: cfg.Location, Schema: freshSchema, Props: cfg.Properties, @@ -2430,21 +2430,21 @@ func (r *Catalog) UpdateView(ctx context.Context, ident table.Identifier, requir return nil, err } - ns, viewName, err := r.splitViewIdentForPath(ident) + ns, encodedView, err := r.splitViewIdentForPath(ident) if err != nil { return nil, err } restIdentifier := identifier{ Namespace: catalog.NamespaceFromIdent(ident), - Name: viewName, + Name: catalog.ObjectNameFromIdent(ident), } type payload struct { Identifier identifier `json:"identifier"` Requirements []view.Requirement `json:"requirements"` Updates []view.Update `json:"updates"` } - path, err := endpointUpdateView.reqPath(ns, viewName) + path, err := endpointUpdateView.reqPath(ns, encodedView) if err != nil { return nil, err } @@ -2476,7 +2476,7 @@ func (r *Catalog) RegisterView(ctx context.Context, identifier table.Identifier, return nil, err } - ns, v, err := r.splitViewIdentForPath(identifier) + ns, _, err := r.splitViewIdentForPath(identifier) if err != nil { return nil, err } @@ -2492,7 +2492,7 @@ func (r *Catalog) RegisterView(ctx context.Context, identifier table.Identifier, } rsp, err := doPost[payload, loadViewResponse](ctx, r.baseURI, path, - payload{Name: v, MetadataLoc: metadataLoc}, r.cl, map[int]error{ + payload{Name: catalog.ObjectNameFromIdent(identifier), MetadataLoc: metadataLoc}, r.cl, map[int]error{ http.StatusNotFound: catalog.ErrNoSuchNamespace, http.StatusConflict: catalog.ErrViewAlreadyExists, }) if err != nil { diff --git a/catalog/rest/rest_test.go b/catalog/rest/rest_test.go index 9b2749960..9f73348c1 100644 --- a/catalog/rest/rest_test.go +++ b/catalog/rest/rest_test.go @@ -2773,9 +2773,50 @@ var ( }`, exampleViewMetadataJSON) ) +func (r *RestCatalogSuite) TestUpdatePathsEncodeNamesAndBodiesRemainRaw() { + const objectName = "a b+c" + + type updatePayload struct { + Identifier struct { + Name string `json:"name"` + } `json:"identifier"` + } + + r.mux.HandleFunc("/v1/namespaces/table-ns/tables/", func(w http.ResponseWriter, req *http.Request) { + r.Equal("/v1/namespaces/table-ns/tables/a%20b%2Bc", req.URL.EscapedPath()) + + var payload updatePayload + r.Require().NoError(json.NewDecoder(req.Body).Decode(&payload)) + r.Equal(objectName, payload.Identifier.Name) + + _, err := w.Write([]byte(createTableRestExample)) + r.Require().NoError(err) + }) + + r.mux.HandleFunc("/v1/namespaces/view-ns/views/", func(w http.ResponseWriter, req *http.Request) { + r.Equal("/v1/namespaces/view-ns/views/a%20b%2Bc", req.URL.EscapedPath()) + + var payload updatePayload + r.Require().NoError(json.NewDecoder(req.Body).Decode(&payload)) + r.Equal(objectName, payload.Identifier.Name) + + _, err := w.Write([]byte(createViewRestExample)) + r.Require().NoError(err) + }) + + cat, err := rest.NewCatalog(context.Background(), "rest", r.srv.URL) + r.Require().NoError(err) + + _, err = cat.UpdateTable(context.Background(), table.Identifier{"table-ns", objectName}, nil, nil) + r.Require().NoError(err) + + _, err = cat.UpdateView(context.Background(), table.Identifier{"view-ns", objectName}, nil, nil) + r.Require().NoError(err) +} + func (r *RestCatalogSuite) TestCreateView200() { ns := "ns" - viewName := "view" + viewName := "a b+c" identifier := table.Identifier{ns, viewName} schema := iceberg.NewSchemaWithIdentifiers(0, []int{1}, iceberg.NestedField{ ID: 1, @@ -2914,7 +2955,7 @@ func (r *RestCatalogSuite) TestCreateView404() { func (r *RestCatalogSuite) TestRegisterView200() { const ( ns = "fokko" - viewName = "myview" + viewName = "a b+c" metadataLoc = "s3://bucket/warehouse/fokko.db/myview/metadata/00001.metadata.json" ) identifier := table.Identifier{ns, viewName}