You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ze...@apache.org on 2026/10/02 20:12:43 UTC
(iceberg-go) branch main updated: fix(rest): Encode identifier path segments (#2074)
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 fa5309b36 fix(rest): Encode identifier path segments (#2074)
fa5309b36 is described below
commit fa5309b3624d5798bde69722b279366bb7f39846
Author: Alessandro Nori <al...@datadoghq.com>
AuthorDate: Fri Oct 2 22:12:37 2026 +0200
fix(rest): Encode identifier path segments (#2074)
* Encode table names in REST paths
* Fix REST path segment encoding
* Keep view payload names unencoded
---
catalog/rest/metrics_reporter_test.go | 4 +--
catalog/rest/rest.go | 51 ++++++++++++++++++++---------------
catalog/rest/rest_internal_test.go | 37 +++++++++++++++++++++++++
catalog/rest/rest_test.go | 45 +++++++++++++++++++++++++++++--
4 files changed, 111 insertions(+), 26 deletions(-)
diff --git a/catalog/rest/metrics_reporter_test.go b/catalog/rest/metrics_reporter_test.go
index b901e7459..7485135b7 100644
--- a/catalog/rest/metrics_reporter_test.go
+++ b/catalog/rest/metrics_reporter_test.go
@@ -573,8 +573,8 @@ 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.
+ // 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 29ca144c8..fae679108 100644
--- a/catalog/rest/rest.go
+++ b/catalog/rest/rest.go
@@ -1395,13 +1395,20 @@ func (r *Catalog) nsSeparator() string {
return r.namespaceSeparator
}
+// 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
// 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 {
- encoded[i] = url.PathEscape(level)
+ encoded[i] = encodePathSegment(level)
}
return strings.Join(encoded, r.nsSeparator())
@@ -1428,7 +1435,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)), encodePathSegment(catalog.ObjectNameFromIdent(ident)), nil
}
func (r *Catalog) splitViewIdentForPath(ident table.Identifier) (string, string, error) {
@@ -1436,7 +1443,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) {
@@ -1444,7 +1451,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) {
@@ -1452,7 +1459,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
}
@@ -1479,7 +1486,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,
@@ -1574,14 +1581,14 @@ 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
}
restIdentifier := identifier{
Namespace: catalog.NamespaceFromIdent(ident),
- Name: tblName,
+ Name: catalog.ObjectNameFromIdent(ident),
}
type payload struct {
@@ -1590,7 +1597,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
}
@@ -1704,7 +1711,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
}
@@ -1729,7 +1736,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 {
@@ -1796,21 +1803,21 @@ 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
}
restIdentifier := identifier{
Namespace: catalog.NamespaceFromIdent(ident),
- Name: tbl,
+ Name: catalog.ObjectNameFromIdent(ident),
}
type payload struct {
Identifier identifier `json:"identifier"`
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
}
@@ -2338,7 +2345,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
}
@@ -2369,7 +2376,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,
@@ -2399,21 +2406,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
}
@@ -2445,7 +2452,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
}
@@ -2461,7 +2468,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_internal_test.go b/catalog/rest/rest_internal_test.go
index 92da26362..c16ad2db5 100644
--- a/catalog/rest/rest_internal_test.go
+++ b/catalog/rest/rest_internal_test.go
@@ -131,6 +131,43 @@ 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%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 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) {
diff --git a/catalog/rest/rest_test.go b/catalog/rest/rest_test.go
index dc4f6653e..75f86c13a 100644
--- a/catalog/rest/rest_test.go
+++ b/catalog/rest/rest_test.go
@@ -2954,9 +2954,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,
@@ -3095,7 +3136,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}