Skip to content

fix(rest): Encode table names in UTF-8 in REST paths #2074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alessandro-nori wants to merge 2 commits into
base: main
Choose a base branch
from
+61 −17
Open

fix(rest): Encode table names in UTF-8 in REST paths #2074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c84a31a
Encode table names in REST paths
alessandro-nori Sep 29, 2026
141d7b2
Fix REST path segment encoding
alessandro-nori Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions catalog/rest/metrics_reporter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
37 changes: 22 additions & 15 deletions catalog/rest/rest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1421,13 +1421,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())
Expand All @@ -1454,31 +1461,31 @@ 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) {
if err := catalog.ValidateViewIdentifier(ident); err != nil {
return "", "", err
}

return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), catalog.ObjectNameFromIdent(ident), nil
return r.encodeNamespace(catalog.NamespaceFromIdent(ident)), encodePathSegment(catalog.ObjectNameFromIdent(ident)), nil

@zeroshade zeroshade Oct 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now returns the encoded name, but CreateView, UpdateView and RegisterView still put that return value into the JSON body. Before this PR they sent the raw name.

Repro on 141d7b2 with table.Identifier{"ns", "my view+x"}:

  • UpdateView sends POST /v1/namespaces/ns/views/my%20view%2Bx with "identifier":{"namespace":["ns"],"name":"my%20view%2Bx"}
  • RegisterView sends "name":"my%20view%2Bx"
  • UpdateTable / RegisterTable correctly send "name":"my view+x"

So CreateView/RegisterView would create a view literally named my%20view%2Bx, and UpdateView's body identifier no longer matches its path.

Fix: do what you already did for tables. Use ns, _, err := in CreateView/RegisterView and pass catalog.ObjectNameFromIdent(identifier) as the name. In UpdateView, rename the second return to encodedView, use it only in reqPath, and use the raw name in restIdentifier.

}

func (r *Catalog) splitFunctionIdentForPath(ident table.Identifier) (string, string, error) {
if err := catalog.ValidateFunctionIdentifier(ident); err != nil {
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) {
if err := r.endpoints.check(endpointCreateTable); err != nil {
return nil, err
}

ns, tbl, err := r.splitIdentForPath(identifier)
ns, _, err := r.splitIdentForPath(identifier)
if err != nil {
return nil, err
}
Expand All @@ -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,
Expand Down Expand Up @@ -1600,14 +1607,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),
Comment thread
alessandro-nori marked this conversation as resolved.
}

type payload struct {
Expand All @@ -1616,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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -1822,21 +1829,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
}
Expand Down
37 changes: 37 additions & 0 deletions catalog/rest/rest_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,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)
})
Comment on lines +90 to +99

@zeroshade zeroshade Oct 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These only check what the split helpers return, so nothing checks that request bodies stay raw. That gap is how the view regression got through. Please add a RestCatalogSuite test with a name like a b+c: assert req.URL.EscapedPath() ends in a%20b%2Bc and that the JSON name (or identifier.name) is exactly a b+c. Cover at least one table call (UpdateTable or RegisterTable) and one view call (CreateView, RegisterView or UpdateView).

}
}

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) {
Expand Down
Loading