diff --git a/weed/s3api/cors/middleware.go b/weed/s3api/cors/middleware.go index 42dd49974..9f343f0ef 100644 --- a/weed/s3api/cors/middleware.go +++ b/weed/s3api/cors/middleware.go @@ -36,18 +36,30 @@ func NewMiddleware(bucketChecker BucketChecker, corsConfigGetter CORSConfigGette // getCORSConfig retrieves the applicable CORS configuration, trying bucket-specific first, then fallback. // Returns the configuration and a boolean indicating if any configuration was found. +// Only falls back to global config when there's explicitly no bucket-level config. +// For other errors (e.g., access denied), returns false to let the handler deny the request. func (m *Middleware) getCORSConfig(bucket string) (*CORSConfiguration, bool) { - // Try bucket-specific config first config, errCode := m.corsConfigGetter.GetCORSConfiguration(bucket) - if errCode == s3err.ErrNone && config != nil { - return config, true + + switch errCode { + case s3err.ErrNone: + // Found a config, use it + if config != nil { + return config, true + } + // ErrNone with nil config means no bucket-level config, fall through to fallback + case s3err.ErrNoSuchCORSConfiguration: + // Explicitly no CORS config, fall through to fallback + default: + // Real error (access denied, network failure, etc.) - do not fall back + return nil, false } - // Fallback to global config + // No bucket-level config found, try global fallback if m.fallbackConfig != nil { return m.fallbackConfig, true } - + return nil, false } diff --git a/weed/s3api/cors/middleware_test.go b/weed/s3api/cors/middleware_test.go index e9335ca17..e9f89a038 100644 --- a/weed/s3api/cors/middleware_test.go +++ b/weed/s3api/cors/middleware_test.go @@ -326,3 +326,80 @@ func TestMiddlewareFallbackConfigWithMultipleOrigins(t *testing.T) { }) } } + +// TestMiddlewareFallbackWithError tests that real errors (not "no config") don't trigger fallback +func TestMiddlewareFallbackWithError(t *testing.T) { + fallbackConfig := &CORSConfiguration{ + CORSRules: []CORSRule{ + { + AllowedOrigins: []string{"*"}, + AllowedMethods: []string{"GET", "POST"}, + AllowedHeaders: []string{"*"}, + }, + }, + } + + tests := []struct { + name string + errCode s3err.ErrorCode + expectedOriginHeader string + description string + }{ + { + name: "ErrAccessDenied should not trigger fallback", + errCode: s3err.ErrAccessDenied, + expectedOriginHeader: "", + description: "Access denied errors should not expose CORS headers", + }, + { + name: "ErrInternalError should not trigger fallback", + errCode: s3err.ErrInternalError, + expectedOriginHeader: "", + description: "Internal errors should not expose CORS headers", + }, + { + name: "ErrNoSuchBucket should not trigger fallback", + errCode: s3err.ErrNoSuchBucket, + expectedOriginHeader: "", + description: "Bucket not found errors should not expose CORS headers", + }, + { + name: "ErrNoSuchCORSConfiguration should trigger fallback", + errCode: s3err.ErrNoSuchCORSConfiguration, + expectedOriginHeader: "https://example.com", + description: "Explicit no CORS config should use fallback", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bucketChecker := &mockBucketChecker{bucketExists: true} + configGetter := &mockCORSConfigGetter{ + config: nil, + errCode: tt.errCode, + } + + middleware := NewMiddleware(bucketChecker, configGetter, fallbackConfig) + + req := httptest.NewRequest("GET", "/testbucket/testobject", nil) + req = mux.SetURLVars(req, map[string]string{ + "bucket": "testbucket", + "object": "testobject", + }) + req.Header.Set("Origin", "https://example.com") + + w := httptest.NewRecorder() + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + middleware.Handler(nextHandler).ServeHTTP(w, req) + + actualOrigin := w.Header().Get("Access-Control-Allow-Origin") + if actualOrigin != tt.expectedOriginHeader { + t.Errorf("%s: expected Access-Control-Allow-Origin='%s', got '%s'", + tt.description, tt.expectedOriginHeader, actualOrigin) + } + }) + } +}