iceberg: return 400 for invalid namespace/table names (#10051)

* iceberg: return 400 for invalid namespace/table names

The S3 Tables name charset (a-z, 0-9, _) is stricter than the Iceberg
REST spec, so clients sending hyphens or uppercase hit a validation
error. That error fell through to 500; it's client input, so map it to
400 BadRequestException across the namespace and table handlers.

* iceberg: tighten name-validation error matching

Match the validator's own phrasings (invalid/must/cannot) instead of a
bare "namespace name"/"table name" substring, so an unrelated fault that
happens to mention a name isn't misreported as a 400. Lowercase first to
stay robust to message capitalization.

weed/s3api/iceberg/handlers_commit.go

@@ -189,7 +189,7 @@ func (s *Server) handleUpdateTable(w http.ResponseWriter, r *http.Request) {
 				return
 			}
 			glog.V(1).Infof("Iceberg: CommitTable GetTable error: %v", err)
-			writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+			writeManagerError(w, err)
 			return
 		}
 

weed/s3api/iceberg/handlers_namespace.go

@@ -83,7 +83,7 @@ func (s *Server) handleListNamespaces(w http.ResponseWriter, r *http.Request) {
 
 	if err != nil {
 		glog.Infof("Iceberg: ListNamespaces error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 
@@ -139,7 +139,7 @@ func (s *Server) handleCreateNamespace(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		glog.Errorf("Iceberg: CreateNamespace error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 
@@ -183,7 +183,7 @@ func (s *Server) handleGetNamespace(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		glog.V(1).Infof("Iceberg: GetNamespace error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 
@@ -332,6 +332,10 @@ func (s *Server) handleNamespaceExists(w http.ResponseWriter, r *http.Request) {
 			w.WriteHeader(http.StatusNotFound)
 			return
 		}
+		if nameValidationError(err) {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
 		glog.V(1).Infof("Iceberg: NamespaceExists error: %v", err)
 		w.WriteHeader(http.StatusInternalServerError)
 		return
@@ -375,7 +379,7 @@ func (s *Server) handleDropNamespace(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		glog.V(1).Infof("Iceberg: DropNamespace error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 

weed/s3api/iceberg/handlers_table.go

@@ -69,7 +69,7 @@ func (s *Server) handleListTables(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		glog.V(1).Infof("Iceberg: ListTables error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 
@@ -212,7 +212,7 @@ func (s *Server) handleCreateTable(w http.ResponseWriter, r *http.Request) {
 	}
 	if !isNoSuchTableError(existsErr) {
 		glog.V(1).Infof("Iceberg: CreateTable existence check failed for %s.%s: %v", flattenNamespacePath(namespace), tableName, existsErr)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", existsErr.Error())
+		writeManagerError(w, existsErr)
 		return
 	}
 
@@ -302,7 +302,7 @@ func (s *Server) handleCreateTable(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		glog.V(1).Infof("Iceberg: CreateTable error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 
@@ -359,7 +359,7 @@ func (s *Server) handleLoadTable(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		glog.V(1).Infof("Iceberg: LoadTable error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 
@@ -509,7 +509,7 @@ func (s *Server) handleDropTable(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		glog.V(1).Infof("Iceberg: DropTable error: %v", err)
-		writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+		writeManagerError(w, err)
 		return
 	}
 

weed/s3api/iceberg/iceberg_invalid_name_test.go

@@ -0,0 +1,65 @@
+package iceberg
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestNameValidationError(t *testing.T) {
+	cases := []struct {
+		err  error
+		want bool
+	}{
+		{nil, false},
+		// Wrapped exactly as the s3tables manager surfaces it.
+		{fmt.Errorf("all filers failed, last error: invalid namespace name: only 'a-z', '0-9', and '_' are allowed"), true},
+		{fmt.Errorf("invalid table name: only 'a-z', '0-9', and '_' are allowed"), true},
+		{fmt.Errorf("namespace name must start with a letter or digit"), true},
+		{fmt.Errorf("namespace name cannot start with reserved prefix 'aws'"), true},
+		{fmt.Errorf("table name must be between 1 and 255 characters"), true},
+		{fmt.Errorf("namespace not found"), false},
+		{fmt.Errorf("all filers failed, last error: rpc timeout"), false},
+		// Unrelated faults that merely mention a name must stay 500.
+		{fmt.Errorf("failed to resolve table name from index"), false},
+		{fmt.Errorf("error fetching namespace name mapping"), false},
+	}
+	for _, c := range cases {
+		if got := nameValidationError(c.err); got != c.want {
+			t.Errorf("nameValidationError(%v) = %v, want %v", c.err, got, c.want)
+		}
+	}
+}
+
+func TestWriteManagerError(t *testing.T) {
+	cases := []struct {
+		name     string
+		err      error
+		wantCode int
+		wantType string
+	}{
+		{"invalid name is a client error", fmt.Errorf("invalid namespace name: only 'a-z', '0-9', and '_' are allowed"), http.StatusBadRequest, "BadRequestException"},
+		{"everything else is a server fault", fmt.Errorf("all filers failed, last error: connection refused"), http.StatusInternalServerError, "InternalServerError"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			rec := httptest.NewRecorder()
+			writeManagerError(rec, c.err)
+			if rec.Code != c.wantCode {
+				t.Fatalf("status = %d, want %d", rec.Code, c.wantCode)
+			}
+			var resp ErrorResponse
+			if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
+				t.Fatalf("decode response: %v", err)
+			}
+			if resp.Error.Type != c.wantType {
+				t.Fatalf("error type = %q, want %q", resp.Error.Type, c.wantType)
+			}
+			if resp.Error.Code != c.wantCode {
+				t.Fatalf("error code = %d, want %d", resp.Error.Code, c.wantCode)
+			}
+		})
+	}
+}

weed/s3api/iceberg/utils.go

@@ -99,6 +99,41 @@ func writeError(w http.ResponseWriter, status int, errType, message string) {
 	writeJSON(w, status, resp)
 }
 
+// nameValidationError reports whether err is an S3 Tables namespace or table
+// name validation failure. Such names are client input (the S3 Tables charset
+// is stricter than the Iceberg REST spec), so the catalog answers 400 rather
+// than treating it as a server fault.
+func nameValidationError(err error) bool {
+	if err == nil {
+		return false
+	}
+	// Match the validator's own phrasings rather than a bare "namespace name"/
+	// "table name" so unrelated faults (e.g. "failed to resolve table name")
+	// aren't misreported as client errors. Lowercased for resilience to
+	// capitalization changes.
+	msg := strings.ToLower(err.Error())
+	for _, marker := range []string{
+		"invalid namespace name", "namespace name must", "namespace name cannot",
+		"invalid table name", "table name must", "table name cannot",
+	} {
+		if strings.Contains(msg, marker) {
+			return true
+		}
+	}
+	return false
+}
+
+// writeManagerError maps a residual s3tables manager error to a response:
+// name validation failures are client errors (400); anything else is a server
+// fault (500).
+func writeManagerError(w http.ResponseWriter, err error) {
+	if nameValidationError(err) {
+		writeError(w, http.StatusBadRequest, "BadRequestException", err.Error())
+		return
+	}
+	writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error())
+}
+
 // getBucketFromPrefix extracts table bucket name from prefix parameter.
 // For now, we use the prefix as the table bucket name.
 //