s3tables: improve robustness, security, and error propagation in handlers
- Implement strict table name validation (prevention of path traversal and character enforcement) - Add nil checks for entry.Entry in all listing loops to prevent panics - Propagate backend errors instead of swallowing them or assuming 404 - Correctly map filer_pb.ErrNotFound to appropriate S3 error codes - Standardize existence checks across bucket, namespace, and table handlers
This commit is contained in:
@@ -2,6 +2,7 @@ package s3tables
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"time"
|
"time"
|
||||||
@@ -47,17 +48,17 @@ func (h *S3TablesHandler) handleCreateTableBucket(w http.ResponseWriter, r *http
|
|||||||
// Check if bucket already exists
|
// Check if bucket already exists
|
||||||
exists := false
|
exists := false
|
||||||
err := filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err := filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
resp, err := client.LookupDirectoryEntry(r.Context(), &filer_pb.LookupDirectoryEntryRequest{
|
_, err := filer_pb.LookupEntry(r.Context(), client, &filer_pb.LookupDirectoryEntryRequest{
|
||||||
Directory: TablesPath,
|
Directory: TablesPath,
|
||||||
Name: req.Name,
|
Name: req.Name,
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// Not found is expected when creating a new bucket
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
if resp.Entry != nil {
|
return err
|
||||||
exists = true
|
|
||||||
}
|
}
|
||||||
|
exists = true
|
||||||
return nil
|
return nil
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -48,7 +48,7 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re
|
|||||||
})
|
})
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if errors.Is(err, ErrNotFound) {
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||||
} else {
|
} else {
|
||||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to get table bucket: %v", err))
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to get table bucket: %v", err))
|
||||||
@@ -108,6 +108,9 @@ func (h *S3TablesHandler) handleListTableBuckets(w http.ResponseWriter, r *http.
|
|||||||
if respErr != nil {
|
if respErr != nil {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
if entry.Entry == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
hasMore = true
|
hasMore = true
|
||||||
lastFileName = entry.Entry.Name
|
lastFileName = entry.Entry.Name
|
||||||
|
|
||||||
@@ -157,7 +160,7 @@ func (h *S3TablesHandler) handleListTableBuckets(w http.ResponseWriter, r *http.
|
|||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// Check if it's a "not found" error - return empty list in that case
|
// Check if it's a "not found" error - return empty list in that case
|
||||||
if errors.Is(err, ErrNotFound) {
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
buckets = []TableBucketSummary{}
|
buckets = []TableBucketSummary{}
|
||||||
} else {
|
} else {
|
||||||
// For other errors, return error response
|
// For other errors, return error response
|
||||||
@@ -228,7 +231,7 @@ func (h *S3TablesHandler) handleDeleteTableBucket(w http.ResponseWriter, r *http
|
|||||||
})
|
})
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if !errors.Is(err, ErrNotFound) {
|
if !errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to list bucket entries: %v", err))
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to list bucket entries: %v", err))
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -50,31 +50,34 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R
|
|||||||
|
|
||||||
// Check if table bucket exists
|
// Check if table bucket exists
|
||||||
bucketPath := getTableBucketPath(bucketName)
|
bucketPath := getTableBucketPath(bucketName)
|
||||||
var bucketExists bool
|
|
||||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
_, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata)
|
_, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata)
|
||||||
bucketExists = err == nil
|
return err
|
||||||
return nil
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if !bucketExists {
|
if err != nil {
|
||||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
return fmt.Errorf("bucket not found")
|
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||||
|
} else {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check table bucket: %v", err))
|
||||||
|
}
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
namespacePath := getNamespacePath(bucketName, namespaceName)
|
namespacePath := getNamespacePath(bucketName, namespaceName)
|
||||||
|
|
||||||
// Check if namespace already exists
|
// Check if namespace already exists
|
||||||
exists := false
|
|
||||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
_, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata)
|
_, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata)
|
||||||
exists = err == nil
|
return err
|
||||||
return nil
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if exists {
|
if err == nil {
|
||||||
h.writeError(w, http.StatusConflict, ErrCodeNamespaceAlreadyExists, fmt.Sprintf("namespace %s already exists", namespaceName))
|
h.writeError(w, http.StatusConflict, ErrCodeNamespaceAlreadyExists, fmt.Sprintf("namespace %s already exists", namespaceName))
|
||||||
return fmt.Errorf("namespace already exists")
|
return fmt.Errorf("namespace already exists")
|
||||||
|
} else if !errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check namespace: %v", err))
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create the namespace
|
// Create the namespace
|
||||||
@@ -231,10 +234,13 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re
|
|||||||
if respErr != nil {
|
if respErr != nil {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
if entry.Entry == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
hasMore = true
|
hasMore = true
|
||||||
lastFileName = entry.Entry.Name
|
lastFileName = entry.Entry.Name
|
||||||
|
|
||||||
if entry.Entry == nil || !entry.Entry.IsDirectory {
|
if !entry.Entry.IsDirectory {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -278,7 +284,12 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re
|
|||||||
})
|
})
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
namespaces = []NamespaceSummary{}
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
|
namespaces = []NamespaceSummary{}
|
||||||
|
} else {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to list namespaces: %v", err))
|
||||||
|
return err
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
resp := &ListNamespacesResponse{
|
resp := &ListNamespacesResponse{
|
||||||
@@ -349,7 +360,7 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R
|
|||||||
})
|
})
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if !errors.Is(err, ErrNotFound) {
|
if !errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to list namespace entries: %v", err))
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to list namespace entries: %v", err))
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -35,16 +35,18 @@ func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *h
|
|||||||
|
|
||||||
// Check if bucket exists
|
// Check if bucket exists
|
||||||
bucketPath := getTableBucketPath(bucketName)
|
bucketPath := getTableBucketPath(bucketName)
|
||||||
var bucketExists bool
|
|
||||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
_, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata)
|
_, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata)
|
||||||
bucketExists = err == nil
|
return err
|
||||||
return nil
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if !bucketExists {
|
if err != nil {
|
||||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
return fmt.Errorf("bucket not found")
|
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||||
|
} else {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check table bucket: %v", err))
|
||||||
|
}
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Write policy
|
// Write policy
|
||||||
@@ -125,7 +127,7 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r
|
|||||||
return h.deleteExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
|
return h.deleteExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
|
||||||
})
|
})
|
||||||
|
|
||||||
if err != nil && !errors.Is(err, ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
if err != nil && !errors.Is(err, filer_pb.ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
||||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, "failed to delete table bucket policy")
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, "failed to delete table bucket policy")
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -166,16 +168,18 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re
|
|||||||
|
|
||||||
// Check if table exists
|
// Check if table exists
|
||||||
tablePath := getTablePath(bucketName, namespaceName, req.Name)
|
tablePath := getTablePath(bucketName, namespaceName, req.Name)
|
||||||
var tableExists bool
|
|
||||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
_, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata)
|
_, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata)
|
||||||
tableExists = err == nil
|
return err
|
||||||
return nil
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if !tableExists {
|
if err != nil {
|
||||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchTable, fmt.Sprintf("table %s not found", req.Name))
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
return fmt.Errorf("table not found")
|
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchTable, fmt.Sprintf("table %s not found", req.Name))
|
||||||
|
} else {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check table: %v", err))
|
||||||
|
}
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Write policy
|
// Write policy
|
||||||
@@ -268,7 +272,7 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http
|
|||||||
return h.deleteExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyPolicy)
|
return h.deleteExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyPolicy)
|
||||||
})
|
})
|
||||||
|
|
||||||
if err != nil && !errors.Is(err, ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
if err != nil && !errors.Is(err, filer_pb.ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
||||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, "failed to delete table policy")
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, "failed to delete table policy")
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package s3tables
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -57,34 +58,48 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque
|
|||||||
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, "table name must be between 1 and 255 characters")
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, "table name must be between 1 and 255 characters")
|
||||||
return fmt.Errorf("invalid table name length")
|
return fmt.Errorf("invalid table name length")
|
||||||
}
|
}
|
||||||
|
if req.Name == "." || req.Name == ".." || strings.Contains(req.Name, "/") {
|
||||||
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, "invalid table name: cannot be '.', '..' or contain '/'")
|
||||||
|
return fmt.Errorf("invalid table name")
|
||||||
|
}
|
||||||
|
for _, ch := range req.Name {
|
||||||
|
if (ch >= 'a' && ch <= 'z') || (ch >= '0' && ch <= '9') || ch == '_' {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, "invalid table name: only 'a-z', '0-9', and '_' are allowed")
|
||||||
|
return fmt.Errorf("invalid table name")
|
||||||
|
}
|
||||||
|
|
||||||
// Check if namespace exists
|
// Check if namespace exists
|
||||||
namespacePath := getNamespacePath(bucketName, namespaceName)
|
namespacePath := getNamespacePath(bucketName, namespaceName)
|
||||||
var namespaceExists bool
|
|
||||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
_, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata)
|
_, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata)
|
||||||
namespaceExists = err == nil
|
return err
|
||||||
return nil
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if !namespaceExists {
|
if err != nil {
|
||||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", namespaceName))
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
return fmt.Errorf("namespace not found")
|
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", namespaceName))
|
||||||
|
} else {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check namespace: %v", err))
|
||||||
|
}
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
tablePath := getTablePath(bucketName, namespaceName, req.Name)
|
tablePath := getTablePath(bucketName, namespaceName, req.Name)
|
||||||
|
|
||||||
// Check if table already exists
|
// Check if table already exists
|
||||||
exists := false
|
|
||||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
_, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata)
|
_, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata)
|
||||||
exists = err == nil
|
return err
|
||||||
return nil
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if exists {
|
if err == nil {
|
||||||
h.writeError(w, http.StatusConflict, ErrCodeTableAlreadyExists, fmt.Sprintf("table %s already exists", req.Name))
|
h.writeError(w, http.StatusConflict, ErrCodeTableAlreadyExists, fmt.Sprintf("table %s already exists", req.Name))
|
||||||
return fmt.Errorf("table already exists")
|
return fmt.Errorf("table already exists")
|
||||||
|
} else if !errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check table: %v", err))
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create the table
|
// Create the table
|
||||||
@@ -299,10 +314,13 @@ func (h *S3TablesHandler) listTablesInNamespaceWithClient(ctx context.Context, c
|
|||||||
if respErr != nil {
|
if respErr != nil {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
if entry.Entry == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
hasMore = true
|
hasMore = true
|
||||||
lastFileName = entry.Entry.Name
|
lastFileName = entry.Entry.Name
|
||||||
|
|
||||||
if entry.Entry == nil || !entry.Entry.IsDirectory {
|
if !entry.Entry.IsDirectory {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -373,10 +391,13 @@ func (h *S3TablesHandler) listTablesInAllNamespaces(ctx context.Context, filerCl
|
|||||||
if respErr != nil {
|
if respErr != nil {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
if entry.Entry == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
hasMore = true
|
hasMore = true
|
||||||
lastFileName = entry.Entry.Name
|
lastFileName = entry.Entry.Name
|
||||||
|
|
||||||
if entry.Entry == nil || !entry.Entry.IsDirectory {
|
if !entry.Entry.IsDirectory {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -434,16 +455,18 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque
|
|||||||
tablePath := getTablePath(bucketName, namespaceName, req.Name)
|
tablePath := getTablePath(bucketName, namespaceName, req.Name)
|
||||||
|
|
||||||
// Check if table exists
|
// Check if table exists
|
||||||
var tableExists bool
|
|
||||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||||
_, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata)
|
_, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata)
|
||||||
tableExists = err == nil
|
return err
|
||||||
return nil
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if !tableExists {
|
if err != nil {
|
||||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchTable, fmt.Sprintf("table %s not found", req.Name))
|
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||||
return fmt.Errorf("table not found")
|
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchTable, fmt.Sprintf("table %s not found", req.Name))
|
||||||
|
} else {
|
||||||
|
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check table: %v", err))
|
||||||
|
}
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Delete the table
|
// Delete the table
|
||||||
|
|||||||
Reference in New Issue
Block a user