fix: remove duplicate cors headers from options router

In the refactor for being able to set global CORS headers, the
options router was incorrectly set to use both CORS middlewares
casuing duplicate headers to be set. The ApplyBucketCORS()
middleware is not needed for options since this is already handled
by the CORSOoptions controller.

Fixes #1819

s3api/router.go

@@ -1445,7 +1445,6 @@ func (sa *S3ApiRouter) Init(app *fiber.App, be backend.Backend, iam auth.IAMServ
 		middlewares.ApplyBucketCORSPreflightFallback(be, corsAllowOrigin),
 		controllers.ProcessHandlers(ctrl.CORSOptions, metrics.ActionOptions, services,
 			middlewares.BucketObjectNameValidator(),
-			middlewares.ApplyBucketCORS(be, corsAllowOrigin),
 			middlewares.ParseAcl(be),
 		),
 	)
@@ -1454,7 +1453,6 @@ func (sa *S3ApiRouter) Init(app *fiber.App, be backend.Backend, iam auth.IAMServ
 		middlewares.ApplyBucketCORSPreflightFallback(be, corsAllowOrigin),
 		controllers.ProcessHandlers(ctrl.CORSOptions, metrics.ActionOptions, services,
 			middlewares.BucketObjectNameValidator(),
-			middlewares.ApplyBucketCORS(be, corsAllowOrigin),
 			middlewares.ParseAcl(be),
 		),
 	)

s3api/router_cors_test.go

@@ -16,9 +16,11 @@ package s3api
 
 import (
 	"context"
+	"encoding/json"
 	"net/http"
 	"testing"
 
+	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/gofiber/fiber/v2"
 	"github.com/versity/versitygw/auth"
 	"github.com/versity/versitygw/backend"
@@ -34,6 +36,26 @@ func (b backendWithCorsOnly) GetBucketCors(ctx context.Context, bucket string) (
 	return nil, s3err.GetAPIError(s3err.ErrNoSuchCORSConfiguration)
 }
 
+type backendWithBucketCors struct {
+	backend.BackendUnsupported
+	corsConfig []byte
+}
+
+func (b backendWithBucketCors) GetBucketCors(ctx context.Context, bucket string) ([]byte, error) {
+	if b.corsConfig != nil {
+		return b.corsConfig, nil
+	}
+	return nil, s3err.GetAPIError(s3err.ErrNoSuchCORSConfiguration)
+}
+
+func (b backendWithBucketCors) GetBucketAcl(ctx context.Context, input *s3.GetBucketAclInput) ([]byte, error) {
+	// Return a minimal ACL in JSON format
+	acl := auth.ACL{
+		Owner: "test",
+	}
+	return json.Marshal(acl)
+}
+
 func TestS3ApiRouter_ListBuckets_DefaultCORSAllowOrigin(t *testing.T) {
 	origin := "https://example.com"
 
@@ -251,3 +273,100 @@ func TestS3ApiRouter_PutObject_ErrorStillIncludesFallbackCORS(t *testing.T) {
 		t.Fatalf("expected Access-Control-Allow-Origin %q, got %q", origin, got)
 	}
 }
+
+func TestS3ApiRouter_OptionsWithBucketCORS_NoDuplicateHeaders(t *testing.T) {
+	// This test reproduces the issue from GitHub issue #1819
+	// where CORS headers were duplicated in OPTIONS responses
+	bucketOrigin := "http://localhost:3333"
+	fallbackOrigin := "https://fallback.example.com"
+
+	// CORS configuration matching the issue reproduction steps
+	corsConfig := []byte(`<CORSConfiguration>
+		<CORSRule>
+			<AllowedOrigin>http://localhost:3333</AllowedOrigin>
+			<AllowedMethod>GET</AllowedMethod>
+			<AllowedMethod>HEAD</AllowedMethod>
+			<AllowedMethod>PUT</AllowedMethod>
+			<AllowedMethod>POST</AllowedMethod>
+			<AllowedHeader>content-length</AllowedHeader>
+			<AllowedHeader>content-type</AllowedHeader>
+		</CORSRule>
+	</CORSConfiguration>`)
+
+	app := fiber.New()
+	(&S3ApiRouter{}).Init(
+		app,
+		backendWithBucketCors{corsConfig: corsConfig},
+		&auth.IAMServiceInternal{},
+		nil,
+		nil,
+		nil,
+		nil,
+		false,
+		"us-east-1",
+		"",
+		middlewares.RootUserConfig{},
+		fallbackOrigin,
+	)
+
+	req, err := http.NewRequest(http.MethodOptions, "/xyz/upload/test.txt", nil)
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+	req.Header.Set("Origin", bucketOrigin)
+	req.Header.Set("Access-Control-Request-Method", "PUT")
+	req.Header.Set("Access-Control-Request-Headers", "content-type")
+
+	resp, err := app.Test(req)
+	if err != nil {
+		t.Fatalf("app.Test: %v", err)
+	}
+
+	// Verify the response has the correct status
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("expected status %d, got %d", http.StatusOK, resp.StatusCode)
+	}
+
+	// Check that headers are not duplicated
+	headers := resp.Header
+
+	// Helper function to count header occurrences
+	countHeader := func(headerName string) int {
+		values := headers.Values(headerName)
+		return len(values)
+	}
+
+	// Verify each CORS header appears exactly once
+	corsHeaders := []string{
+		"Access-Control-Allow-Origin",
+		"Access-Control-Allow-Methods",
+		"Access-Control-Allow-Headers",
+		"Access-Control-Allow-Credentials",
+		"Vary",
+	}
+
+	for _, header := range corsHeaders {
+		count := countHeader(header)
+		if count > 1 {
+			t.Errorf("Header %q appears %d times (expected 1). Values: %v",
+				header, count, headers.Values(header))
+		}
+	}
+
+	// Verify the bucket CORS values are used (not the fallback)
+	if got := resp.Header.Get("Access-Control-Allow-Origin"); got != bucketOrigin {
+		t.Errorf("expected Access-Control-Allow-Origin %q (from bucket CORS), got %q", bucketOrigin, got)
+	}
+
+	// Verify the requested method is in the allowed methods
+	allowedMethods := resp.Header.Get("Access-Control-Allow-Methods")
+	if allowedMethods == "" {
+		t.Error("Access-Control-Allow-Methods header is empty")
+	}
+
+	// Verify the requested header is in the allowed headers
+	allowedHeaders := resp.Header.Get("Access-Control-Allow-Headers")
+	if allowedHeaders == "" {
+		t.Error("Access-Control-Allow-Headers header is empty")
+	}
+}