Browse Source

s3tables: Fix bucket policy error handling in permission checks

Replace error-swallowing pattern where all errors from getExtendedAttribute
were ignored for bucket policy reads. Now properly distinguish between:

- ErrAttributeNotFound: Policy not found is expected; continue with empty policy
- Other errors: Return internal server error and stop processing

Applied fix to all bucket policy reads in:
- handleDeleteTableBucketPolicy (line 220)
- handleTagResource (line 313)
- handleUntagResource (line 405)
- handleListTagsForResource (line 488)
- And additional occurrences in closures

This prevents silent failures and ensures policy-related errors are surfaced
to callers rather than being silently ignored.
pull/8147/head
Chris Lu 4 days ago
parent
commit
8eee6b2a0e
  1. 49
      weed/s3api/s3tables/handler_policy.go

49
weed/s3api/s3tables/handler_policy.go

@ -218,7 +218,12 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r
// Fetch bucket policy if it exists
policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
if err == nil {
if err != nil {
if !errors.Is(err, ErrAttributeNotFound) {
return fmt.Errorf("failed to read bucket policy: %w", err)
}
// Policy not found is not an error; bucketPolicy remains empty
} else {
bucketPolicy = string(policyData)
}
@ -306,7 +311,12 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re
// Fetch bucket policy if it exists
policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
if err == nil {
if err != nil {
if !errors.Is(err, ErrAttributeNotFound) {
return fmt.Errorf("failed to read bucket policy: %w", err)
}
// Policy not found is not an error; bucketPolicy remains empty
} else {
bucketPolicy = string(policyData)
}
@ -398,7 +408,12 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re
// Fetch bucket policy if it exists
policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
if err == nil {
if err != nil {
if !errors.Is(err, ErrAttributeNotFound) {
return fmt.Errorf("failed to read bucket policy: %w", err)
}
// Policy not found is not an error; bucketPolicy remains empty
} else {
bucketPolicy = string(policyData)
}
@ -481,7 +496,12 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http
// Fetch bucket policy if it exists
policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
if err == nil {
if err != nil {
if !errors.Is(err, ErrAttributeNotFound) {
return fmt.Errorf("failed to read bucket policy: %w", err)
}
// Policy not found is not an error; bucketPolicy remains empty
} else {
bucketPolicy = string(policyData)
}
@ -560,7 +580,12 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque
if bucketName != "" {
bucketPath := getTableBucketPath(bucketName)
policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
if err == nil {
if err != nil {
if !errors.Is(err, ErrAttributeNotFound) {
return fmt.Errorf("failed to read bucket policy: %w", err)
}
// Policy not found is not an error; bucketPolicy remains empty
} else {
bucketPolicy = string(policyData)
}
}
@ -659,7 +684,12 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht
if bucketName != "" {
bucketPath := getTableBucketPath(bucketName)
policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
if err == nil {
if err != nil {
if !errors.Is(err, ErrAttributeNotFound) {
return fmt.Errorf("failed to read bucket policy: %w", err)
}
// Policy not found is not an error; bucketPolicy remains empty
} else {
bucketPolicy = string(policyData)
}
}
@ -746,7 +776,12 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req
if bucketName != "" {
bucketPath := getTableBucketPath(bucketName)
policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
if err == nil {
if err != nil {
if !errors.Is(err, ErrAttributeNotFound) {
return fmt.Errorf("failed to read bucket policy: %w", err)
}
// Policy not found is not an error; bucketPolicy remains empty
} else {
bucketPolicy = string(policyData)
}
}

Loading…
Cancel
Save