fix: address CORS final-review findings (multi-rule preflight, raw GET, e2e test)

Signed-off-by: h5vx <h5v@protonmail.com>

cmd/api-router.go

@@ -646,7 +646,6 @@ func registerAPIRouter(router *mux.Router) {
 	apiRouter.MethodNotAllowedHandler = collectAPIStats("methodnotallowed", httpTraceAll(methodNotAllowedHandler("S3")))
 }
 
-// corsHandler handler for CORS (Cross Origin Resource Sharing)
 // applyBucketCors applies a bucket's CORS configuration to the request.
 // For an OPTIONS preflight it writes the full CORS response and returns true
 // (request is complete). For an actual request it adds the applicable
@@ -664,13 +663,8 @@ func applyBucketCors(w http.ResponseWriter, r *http.Request, cfg *bktcors.Config
 
 	if isPreflight {
 		method := r.Header.Get("Access-Control-Request-Method")
-		rule, ok := cfg.MatchRule(origin, method)
-		if !ok {
-			writeResponse(w, http.StatusForbidden, nil, mimeNone)
-			return true
-		}
 		reqHeaders := splitAndTrim(r.Header.Get("Access-Control-Request-Headers"))
-		allowedHeaders, ok := rule.FilterAllowedHeaders(reqHeaders)
+		rule, allowedHeaders, ok := cfg.MatchPreflight(origin, method, reqHeaders)
 		if !ok {
 			writeResponse(w, http.StatusForbidden, nil, mimeNone)
 			return true
@@ -720,6 +714,7 @@ func splitAndTrim(s string) []string {
 	return out
 }
 
+// corsHandler handler for CORS (Cross Origin Resource Sharing)
 func corsHandler(handler http.Handler) http.Handler {
 	commonS3Headers := []string{
 		xhttp.Date,

cmd/bucket-cors-handlers.go

@@ -20,7 +20,6 @@ package cmd
 import (
 	"bytes"
 	"encoding/base64"
-	"encoding/xml"
 	"errors"
 	"io"
 	"net/http"
@@ -136,7 +135,7 @@ func (api objectAPIHandlers) GetBucketCorsHandler(w http.ResponseWriter, r *http
 		return
 	}
 
-	config, _, err := globalBucketMetadataSys.GetCorsConfig(bucket)
+	configData, _, err := globalBucketMetadataSys.GetCorsConfigXML(bucket)
 	if err != nil {
 		if errors.Is(err, errConfigNotFound) {
 			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoSuchCORSConfiguration), r.URL)
@@ -146,12 +145,6 @@ func (api objectAPIHandlers) GetBucketCorsHandler(w http.ResponseWriter, r *http
 		return
 	}
 
-	configData, err := xml.Marshal(config)
-	if err != nil {
-		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
-		return
-	}
-
 	writeSuccessResponseXML(w, configData)
 }
 

cmd/bucket-cors-handlers_test.go

@@ -97,4 +97,36 @@ func testBucketCorsHandlers(obj ObjectLayer, instanceType, bucketName string, ap
 	if rec.Code != http.StatusBadRequest {
 		t.Fatalf("PUT malformed cors: expected 400, got %d", rec.Code)
 	}
+
+	// Re-PUT the config so the store→GetCorsConfig→enforce seam below has
+	// something to enforce (the earlier DELETE removed it).
+	req, err = newTestSignedRequestV4(http.MethodPut, getBucketCorsURL("", bucketName),
+		int64(len(testCORSDoc)), bytes.NewReader([]byte(testCORSDoc)), creds.AccessKey, creds.SecretKey, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec = httptest.NewRecorder()
+	apiRouter.ServeHTTP(rec, req)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("PUT cors (re-put): expected 200, got %d: %s", rec.Code, rec.Body.String())
+	}
+
+	// End-to-end enforcement: drive an OPTIONS preflight through the real
+	// corsHandler wrapper (not applyBucketCors in isolation), exercising the
+	// full store -> globalBucketMetadataSys.GetCorsConfig -> enforce seam.
+	wrapped := corsHandler(apiRouter)
+
+	preflightURL := getBucketCorsURL("", bucketName)
+	preflightReq := httptest.NewRequest(http.MethodOptions, preflightURL, nil)
+	preflightReq.Header.Set("Origin", "http://example.com")
+	preflightReq.Header.Set("Access-Control-Request-Method", http.MethodGet)
+
+	rec = httptest.NewRecorder()
+	wrapped.ServeHTTP(rec, preflightReq)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("OPTIONS preflight via corsHandler: expected 200, got %d: %s", rec.Code, rec.Body.String())
+	}
+	if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "http://example.com" {
+		t.Fatalf("OPTIONS preflight via corsHandler: expected Access-Control-Allow-Origin echoed, got %q", got)
+	}
 }

cmd/bucket-metadata-sys.go

@@ -376,6 +376,20 @@ func (sys *BucketMetadataSys) GetCorsConfig(bucket string) (*cors.Config, time.T
 	return meta.corsConfig, meta.CorsConfigUpdatedAt, nil
 }
 
+// GetCorsConfigXML returns the raw stored CORS configuration XML for the
+// given bucket, preserving the document exactly as it was PUT (including
+// the S3 xmlns and any unmodeled elements).
+func (sys *BucketMetadataSys) GetCorsConfigXML(bucket string) ([]byte, time.Time, error) {
+	meta, _, err := sys.GetConfig(GlobalContext, bucket)
+	if err != nil {
+		return nil, time.Time{}, err
+	}
+	if len(meta.CorsConfigXML) == 0 {
+		return nil, time.Time{}, errConfigNotFound
+	}
+	return meta.CorsConfigXML, meta.CorsConfigUpdatedAt, nil
+}
+
 // CreatedAt returns the time of creation of bucket
 func (sys *BucketMetadataSys) CreatedAt(bucket string) (time.Time, error) {
 	meta, _, err := sys.GetConfig(GlobalContext, bucket)

internal/bucket/cors/cors.go

@@ -148,3 +148,23 @@ func (c *Config) MatchRule(origin, method string) (*Rule, bool) {
 	}
 	return nil, false
 }
+
+// MatchPreflight returns the first rule whose origin and method match and
+// whose AllowedHeaders permit every header in reqHeaders. Unlike MatchRule,
+// this keeps evaluating subsequent rules until one fully satisfies the
+// preflight request, since an earlier origin/method match with a more
+// restrictive header list must not shadow a later, more permissive rule.
+func (c *Config) MatchPreflight(origin, method string, reqHeaders []string) (rule *Rule, allowedHeaders []string, ok bool) {
+	for i := range c.CORSRules {
+		r := &c.CORSRules[i]
+		if !r.HasAllowedOrigin(origin) || !r.HasAllowedMethod(method) {
+			continue
+		}
+		allowed, headersOK := r.FilterAllowedHeaders(reqHeaders)
+		if !headersOK {
+			continue
+		}
+		return r, allowed, true
+	}
+	return nil, nil, false
+}

internal/bucket/cors/cors_test.go

@@ -89,3 +89,40 @@ func TestMatching(t *testing.T) {
 		t.Fatal("did not expect authorization to be allowed")
 	}
 }
+
+func TestMatchPreflightFallsThroughToLaterRule(t *testing.T) {
+	// Rule A matches origin+method but only allows a restrictive header set.
+	// Rule B, listed after A, matches the same origin+method and allows any
+	// header. A preflight requesting a header only B permits must not be
+	// rejected just because A was tried first.
+	const doc = `<CORSConfiguration>
+  <CORSRule>
+    <ID>A-restrictive</ID>
+    <AllowedOrigin>https://app.example.com</AllowedOrigin>
+    <AllowedMethod>GET</AllowedMethod>
+    <AllowedHeader>x-amz-date</AllowedHeader>
+  </CORSRule>
+  <CORSRule>
+    <ID>B-permissive</ID>
+    <AllowedOrigin>https://app.example.com</AllowedOrigin>
+    <AllowedMethod>GET</AllowedMethod>
+    <AllowedHeader>*</AllowedHeader>
+  </CORSRule>
+</CORSConfiguration>`
+
+	c, err := ParseBucketCorsConfig(strings.NewReader(doc))
+	if err != nil {
+		t.Fatalf("parse failed: %v", err)
+	}
+
+	rule, allowed, ok := c.MatchPreflight("https://app.example.com", "GET", []string{"x-custom-header"})
+	if !ok {
+		t.Fatal("expected MatchPreflight to succeed via the later, permissive rule")
+	}
+	if rule.ID != "B-permissive" {
+		t.Fatalf("expected rule B-permissive to be selected, got %q", rule.ID)
+	}
+	if len(allowed) != 1 || allowed[0] != "x-custom-header" {
+		t.Fatalf("unexpected allowed headers: %v", allowed)
+	}
+}