Merge pull request #80 from pgsty/codex/issue-75-cors-hardening

fix: complete per-bucket CORS release hardening
This commit is contained in:
Feng Ruohang
2026-08-29 09:44:16 +08:00
committed by GitHub
14 changed files with 3133 additions and 146 deletions
+45 -19
View File
@@ -18,6 +18,8 @@
package cmd
import (
"context"
"errors"
"net"
"net/http"
"strconv"
@@ -31,6 +33,8 @@ import (
"github.com/rs/cors"
)
type bucketCorsAppliedKey struct{}
func newHTTPServerFn() *xhttp.Server {
globalObjLayerMutex.RLock()
defer globalObjLayerMutex.RUnlock()
@@ -651,12 +655,15 @@ func registerAPIRouter(router *mux.Router) {
// (request is complete). For an actual request it adds the applicable
// Access-Control-* response headers and returns false so the request
// continues down the handler chain. If no rule matches a preflight it writes
// 403 and returns true.
// 403 and returns true. A matched actual request is marked in its context so
// inner legacy middleware does not rewrite an explicitly allowed null origin.
func applyBucketCors(w http.ResponseWriter, r *http.Request, cfg *bktcors.Config) (handled bool) {
origin := r.Header.Get("Origin")
if origin == "" {
return false // not a CORS request
}
h := w.Header()
h.Add("Vary", "Origin")
isPreflight := r.Method == http.MethodOptions &&
r.Header.Get("Access-Control-Request-Method") != ""
@@ -664,45 +671,58 @@ func applyBucketCors(w http.ResponseWriter, r *http.Request, cfg *bktcors.Config
if isPreflight {
method := r.Header.Get("Access-Control-Request-Method")
reqHeaders := splitAndTrim(r.Header.Get("Access-Control-Request-Headers"))
rule, allowedHeaders, ok := cfg.MatchPreflight(origin, method, reqHeaders)
// A preflight response depends on all three request headers that
// determine the outcome, including when the request is rejected.
h.Add("Vary", "Access-Control-Request-Method")
h.Add("Vary", "Access-Control-Request-Headers")
rule, allowedOrigin, allowedHeaders, maxAgeSeconds, ok := cfg.MatchPreflight(origin, method, reqHeaders)
if !ok {
writeResponse(w, http.StatusForbidden, nil, mimeNone)
return true
}
h := w.Header()
h.Set("Access-Control-Allow-Origin", origin)
h.Set("Access-Control-Allow-Methods", method)
setBucketCorsOriginHeaders(h, allowedOrigin, origin)
h.Set("Access-Control-Allow-Methods", strings.Join(rule.AllowedMethods, ", "))
if len(allowedHeaders) > 0 {
h.Set("Access-Control-Allow-Headers", strings.Join(allowedHeaders, ", "))
}
if rule.MaxAgeSeconds > 0 {
h.Set("Access-Control-Max-Age", strconv.Itoa(rule.MaxAgeSeconds))
if len(rule.ExposeHeaders) > 0 {
h.Set("Access-Control-Expose-Headers", strings.Join(rule.ExposeHeaders, ", "))
}
if maxAgeSeconds != nil {
h.Set("Access-Control-Max-Age", strconv.Itoa(*maxAgeSeconds))
}
h.Set("Access-Control-Allow-Credentials", "true")
// A preflight response depends on all three request headers that
// determine the outcome, so cache variation must key on each of them.
h.Add("Vary", "Origin")
h.Add("Vary", "Access-Control-Request-Method")
h.Add("Vary", "Access-Control-Request-Headers")
writeResponse(w, http.StatusOK, nil, mimeNone)
return true
}
// Actual request: attach headers if the origin+method match.
rule, ok := cfg.MatchRule(origin, r.Method)
rule, allowedOrigin, ok := cfg.MatchRule(origin, r.Method)
if !ok {
return false // no matching rule → no CORS headers, continue normally
}
h := w.Header()
h.Set("Access-Control-Allow-Origin", origin)
h.Set("Access-Control-Allow-Credentials", "true")
*r = *r.WithContext(context.WithValue(r.Context(), bucketCorsAppliedKey{}, struct{}{}))
setBucketCorsOriginHeaders(h, allowedOrigin, origin)
if len(rule.ExposeHeaders) > 0 {
h.Set("Access-Control-Expose-Headers", strings.Join(rule.ExposeHeaders, ", "))
}
h.Add("Vary", "Origin")
return false
}
func bucketCorsWasApplied(r *http.Request) bool {
_, ok := r.Context().Value(bucketCorsAppliedKey{}).(struct{})
return ok
}
func setBucketCorsOriginHeaders(h http.Header, allowedOrigin, requestOrigin string) {
if allowedOrigin == "*" {
h.Set("Access-Control-Allow-Origin", "*")
h.Del("Access-Control-Allow-Credentials")
return
}
h.Set("Access-Control-Allow-Origin", requestOrigin)
h.Set("Access-Control-Allow-Credentials", "true")
}
// splitAndTrim splits a comma-separated header list into trimmed, non-empty values.
func splitAndTrim(s string) []string {
if s == "" {
@@ -766,13 +786,19 @@ func corsHandler(handler http.Handler) http.Handler {
globalCors := cors.New(opts).Handler(handler)
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if bucket, _ := request2BucketObjectName(r); bucket != "" && globalBucketMetadataSys != nil {
if cfg, _, err := globalBucketMetadataSys.GetCorsConfig(bucket); err == nil && cfg != nil {
cfg, _, err := globalBucketMetadataSys.GetCorsConfig(bucket)
if err == nil && cfg != nil {
if applyBucketCors(w, r, cfg) {
return
}
handler.ServeHTTP(w, r)
return
}
if err != nil && !errors.Is(err, errConfigNotFound) && r.Header.Get("Origin") != "" {
internalLogOnceIf(r.Context(), err, "bucket-cors-metadata")
handler.ServeHTTP(w, r)
return
}
}
globalCors.ServeHTTP(w, r)
})
+235
View File
@@ -0,0 +1,235 @@
// Copyright (c) 2015-2021 MinIO, Inc.
//
// This file is part of MinIO Object Storage stack
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
package cmd
import (
"bytes"
"encoding/base64"
"encoding/binary"
"hash/crc32"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/minio/minio/internal/auth"
)
func TestPutBucketCorsWireValidation(t *testing.T) {
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
t: t,
objAPITest: testPutBucketCorsWireValidation,
endpoints: []string{"PutBucketCors"},
})
}
func testPutBucketCorsWireValidation(_ ObjectLayer, _ string, bucketName string, apiRouter http.Handler,
creds auth.Credentials, t *testing.T,
) {
valid := `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`
rule := `<CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule>`
tests := []struct {
name string
body string
want int
wantCode string
}{
{
name: "second XML root",
body: valid + `<Extra/>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "255 Unicode character ID",
body: `<CORSConfiguration><CORSRule><ID>` + strings.Repeat("界", 255) + `</ID><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
want: http.StatusOK,
},
{
name: "256 Unicode character ID",
body: `<CORSConfiguration><CORSRule><ID>` + strings.Repeat("界", 256) + `</ID><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "lowercase method",
body: `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>get</AllowedMethod></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "empty origin",
body: `<CORSConfiguration><CORSRule><AllowedOrigin/><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "question mark origin wildcard",
body: `<CORSConfiguration><CORSRule><AllowedOrigin>https://?.example.com</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "question mark header wildcard",
body: `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><AllowedHeader>x-amz-?</AllowedHeader></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "unknown element",
body: `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><Unknown/></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "empty max age",
body: `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><MaxAgeSeconds/></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "zero max age",
body: `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><MaxAgeSeconds>0</MaxAgeSeconds></CORSRule></CORSConfiguration>`,
want: http.StatusOK,
},
{
name: "max age int32 overflow",
body: `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><MaxAgeSeconds>2147483648</MaxAgeSeconds></CORSRule></CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "100 rules",
body: `<CORSConfiguration>` + strings.Repeat(rule, 100) + `</CORSConfiguration>`,
want: http.StatusOK,
},
{
name: "101 rules",
body: `<CORSConfiguration>` + strings.Repeat(rule, 101) + `</CORSConfiguration>`,
want: http.StatusBadRequest,
wantCode: "MalformedXML",
},
{
name: "exactly 64 KiB",
body: sizedCORSConfig(maxBucketCorsSize),
want: http.StatusOK,
},
{
name: "over 64 KiB",
body: sizedCORSConfig(maxBucketCorsSize + 1),
want: http.StatusBadRequest,
wantCode: "EntityTooLarge",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req, err := newTestSignedRequestV4(http.MethodPut, getBucketCorsURL("", bucketName),
int64(len(tt.body)), bytes.NewReader([]byte(tt.body)), creds.AccessKey, creds.SecretKey, nil)
if err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
if rec.Code != tt.want {
t.Fatalf("expected status %d, got %d: %s", tt.want, rec.Code, rec.Body.String())
}
if tt.wantCode != "" && !bytes.Contains(rec.Body.Bytes(), []byte("<Code>"+tt.wantCode+"</Code>")) {
t.Fatalf("expected error code %s, got: %s", tt.wantCode, rec.Body.String())
}
})
}
}
func sizedCORSConfig(size int) string {
prefix := `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod>`
suffix := `</CORSRule></CORSConfiguration>`
return prefix + strings.Repeat(" ", size-len(prefix)-len(suffix)) + suffix
}
func TestPutBucketCorsChecksumValidation(t *testing.T) {
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
t: t,
objAPITest: testPutBucketCorsChecksumValidation,
endpoints: []string{"PutBucketCors"},
})
}
func testPutBucketCorsChecksumValidation(_ ObjectLayer, _ string, bucketName string, apiRouter http.Handler,
creds auth.Credentials, t *testing.T,
) {
body := []byte(`<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`)
tests := []struct {
name string
configure func(*http.Request)
want int
wantCode string
}{
{
name: "missing checksum",
configure: func(req *http.Request) {
req.Header.Del("Content-Md5")
},
want: http.StatusBadRequest,
wantCode: "MissingContentMD5",
},
{
name: "bad content md5",
configure: func(req *http.Request) {
req.Header.Set("Content-Md5", getMD5HashBase64([]byte("different body")))
},
want: http.StatusBadRequest,
wantCode: "BadDigest",
},
{
name: "valid sdk crc32",
configure: func(req *http.Request) {
req.Header.Del("Content-Md5")
req.Header.Set("X-Amz-Sdk-Checksum-Algorithm", "CRC32")
req.Header.Set("X-Amz-Checksum-Crc32", corsCRC32Base64(body))
},
want: http.StatusOK,
},
{
name: "bad sdk crc32",
configure: func(req *http.Request) {
req.Header.Del("Content-Md5")
req.Header.Set("X-Amz-Sdk-Checksum-Algorithm", "CRC32")
req.Header.Set("X-Amz-Checksum-Crc32", corsCRC32Base64([]byte("different body")))
},
want: http.StatusBadRequest,
wantCode: "BadDigest",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req, err := newTestRequest(http.MethodPut, getBucketCorsURL("", bucketName), int64(len(body)), bytes.NewReader(body))
if err != nil {
t.Fatal(err)
}
tt.configure(req)
if err = signRequestV4(req, creds.AccessKey, creds.SecretKey); err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
if rec.Code != tt.want || (tt.wantCode != "" && !bytes.Contains(rec.Body.Bytes(), []byte("<Code>"+tt.wantCode+"</Code>"))) {
t.Fatalf("expected status %d and code %s, got %d: %s", tt.want, tt.wantCode, rec.Code, rec.Body.String())
}
})
}
}
func corsCRC32Base64(data []byte) string {
var checksum [4]byte
binary.BigEndian.PutUint32(checksum[:], crc32.ChecksumIEEE(data))
return base64.StdEncoding.EncodeToString(checksum[:])
}
+11 -6
View File
@@ -27,6 +27,7 @@ import (
humanize "github.com/dustin/go-humanize"
"github.com/minio/madmin-go/v3"
"github.com/minio/minio/internal/bucket/cors"
hashpkg "github.com/minio/minio/internal/hash"
"github.com/minio/minio/internal/logger"
"github.com/minio/mux"
"github.com/minio/pkg/v3/policy"
@@ -69,16 +70,20 @@ func (api objectAPIHandlers) PutBucketCorsHandler(w http.ResponseWriter, r *http
return
}
// PutBucketCors requires a Content-Md5 (or a supported trailing/full
// checksum). validateLengthAndChecksum wraps r.Body so the supplied
// digest is verified as the body is read below.
// PutBucketCors requires a Content-Md5 or a supported full-header
// checksum. validateLengthAndChecksum wraps r.Body so the supplied digest
// is verified as the body is read below.
if !validateLengthAndChecksum(r) {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingContentMD5), r.URL)
return
}
corsBytes, err := io.ReadAll(io.LimitReader(r.Body, r.ContentLength))
corsBytes, err := io.ReadAll(r.Body)
if err != nil {
if errors.Is(err, hashpkg.ErrInvalidChecksum) {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadDigest), r.URL)
return
}
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
}
@@ -97,7 +102,7 @@ func (api objectAPIHandlers) PutBucketCorsHandler(w http.ResponseWriter, r *http
return
}
updatedAt, err := globalBucketMetadataSys.Update(ctx, bucket, bucketCorsConfig, corsBytes)
updatedAt, err := updateLocalBucketCORSMetadata(ctx, objAPI, bucket, corsBytes)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
@@ -181,7 +186,7 @@ func (api objectAPIHandlers) DeleteBucketCorsHandler(w http.ResponseWriter, r *h
return
}
updatedAt, err := globalBucketMetadataSys.Delete(ctx, bucket, bucketCorsConfig)
updatedAt, err := updateLocalBucketCORSMetadata(ctx, objAPI, bucket, nil)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
+32
View File
@@ -98,6 +98,38 @@ func testBucketCorsHandlers(obj ObjectLayer, instanceType, bucketName string, ap
t.Fatalf("PUT malformed cors: expected 400, got %d", rec.Code)
}
// Missing Content-MD5 is rejected before the body is parsed.
req, err = newTestRequest(http.MethodPut, getBucketCorsURL("", bucketName),
int64(len(testCORSDoc)), bytes.NewReader([]byte(testCORSDoc)))
if err != nil {
t.Fatal(err)
}
req.Header.Del("Content-Md5")
if err = signRequestV4(req, creds.AccessKey, creds.SecretKey); err != nil {
t.Fatal(err)
}
rec = httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
if rec.Code != http.StatusBadRequest || !bytes.Contains(rec.Body.Bytes(), []byte("<Code>MissingContentMD5</Code>")) {
t.Fatalf("PUT cors without Content-MD5: expected MissingContentMD5, got %d: %s", rec.Code, rec.Body.String())
}
// A signed but incorrect Content-MD5 is rejected while reading the body.
req, err = newTestRequest(http.MethodPut, getBucketCorsURL("", bucketName),
int64(len(testCORSDoc)), bytes.NewReader([]byte(testCORSDoc)))
if err != nil {
t.Fatal(err)
}
req.Header.Set("Content-Md5", getMD5HashBase64([]byte("different body")))
if err = signRequestV4(req, creds.AccessKey, creds.SecretKey); err != nil {
t.Fatal(err)
}
rec = httptest.NewRecorder()
apiRouter.ServeHTTP(rec, req)
if rec.Code != http.StatusBadRequest || !bytes.Contains(rec.Body.Bytes(), []byte("<Code>BadDigest</Code>")) {
t.Fatalf("PUT cors with bad Content-MD5: expected BadDigest, got %d: %s", rec.Code, rec.Body.String())
}
// 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),
+287 -1
View File
@@ -20,8 +20,10 @@ package cmd
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/minio/minio/internal/auth"
"github.com/minio/minio/internal/bucket/cors"
)
@@ -38,6 +40,7 @@ func TestPerBucketCorsPreflight(t *testing.T) {
req := httptest.NewRequest(http.MethodOptions, "/mybucket/obj", nil)
req.Header.Set("Origin", "http://example.com")
req.Header.Set("Access-Control-Request-Method", "GET")
req.Header.Set("Access-Control-Request-Headers", "X-Amz-Date")
handled := applyBucketCors(rec, req, cfg)
if !handled {
@@ -46,9 +49,44 @@ func TestPerBucketCorsPreflight(t *testing.T) {
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "http://example.com" {
t.Fatalf("allow-origin = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" {
t.Fatalf("allow-credentials = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Methods"); got != "GET, PUT" {
t.Fatalf("allow-methods = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Headers"); got != "X-Amz-Date" {
t.Fatalf("allow-headers = %q", got)
}
if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "ETag" {
t.Fatalf("expose-headers = %q", got)
}
if got := rec.Header().Get("Access-Control-Max-Age"); got != "3000" {
t.Fatalf("max-age = %q", got)
}
requireCorsVary(t, rec.Header())
if rec.Code != http.StatusOK {
t.Fatalf("preflight status = %d", rec.Code)
}
requireCorsOriginVary(t, rec.Header())
}
func TestPerBucketCorsActualRequestNoMatchVariesByOrigin(t *testing.T) {
cfg := &cors.Config{CORSRules: []cors.Rule{{
AllowedOrigins: []string{"https://allowed.example.com"},
AllowedMethods: []string{"GET"},
}}}
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/mybucket/obj", nil)
req.Header.Set("Origin", "https://denied.example.com")
if handled := applyBucketCors(rec, req, cfg); handled {
t.Fatal("actual request must continue when CORS does not match")
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
t.Fatalf("allow-origin = %q", got)
}
requireCorsOriginVary(t, rec.Header())
}
func TestPerBucketCorsPreflightNoMatch(t *testing.T) {
@@ -68,6 +106,84 @@ func TestPerBucketCorsPreflightNoMatch(t *testing.T) {
if rec.Code != http.StatusForbidden {
t.Fatalf("expected 403 for disallowed origin, got %d", rec.Code)
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
t.Fatalf("rejected preflight returned allow-origin %q", got)
}
requireCorsVary(t, rec.Header())
}
func TestPerBucketCorsPreflightWildcardOriginAndZeroMaxAge(t *testing.T) {
doc := `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><AllowedMethod>HEAD</AllowedMethod><AllowedHeader>*</AllowedHeader><ExposeHeader>ETag</ExposeHeader><MaxAgeSeconds>0</MaxAgeSeconds></CORSRule></CORSConfiguration>`
cfg, err := cors.ParseBucketCorsConfig(strings.NewReader(doc))
if err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodOptions, "/mybucket/obj", nil)
req.Header.Set("Origin", "https://app.example.com")
req.Header.Set("Access-Control-Request-Method", "GET")
req.Header.Set("Access-Control-Request-Headers", "RANGE")
if handled := applyBucketCors(rec, req, cfg); !handled {
t.Fatal("expected preflight to be handled")
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" {
t.Fatalf("allow-origin = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" {
t.Fatalf("allow-credentials = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Methods"); got != "GET, HEAD" {
t.Fatalf("allow-methods = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Headers"); got != "RANGE" {
t.Fatalf("allow-headers = %q", got)
}
if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "ETag" {
t.Fatalf("expose-headers = %q", got)
}
if got := rec.Header().Get("Access-Control-Max-Age"); got != "0" {
t.Fatalf("max-age = %q", got)
}
requireCorsVary(t, rec.Header())
}
func TestPerBucketCorsPreflightUsesFirstFullyMatchingRule(t *testing.T) {
cfg := &cors.Config{CORSRules: []cors.Rule{
{
AllowedOrigins: []string{"https://app.example.com"},
AllowedMethods: []string{"GET"},
AllowedHeaders: []string{"x-a"},
ExposeHeaders: []string{"x-rule-a"},
MaxAgeSeconds: 1,
},
{
AllowedOrigins: []string{"https://app.example.com"},
AllowedMethods: []string{"GET", "HEAD"},
AllowedHeaders: []string{"*"},
ExposeHeaders: []string{"x-rule-b"},
MaxAgeSeconds: 2,
},
}}
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodOptions, "/mybucket/obj", nil)
req.Header.Set("Origin", "https://app.example.com")
req.Header.Set("Access-Control-Request-Method", "GET")
req.Header.Set("Access-Control-Request-Headers", "X-B")
if handled := applyBucketCors(rec, req, cfg); !handled {
t.Fatal("expected preflight to be handled")
}
if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "x-rule-b" {
t.Fatalf("selected rule expose-headers = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Methods"); got != "GET, HEAD" {
t.Fatalf("selected rule allow-methods = %q", got)
}
if got := rec.Header().Get("Access-Control-Max-Age"); got != "2" {
t.Fatalf("selected rule max-age = %q", got)
}
}
func TestPerBucketCorsActualRequest(t *testing.T) {
@@ -84,10 +200,180 @@ func TestPerBucketCorsActualRequest(t *testing.T) {
if handled {
t.Fatal("actual (non-preflight) request must not be terminated by CORS")
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "http://any.com" {
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" {
t.Fatalf("allow-origin = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" {
t.Fatalf("allow-credentials = %q", got)
}
if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "ETag" {
t.Fatalf("expose-headers = %q", got)
}
}
func TestPerBucketCorsOriginPatternResponse(t *testing.T) {
cfg := &cors.Config{CORSRules: []cors.Rule{{
AllowedOrigins: []string{"https://app.example.com", "https://*", "*"},
AllowedMethods: []string{"GET"},
}}}
tests := []struct {
origin string
wantOrigin string
wantCredentials string
}{
{"https://app.example.com", "https://app.example.com", "true"},
{"https://other.example.com", "https://other.example.com", "true"},
{"http://other.example.com", "*", ""},
}
for _, tt := range tests {
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/mybucket/obj", nil)
req.Header.Set("Origin", tt.origin)
if handled := applyBucketCors(rec, req, cfg); handled {
t.Fatal("actual request must not be terminated by CORS")
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != tt.wantOrigin {
t.Fatalf("origin %q: allow-origin = %q, want %q", tt.origin, got, tt.wantOrigin)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != tt.wantCredentials {
t.Fatalf("origin %q: allow-credentials = %q, want %q", tt.origin, got, tt.wantCredentials)
}
}
}
func TestBucketCorsMetadataErrorFailsClosed(t *testing.T) {
oldObjectAPI := newObjectLayerFn()
setObjectLayer(nil)
defer setObjectLayer(oldObjectAPI)
wrapped := corsHandler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNoContent)
}))
for _, method := range []string{http.MethodGet, http.MethodOptions} {
rec := httptest.NewRecorder()
req := httptest.NewRequest(method, getGetObjectURL("", "cors-metadata-error", "object"), nil)
req.Header.Set("Origin", "https://app.example.com")
if method == http.MethodOptions {
req.Header.Set("Access-Control-Request-Method", http.MethodGet)
}
wrapped.ServeHTTP(rec, req)
if rec.Code != http.StatusNoContent {
t.Fatalf("%s status = %d, want %d", method, rec.Code, http.StatusNoContent)
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
t.Fatalf("%s metadata error fell back to global allow-origin %q", method, got)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" {
t.Fatalf("%s metadata error fell back to global credentials %q", method, got)
}
}
}
func TestBucketCorsNoConfigUsesGlobalFallback(t *testing.T) {
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
t: t,
objAPITest: testBucketCorsNoConfigUsesGlobalFallback,
endpoints: []string{"GetBucketCors"},
})
}
func testBucketCorsNoConfigUsesGlobalFallback(_ ObjectLayer, _ string, bucket string, _ http.Handler, _ auth.Credentials, t *testing.T) {
wrapped := corsHandler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNoContent)
}))
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, getGetObjectURL("", bucket, "object"), nil)
req.Header.Set("Origin", "https://app.example.com")
wrapped.ServeHTTP(rec, req)
if rec.Code != http.StatusNoContent {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusNoContent)
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://app.example.com" {
t.Fatalf("allow-origin = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" {
t.Fatalf("allow-credentials = %q", got)
}
}
func TestPerBucketCorsActualPatternOriginSupportsCredentials(t *testing.T) {
cfg := &cors.Config{CORSRules: []cors.Rule{{
AllowedOrigins: []string{"https://*.example.com"},
AllowedMethods: []string{"GET"},
}}}
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/mybucket/obj", nil)
req.Header.Set("Origin", "https://app.example.com")
if handled := applyBucketCors(rec, req, cfg); handled {
t.Fatal("actual request must continue")
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://app.example.com" {
t.Fatalf("allow-origin = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" {
t.Fatalf("allow-credentials = %q", got)
}
}
func TestPerBucketCorsActualNullOriginSurvivesForwardingMiddleware(t *testing.T) {
next := setBucketForwardingMiddleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNoContent)
}))
t.Run("per-bucket null origin", func(t *testing.T) {
cfg := &cors.Config{CORSRules: []cors.Rule{{
AllowedOrigins: []string{"null"},
AllowedMethods: []string{"GET"},
}}}
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/mybucket/obj", nil)
req.Header.Set("Origin", "null")
if handled := applyBucketCors(rec, req, cfg); handled {
t.Fatal("actual request must continue")
}
next.ServeHTTP(rec, req)
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "null" {
t.Fatalf("allow-origin = %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" {
t.Fatalf("allow-credentials = %q", got)
}
})
t.Run("legacy unmarked null origin", func(t *testing.T) {
rec := httptest.NewRecorder()
rec.Header().Set("Access-Control-Allow-Origin", "null")
req := httptest.NewRequest(http.MethodGet, "/mybucket/obj", nil)
next.ServeHTTP(rec, req)
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" {
t.Fatalf("allow-origin = %q", got)
}
})
}
func requireCorsVary(t *testing.T, header http.Header) {
t.Helper()
values := strings.Join(header.Values("Vary"), ",")
for _, want := range []string{"Origin", "Access-Control-Request-Method", "Access-Control-Request-Headers"} {
if !strings.Contains(values, want) {
t.Fatalf("Vary = %q, missing %q", values, want)
}
}
}
func requireCorsOriginVary(t *testing.T, header http.Header) {
t.Helper()
if values := strings.Join(header.Values("Vary"), ","); !strings.Contains(values, "Origin") {
t.Fatalf("Vary = %q, missing Origin", values)
}
}
File diff suppressed because it is too large Load Diff
+6
View File
@@ -370,6 +370,9 @@ func (sys *BucketMetadataSys) GetCorsConfig(bucket string) (*cors.Config, time.T
if err != nil {
return nil, time.Time{}, err
}
if meta.corsConfigErr != nil {
return nil, meta.CorsConfigUpdatedAt, meta.corsConfigErr
}
if meta.corsConfig == nil {
return nil, time.Time{}, errConfigNotFound
}
@@ -384,6 +387,9 @@ func (sys *BucketMetadataSys) GetCorsConfigXML(bucket string) ([]byte, time.Time
if err != nil {
return nil, time.Time{}, err
}
if meta.corsConfigErr != nil {
return nil, meta.CorsConfigUpdatedAt, meta.corsConfigErr
}
if len(meta.CorsConfigXML) == 0 {
return nil, time.Time{}, errConfigNotFound
}
+20 -3
View File
@@ -113,6 +113,7 @@ type BucketMetadata struct {
bucketTargetConfig *madmin.BucketTargets
bucketTargetConfigMeta map[string]string
corsConfig *cors.Config
corsConfigErr error
}
// newBucketMetadata creates BucketMetadata with the supplied name and Created to Now.
@@ -261,6 +262,12 @@ func loadBucketMetadataParse(ctx context.Context, objectAPI ObjectLayer, bucket
return b, err
}
}
if b.corsConfigErr != nil {
// Keep the rest of the bucket metadata available so an operator can
// replace or delete a CORS document accepted by an older, more lenient
// build. Defer unrelated metadata migration until CORS is repaired.
return b, nil
}
// migrate unencrypted remote targets
if err = b.migrateTargetConfig(ctx, objectAPI); err != nil {
@@ -320,10 +327,17 @@ func (b *BucketMetadata) parseAllConfigs(ctx context.Context, objectAPI ObjectLa
b.taggingConfig = nil
}
b.corsConfigErr = nil
if len(b.CorsConfigXML) != 0 {
b.corsConfig, err = cors.ParseBucketCorsConfig(bytes.NewReader(b.CorsConfigXML))
if err != nil {
return err
cfg, corsErr := cors.ParseBucketCorsConfig(bytes.NewReader(b.CorsConfigXML))
if corsErr == nil {
corsErr = cfg.Validate()
}
if corsErr != nil {
b.corsConfig = nil
b.corsConfigErr = fmt.Errorf("invalid bucket CORS configuration: %w", corsErr)
} else {
b.corsConfig = cfg
}
} else {
b.corsConfig = nil
@@ -522,6 +536,9 @@ func (b *BucketMetadata) Save(ctx context.Context, api ObjectLayer) error {
if err := b.parseAllConfigs(ctx, api); err != nil {
return err
}
if b.corsConfigErr != nil {
return b.corsConfigErr
}
data := make([]byte, 4, b.Msgsize()+4)
+3 -2
View File
@@ -476,9 +476,10 @@ func setRequestValidityMiddleware(h http.Handler) http.Handler {
// is obtained from centralized etcd configuration service.
func setBucketForwardingMiddleware(h http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if origin := w.Header().Get("Access-Control-Allow-Origin"); origin == "null" {
if origin := w.Header().Get("Access-Control-Allow-Origin"); origin == "null" && !bucketCorsWasApplied(r) {
// This is a workaround change to ensure that "Origin: null"
// incoming request to a response back as "*" instead of "null"
// incoming request to a response back as "*" instead of "null".
// Per-bucket CORS preserves an explicitly allowed "null" origin.
w.Header().Set("Access-Control-Allow-Origin", "*")
}
if globalDNSConfig == nil || !globalBucketFederation ||
+272 -88
View File
@@ -43,6 +43,7 @@ import (
"github.com/minio/minio-go/v7/pkg/replication"
"github.com/minio/minio-go/v7/pkg/set"
"github.com/minio/minio/internal/auth"
"github.com/minio/minio/internal/bucket/cors"
"github.com/minio/minio/internal/bucket/lifecycle"
sreplication "github.com/minio/minio/internal/bucket/replication"
"github.com/minio/minio/internal/logger"
@@ -1577,12 +1578,33 @@ func (c *SiteReplicationSys) PeerBucketMetadataUpdateHandler(ctx context.Context
return wrapSRErr(errInvalidArgument)
}
var corsConfigData []byte
if item.Cors != nil {
var err error
corsConfigData, err = decodeCORSReplicationPayload(item.Cors)
if err != nil {
return wrapSRErr(err)
}
if err = validateCORSReplicationPayload(corsConfigData); err != nil {
return wrapSRErr(err)
}
var unlock func()
ctx, unlock, err = lockBucketCORSMetadata(ctx, objectAPI, item.Bucket)
if err != nil {
return wrapSRErr(err)
}
defer unlock()
}
meta, err := readBucketMetadata(ctx, objectAPI, item.Bucket)
if err != nil {
return wrapSRErr(err)
}
if meta.Created.After(item.UpdatedAt) {
if item.Cors != nil {
replLogOnceIf(ctx, fmt.Errorf("ignoring CORS event for bucket %s from %v before bucket creation at %v", item.Bucket, item.UpdatedAt, meta.Created), "cors-event-before-bucket-creation-"+item.Bucket)
}
return nil
}
@@ -1633,12 +1655,12 @@ func (c *SiteReplicationSys) PeerBucketMetadataUpdateHandler(ctx context.Context
}
if item.Cors != nil {
configData, err := base64.StdEncoding.DecodeString(*item.Cors)
if err != nil {
return wrapSRErr(err)
localState := newCORSReplicationState(meta.CorsConfigXML, meta.CorsConfigUpdatedAt)
incoming := newCORSReplicationState(corsConfigData, item.UpdatedAt)
if compareCORSReplicationStates(localState, incoming) < 0 {
meta.CorsConfigXML = bytes.Clone(corsConfigData)
meta.CorsConfigUpdatedAt = item.UpdatedAt
}
meta.CorsConfigXML = configData
meta.CorsConfigUpdatedAt = item.UpdatedAt
}
return globalBucketMetadataSys.save(ctx, meta)
@@ -1758,30 +1780,222 @@ func (c *SiteReplicationSys) PeerBucketSSEConfigHandler(ctx context.Context, buc
return nil
}
// PeerBucketCorsConfigHandler - copies/deletes CORS config to local cluster.
func (c *SiteReplicationSys) PeerBucketCorsConfigHandler(ctx context.Context, bucket string, corsConfig *string, updatedAt time.Time) error {
// skip overwrite if local update is newer than peer update.
if !updatedAt.IsZero() {
if _, updateTm, err := globalBucketMetadataSys.GetCorsConfig(bucket); err == nil && updateTm.After(updatedAt) {
return nil
}
}
type corsReplicationStateKind uint8
if corsConfig != nil {
configData, err := base64.StdEncoding.DecodeString(*corsConfig)
if err != nil {
return wrapSRErr(err)
}
_, err = globalBucketMetadataSys.Update(ctx, bucket, bucketCorsConfig, configData)
if err != nil {
return wrapSRErr(err)
}
const (
corsReplicationBaseline corsReplicationStateKind = iota
corsReplicationLive
corsReplicationTombstone
)
type corsReplicationState struct {
kind corsReplicationStateKind
payload []byte
updatedAt time.Time
}
func newCORSReplicationState(payload []byte, updatedAt time.Time) corsReplicationState {
state := corsReplicationState{updatedAt: updatedAt.UTC()}
switch {
case len(payload) > 0:
state.kind = corsReplicationLive
state.payload = bytes.Clone(payload)
case updatedAt.IsZero():
state.kind = corsReplicationBaseline
default:
state.kind = corsReplicationTombstone
}
return state
}
func compareCORSReplicationStates(a, b corsReplicationState) int {
switch {
case a.updatedAt.Before(b.updatedAt):
return -1
case a.updatedAt.After(b.updatedAt):
return 1
case a.kind < b.kind:
return -1
case a.kind > b.kind:
return 1
case a.kind == corsReplicationLive:
return bytes.Compare(a.payload, b.payload)
default:
return 0
}
}
func equalCORSReplicationStates(a, b corsReplicationState) bool {
return compareCORSReplicationStates(a, b) == 0
}
func decodeCORSReplicationPayload(encoded *string) ([]byte, error) {
if encoded == nil {
return nil, nil
}
payload, err := base64.StdEncoding.Strict().DecodeString(*encoded)
if err != nil {
return nil, fmt.Errorf("invalid CORS replication payload: %w", err)
}
if len(payload) == 0 || base64.StdEncoding.EncodeToString(payload) != *encoded {
return nil, fmt.Errorf("invalid CORS replication payload: %w", errInvalidArgument)
}
return payload, nil
}
func validateCORSReplicationPayload(payload []byte) error {
if payload == nil {
return nil
}
// Delete cors config
_, err := globalBucketMetadataSys.Delete(ctx, bucket, bucketCorsConfig)
config, err := cors.ParseBucketCorsConfig(bytes.NewReader(payload))
if err != nil {
return fmt.Errorf("invalid CORS replication payload: %w", errInvalidArgument)
}
if err = config.Validate(); err != nil {
return fmt.Errorf("invalid CORS replication payload: %w: %v", errInvalidArgument, err)
}
return nil
}
func corsReplicationStateFromInfo(info madmin.SRBucketInfo) (corsReplicationState, error) {
payload, err := decodeCORSReplicationPayload(info.CorsConfig)
if err != nil {
return corsReplicationState{}, err
}
if err = validateCORSReplicationPayload(payload); err != nil {
return corsReplicationState{}, err
}
if info.CorsConfig != nil && info.CorsConfigUpdatedAt.IsZero() {
return corsReplicationState{}, fmt.Errorf("live CORS replication payload has no source timestamp: %w", errInvalidArgument)
}
return newCORSReplicationState(payload, info.CorsConfigUpdatedAt), nil
}
func areCORSReplicationStatesEqual(sites []srBucketMetaInfo) bool {
if len(sites) == 0 {
return true
}
reference, err := corsReplicationStateFromInfo(sites[0].SRBucketInfo)
if err != nil {
return false
}
for _, site := range sites[1:] {
state, err := corsReplicationStateFromInfo(site.SRBucketInfo)
if err != nil || !equalCORSReplicationStates(reference, state) {
return false
}
}
return true
}
func (s corsReplicationState) encodedPayload() *string {
if s.kind != corsReplicationLive {
return nil
}
encoded := base64.StdEncoding.EncodeToString(s.payload)
return &encoded
}
func newBucketCORSReplicationEvent(bucket string, meta BucketMetadata) (madmin.SRBucketMeta, bool) {
if meta.CorsConfigUpdatedAt.IsZero() {
return madmin.SRBucketMeta{}, false
}
return madmin.SRBucketMeta{
Type: madmin.SRBucketMetaTypeCorsConfig,
Bucket: bucket,
Cors: newCORSReplicationState(meta.CorsConfigXML, meta.CorsConfigUpdatedAt).encodedPayload(),
UpdatedAt: meta.CorsConfigUpdatedAt,
}, true
}
func lockBucketCORSMetadata(ctx context.Context, objectAPI ObjectLayer, bucket string) (context.Context, func(), error) {
// The lock name is deliberately different from .metadata.bin. Saving the
// metadata locks that object internally, and namespace locks are not
// re-entrant.
lock := objectAPI.NewNSLock(minioMetaBucket, pathJoin(bucketMetaPrefix, bucket, "cors-config.lock"))
lkctx, err := lock.GetLock(ctx, globalOperationTimeout)
if err != nil {
return nil, nil, err
}
return lkctx.Context(), func() { lock.Unlock(lkctx) }, nil
}
func updateLocalBucketCORSMetadata(ctx context.Context, objectAPI ObjectLayer, bucket string, configData []byte) (time.Time, error) {
return applyBucketCORSMetadata(ctx, objectAPI, bucket, configData, time.Time{}, true)
}
func applyBucketCORSMetadata(ctx context.Context, objectAPI ObjectLayer, bucket string, configData []byte, sourceUpdatedAt time.Time, local bool) (time.Time, error) {
if bucket == "" || (configData != nil && len(configData) == 0) {
return time.Time{}, errInvalidArgument
}
if err := validateCORSReplicationPayload(configData); err != nil {
return time.Time{}, err
}
ctx, unlock, err := lockBucketCORSMetadata(ctx, objectAPI, bucket)
if err != nil {
return time.Time{}, err
}
defer unlock()
var meta BucketMetadata
if local {
meta, err = loadBucketMetadataParse(ctx, objectAPI, bucket, true)
} else {
meta, err = readBucketMetadata(ctx, objectAPI, bucket)
}
if err != nil {
return time.Time{}, err
}
localState := newCORSReplicationState(meta.CorsConfigXML, meta.CorsConfigUpdatedAt)
updatedAt := sourceUpdatedAt.UTC()
if local {
updatedAt = UTCNow()
floor := meta.Created
if localState.updatedAt.After(floor) {
floor = localState.updatedAt
}
if !updatedAt.After(floor) {
updatedAt = floor.Add(time.Nanosecond)
}
} else {
// CreatedAt is the bucket-lineage floor: an event from an older
// incarnation of the bucket must not change the current one.
if updatedAt.Before(meta.Created) {
replLogOnceIf(ctx, fmt.Errorf("ignoring CORS event for bucket %s from %v before bucket creation at %v", bucket, updatedAt, meta.Created), "cors-event-before-bucket-creation-"+bucket)
return localState.updatedAt, nil
}
incoming := newCORSReplicationState(configData, updatedAt)
if compareCORSReplicationStates(localState, incoming) >= 0 {
return localState.updatedAt, nil
}
}
meta.CorsConfigXML = bytes.Clone(configData)
meta.CorsConfigUpdatedAt = updatedAt
if err = globalBucketMetadataSys.save(ctx, meta); err != nil {
return time.Time{}, err
}
return updatedAt, nil
}
// PeerBucketCorsConfigHandler - copies/deletes CORS config to local cluster.
func (c *SiteReplicationSys) PeerBucketCorsConfigHandler(ctx context.Context, bucket string, corsConfig *string, updatedAt time.Time) error {
objectAPI := newObjectLayerFn()
if objectAPI == nil {
return errSRObjectLayerNotReady
}
if bucket == "" || updatedAt.IsZero() {
return wrapSRErr(errInvalidArgument)
}
configData, err := decodeCORSReplicationPayload(corsConfig)
if err != nil {
return wrapSRErr(err)
}
if _, err = applyBucketCORSMetadata(ctx, objectAPI, bucket, configData, updatedAt, false); err != nil {
return wrapSRErr(err)
}
return nil
@@ -1989,15 +2203,8 @@ func (c *SiteReplicationSys) syncToAllPeers(ctx context.Context, addOpts madmin.
}
// Replicate existing bucket CORS settings
corsConfigData, tm := meta.CorsConfigXML, meta.CorsConfigUpdatedAt
if len(corsConfigData) > 0 {
corsConfigStr := base64.StdEncoding.EncodeToString(corsConfigData)
err = c.BucketMetaHook(ctx, madmin.SRBucketMeta{
Type: madmin.SRBucketMetaTypeCorsConfig,
Bucket: bucket,
Cors: &corsConfigStr,
UpdatedAt: tm,
})
if corsEvent, ok := newBucketCORSReplicationEvent(bucket, meta); ok {
err = c.BucketMetaHook(ctx, corsEvent)
if err != nil {
return errSRBucketMetaError(err)
}
@@ -3198,7 +3405,6 @@ func (c *SiteReplicationSys) siteReplicationStatus(ctx context.Context, objAPI O
replCfgs := make([]*sreplication.Config, numSites)
quotaCfgs := make([]*madmin.BucketQuota, numSites)
sseCfgSet := set.NewStringSet()
corsCfgSet := set.NewStringSet()
versionCfgSet := set.NewStringSet()
var tagCount, olockCfgCount, sseCfgCount, corsCfgCount, versionCfgCount int
for i, s := range slc {
@@ -3272,13 +3478,8 @@ func (c *SiteReplicationSys) siteReplicationStatus(ctx context.Context, objAPI O
}
}
if s.CorsConfig != nil {
configData, err := base64.StdEncoding.DecodeString(*s.CorsConfig)
if err != nil {
continue
}
corsCfgCount++
if !corsCfgSet.Contains(string(configData)) {
corsCfgSet.Add(string(configData))
if _, err := decodeCORSReplicationPayload(s.CorsConfig); err == nil {
corsCfgCount++
}
}
ss, ok := info.StatsSummary[s.DeploymentID]
@@ -3299,7 +3500,7 @@ func (c *SiteReplicationSys) siteReplicationStatus(ctx context.Context, objAPI O
if sseCfgCount > 0 {
ss.TotalSSEConfigCount++
}
if corsCfgCount > 0 {
if s.CorsConfig != nil {
ss.TotalCorsConfigCount++
}
if versionCfgCount > 0 {
@@ -3313,7 +3514,7 @@ func (c *SiteReplicationSys) siteReplicationStatus(ctx context.Context, objAPI O
tagMismatch := !isReplicated(tagCount, numSites, tagSet)
olockCfgMismatch := !isReplicated(olockCfgCount, numSites, olockConfigSet)
sseCfgMismatch := !isReplicated(sseCfgCount, numSites, sseCfgSet)
corsCfgMismatch := !isReplicated(corsCfgCount, numSites, corsCfgSet)
corsCfgMismatch := !areCORSReplicationStatesEqual(slc)
versionCfgMismatch := !isReplicated(versionCfgCount, numSites, versionCfgSet)
policyMismatch := !isBktPolicyReplicated(numSites, policies)
replCfgMismatch := !isBktReplCfgReplicated(numSites, replCfgs)
@@ -3783,10 +3984,10 @@ func (c *SiteReplicationSys) SiteReplicationMetaInfo(ctx context.Context, objAPI
bms.SSEConfigUpdatedAt = meta.EncryptionConfigUpdatedAt
}
bms.CorsConfigUpdatedAt = meta.CorsConfigUpdatedAt
if len(meta.CorsConfigXML) > 0 {
corsConfigStr := base64.StdEncoding.EncodeToString(meta.CorsConfigXML)
bms.CorsConfig = &corsConfigStr
bms.CorsConfigUpdatedAt = meta.CorsConfigUpdatedAt
}
if len(meta.ReplicationConfigXML) > 0 {
@@ -4997,62 +5198,45 @@ func (c *SiteReplicationSys) healSSEMetadata(ctx context.Context, objAPI ObjectL
return nil
}
func latestCORSConfig(bs map[string]srBucketStatsSummary) (latestID string, latest corsReplicationState, ok bool) {
for dID, status := range bs {
state, err := corsReplicationStateFromInfo(status.meta.SRBucketInfo)
if err != nil || state.kind == corsReplicationBaseline {
continue
}
cmp := compareCORSReplicationStates(latest, state)
if !ok || cmp < 0 || (cmp == 0 && dID > latestID) {
latestID = dID
latest = state
ok = true
}
}
return latestID, latest, ok
}
func (c *SiteReplicationSys) healCORSMetadata(ctx context.Context, objAPI ObjectLayer, bucket string, info srStatusInfo) error {
c.RLock()
defer c.RUnlock()
if !c.enabled {
return nil
}
var (
latestID, latestPeerName string
lastUpdate time.Time
latestCorsConfig *string
)
bs := info.BucketStats[bucket]
for dID, ss := range bs {
if lastUpdate.IsZero() {
lastUpdate = ss.meta.CorsConfigUpdatedAt
latestID = dID
latestCorsConfig = ss.meta.CorsConfig
}
// avoid considering just created buckets as latest. Perhaps this site
// just joined cluster replication and yet to be sync'd
if ss.meta.CreatedAt.Equal(ss.meta.CorsConfigUpdatedAt) {
continue
}
if ss.meta.CorsConfigUpdatedAt.After(lastUpdate) {
lastUpdate = ss.meta.CorsConfigUpdatedAt
latestID = dID
latestCorsConfig = ss.meta.CorsConfig
}
latestID, latestState, ok := latestCORSConfig(bs)
if !ok {
return nil
}
latestPeerName = info.Sites[latestID].Name
var latestCorsConfigBytes []byte
var err error
if latestCorsConfig != nil {
latestCorsConfigBytes, err = base64.StdEncoding.DecodeString(*latestCorsConfig)
if err != nil {
return err
}
}
latestPeerName := info.Sites[latestID].Name
latestCorsConfig := latestState.encodedPayload()
for dID, bStatus := range bs {
if !bStatus.CorsCfgMismatch {
continue
}
if isBucketMetadataEqual(latestCorsConfig, bStatus.meta.CorsConfig) {
currentState, err := corsReplicationStateFromInfo(bStatus.meta.SRBucketInfo)
if err == nil && equalCORSReplicationStates(latestState, currentState) {
continue
}
if dID == globalDeploymentID() {
if latestCorsConfig == nil {
if _, err := globalBucketMetadataSys.Delete(ctx, bucket, bucketCorsConfig); err != nil {
replLogIf(ctx, fmt.Errorf("Unable to heal CORS metadata from peer site %s : %w", latestPeerName, err))
}
continue
}
if _, err := globalBucketMetadataSys.Update(ctx, bucket, bucketCorsConfig, latestCorsConfigBytes); err != nil {
if err := c.PeerBucketCorsConfigHandler(ctx, bucket, latestCorsConfig, latestState.updatedAt); err != nil {
replLogIf(ctx, fmt.Errorf("Unable to heal CORS metadata from peer site %s : %w", latestPeerName, err))
}
continue
@@ -5067,7 +5251,7 @@ func (c *SiteReplicationSys) healCORSMetadata(ctx context.Context, objAPI Object
Type: madmin.SRBucketMetaTypeCorsConfig,
Bucket: bucket,
Cors: latestCorsConfig,
UpdatedAt: lastUpdate,
UpdatedAt: latestState.updatedAt,
})
if err != nil {
replLogIf(ctx, c.annotatePeerErr(peerName, replicateBucketMetadata,
@@ -5394,7 +5578,7 @@ func isBucketMetadataEqual(one, two *string) bool {
case one == nil || two == nil:
return false
default:
return strings.EqualFold(*one, *two)
return *one == *two
}
}
+584
View File
@@ -0,0 +1,584 @@
# Per-Bucket CORS Site-Replication Convergence Design
## Status
- Issue: [pgsty/silo#75](https://github.com/pgsty/silo/issues/75)
- Baseline: `e4e3007da6d7d1198a6a050e34f84566d40a9654`
- Working branch: `codex/issue-75-cors-hardening`
- Decision: CORS-specific deterministic last-writer-wins register, described below
- Implementation state: B2 is commit `724f8703d`; the final B2+B3 integration is signed commit `0eebc928f` on the PR #80 branch and has passed combined local acceptance
- Release state: PR #80 remains open; nothing is merged, tagged, packaged, published as an image, or deployed
- Final design/implementation review: the B2 implementation was GO; the combined B2+B3 Opus 5 Max review found one test-build conflict and one legacy-metadata load risk, both corrected before combined testing
This document defines the replication state, ordering, persistence, status,
healing, concurrency, compatibility, and test contract for per-bucket CORS.
It is an implementation design record, not public upgrade or rollback guidance.
Public operator documentation belongs in the separate `silo.pgsty.com`
repository.
## Scope
This design covers the current-version CORS path:
```text
PutBucketCors / DeleteBucketCors
-> persist local CORS state
-> BucketMetaHook
-> madmin SRBucketMeta transport
-> SRPeerReplicateBucketItem dispatch
-> PeerBucketCorsConfigHandler
-> SiteReplicationMetaInfo
-> siteReplicationStatus
-> latestCORSConfig
-> healCORSMetadata
```
It also covers retry, duplicate delivery, reordering, equal timestamps,
initial site sync, missed DELETE recovery, cache reload, process restart, and
concurrent CORS mutations on different nodes of one cluster.
The following are deliberately out of scope:
- redesigning the replication semantics of policy, tags, SSE, quota,
versioning, or Object Lock;
- eliminating lost updates between different bucket-metadata types that all
rewrite `.metadata.bin`; this inherited problem is tracked by
[pgsty/silo#77](https://github.com/pgsty/silo/issues/77);
- mixed-version support that permits CORS writes before every site runs a
CORS-aware binary;
- public downgrade, rollback, and global-fallback documentation;
- Console UI for bucket CORS.
### Adjacent issue-75 changes in the same candidate
The final issue-75 candidate also contains CORS work outside the LWW register
itself:
- a strict, namespace-tolerant XML wire parser that rejects trailing roots,
unknown/nested elements, duplicate singleton fields, invalid integer shape,
and non-whitespace character data;
- Unicode code-point ID counting, exact uppercase S3 methods, non-empty header
elements, and int32-compatible MaxAge validation;
- a single-`*` matcher and response-selection changes needed to distinguish a
literal `*` origin from a patterned or explicit `null` match;
- fail-closed metadata-error handling in the HTTP middleware;
- complete allowed-method, explicit MaxAge=0, expose-header, credentials, and
`Vary` preflight behavior;
- checksum mismatch classification as `BadDigest`; and
- parser, signed-handler, browser-response, and protocol adversarial tests.
Those changes share the same CORS release gate and are present in the reviewed
diff, but they are not part of the replication conflict key or join algorithm.
This document describes them only where they constrain replication validation
or the final verification boundary.
## Confirmed Failures in the Pre-Fix Candidate
The pre-fix issue-75 candidate had four independently reproduced convergence
defects:
1. Heal compared only payloads. If two sites stored identical payload bytes
with different source timestamps, heal skipped the older site. The sites
retained different ordering barriers and could disagree on a later delayed
event.
2. `isBucketMetadataEqual` used `strings.EqualFold` for base64. For example,
`QQ==` and `qQ==` decode to different bytes but compared equal.
3. Status derived `CorsCfgMismatch` from live payload count and payload set.
It did not include source timestamp or tombstone state, so it could report
divergent sites as converged and suppress healing.
4. Equal-timestamp conflicting events had no stable tie-breaker. Peer apply
accepted whichever event arrived last, while heal selected whichever map
entry happened to be visited first.
Two additional correctness requirements followed from the state model:
- the read, compare, and save transition must be atomic across nodes in one
cluster; and
- a successful local PUT or DELETE must advance beyond an already stored
future source timestamp instead of moving the local barrier backwards.
## Constraints
The minimum fix must satisfy these constraints:
- preserve `madmin.SRBucketMeta` and `madmin.SRBucketInfo` wire schemas;
- preserve the exact source `UpdatedAt` on peer apply and heal;
- distinguish a never-configured bucket from a persisted deletion;
- converge without relying on event arrival order, map iteration order, or a
particular site being the healer;
- serialize CORS-versus-CORS transitions cluster-wide without introducing a
broad bucket-metadata redesign;
- reject malformed replication payloads before persistence;
- remain idempotent under retry and initial-sync replay;
- keep the replication state-machine change limited to CORS except for the
directly shared base64 equality bug; do not infer that the same dirty
candidate contains no adjacent CORS protocol or middleware changes.
## State Model
For one bucket lineage, the persisted CORS state is:
```text
State = (Payload, SourceUpdatedAt)
```
`BucketMetadata.Created` is not part of the conflict key. It is the lineage
floor used to reject an event from an older incarnation of the bucket.
### State kinds
| Kind | Payload | `CorsConfigUpdatedAt` | Meaning |
| --- | --- | --- | --- |
| Baseline | nil | zero | CORS has never been configured for this bucket lineage |
| Live | non-empty XML bytes | non-zero | A live per-bucket CORS configuration |
| Tombstone | nil | non-zero | CORS was explicitly deleted at the source timestamp |
The baseline uses a zero timestamp deliberately. Defaulting a missing CORS
timestamp to `CreatedAt` would make classification depend on two values that
can be obtained from different cache/disk snapshots. It would also make a
never-configured state indistinguishable from a deletion at bucket creation.
Per-bucket CORS and `CorsConfigUpdatedAt` were introduced together, so there
is no released legacy live-CORS state that requires synthesizing a timestamp.
### Wire canonicalization
A non-nil wire payload must satisfy all of the following:
1. strict standard base64 decoding succeeds;
2. re-encoding the decoded bytes produces exactly the received string;
3. the decoded payload is non-empty;
4. CORS XML parsing succeeds; and
5. `cors.Config.Validate()` succeeds.
The canonical re-encode check rejects ignored newlines and alternate textual
representations. Equality is therefore equality of decoded bytes, with exact
base64 string equality remaining safe for the shared metadata helper.
An invalid wire value is not a candidate winner and is never propagated.
Peer apply rejects it before any metadata write.
A bucket may nevertheless contain a CORS document written by a pre-release,
more lenient build. Loading such metadata keeps policy, lifecycle, versioning,
and the other bucket fields available, but stashes the CORS parse/validation
error and exposes no active CORS config. CORS GET and middleware lookup return
that error, so browser handling fails closed. A valid PUT or DELETE can repair
the record; any attempt to save a newly invalid CORS document remains rejected.
## Deterministic Ordering
States use the following total order:
```text
1. SourceUpdatedAt
2. Kind: baseline < live < tombstone
3. For live/live ties: lexicographic decoded payload bytes
```
The greater state wins.
Consequences:
- a newer source event wins regardless of arrival order;
- the same payload with a newer timestamp is a greater state and advances the
ordering barrier;
- a DELETE wins an equal-timestamp PUT/DELETE conflict;
- two equal-timestamp live payloads choose the same bytewise winner at every
site;
- an exact duplicate is equal and therefore a no-op;
- retry, reordering, and duplicate delivery cannot move local state backward.
The live-payload tie-breaker is not intended to identify the human's temporal
intent. It supplies the deterministic result required when the timestamp has
already failed to distinguish two writes.
## Why No Source-Site Tie-Breaker
The rejected source-site alternative ordered states by timestamp plus origin
deployment ID. It would require a new origin field in madmin-go transport and
a persisted origin field in `BucketMetadata`. That adds a dependency release,
wire compatibility work, and an on-disk schema change without improving the
convergence guarantee over the content-based total order.
If a future product requirement needs provenance-aware conflict explanation,
the source-site design can be introduced as a versioned protocol. It is not
required to make the current register converge.
## Bucket Lineage and `CreatedAt`
`CreatedAt` protects a recreated bucket from delayed metadata events belonging
to the prior bucket incarnation:
```text
if incoming.SourceUpdatedAt < local.CreatedAt:
ignore and log once per bucket
```
The floor is retained because removing it could install an old CORS grant on a
new bucket with the same name. It is intentionally not used to classify the
baseline.
For current-version site replication, local events are generated strictly
after `max(CreatedAt, current CORS barrier)`, and bucket creation timestamps are
propagated before initial metadata sync. A floor rejection therefore indicates
a stale lineage event, clock/history corruption, or a mixed/unsupported setup.
The rejection is observable through a bucket-scoped log-once message and the
remaining status mismatch.
## Local Transition
PUT and DELETE use the same CORS-specific transition helper.
Under the bucket CORS namespace lock:
1. load the current `.metadata.bin` through the migration-aware parsed loader;
2. validate the new live payload, if any;
3. choose:
```text
UpdatedAt = max(UTCNow, CreatedAt + epsilon, CurrentBarrier + epsilon)
```
4. store either the live bytes or a nil tombstone with that timestamp;
5. save and refresh the parsed cache; and
6. release the lock before invoking `BucketMetaHook`.
This preserves HTTP semantics while ensuring a local administrative action is
strictly greater than the state it observed, including a future-dated peer
barrier caused by clock skew. The peer path deliberately uses a raw metadata
read instead: it must preserve the exact zero baseline and reject missing
metadata rather than implicitly creating a peer bucket record.
## Peer and Legacy-Bulk Transition
Typed CORS dispatch decodes and validates the payload, then performs this join
under the same lock:
```text
if incoming timestamp is zero:
reject
if incoming timestamp is before CreatedAt:
ignore and log
if incoming state <= local state:
no-op
otherwise:
persist incoming payload and exact source timestamp
```
The admin handler's legacy/default bulk metadata path can also carry a non-nil
CORS field. It therefore takes the same CORS lock, applies strict decoding and
validation, and uses the same state comparison before saving. A nil CORS field
in that untyped legacy shape means "not included" and cannot represent a
tombstone; current producers use the typed CORS event for deletion.
## Concurrency and Locking
The transition lock is:
```text
.minio.sys / buckets/<bucket>/cors-config.lock
```
It is a virtual distributed namespace lock. The name deliberately differs from
the real `buckets/<bucket>/.metadata.bin` object because the metadata save path
locks that object internally and namespace locks are not re-entrant.
The lock serializes every intentional current-version local, typed-peer,
legacy-bulk, and local-heal CORS transition across nodes of one cluster. It
cannot prevent an unrelated whole-record writer from restoring stale CORS
columns. Residual paths include another metadata type's `Update`/`Delete`, a
legacy bulk item whose nil CORS field means "not included",
`ImportBucketMetadata`, and bucket-make metadata rewriting. Their inherited
whole-record behavior is the separate architectural problem under issue #77.
No cross-site admin call or `BucketMetaHook` dispatch is made while holding the
CORS lock. The metadata save can perform blocking intra-cluster notification
fan-out before the lock is released. Local handlers release the lock before
cross-site dispatch; reordered network delivery is handled by the total-order
join.
## Dispatch and Retry
PUT sends a typed `SRBucketMetaTypeCorsConfig` event with canonical base64 XML
and the local source timestamp. DELETE sends the same type with `Cors == nil`
and the tombstone timestamp.
`BucketMetaHook` may deliver concurrently to sites, fail on a subset, or be
retried by an external operation. The receiver transition is idempotent, so the
transport does not need to impose a global event order.
Current-version admin dispatch routes the typed event directly to
`PeerBucketCorsConfigHandler`. The legacy/default path is hardened only to
prevent a non-nil CORS field from bypassing the join; it is not a tombstone
compatibility protocol.
## Status Projection
`SiteReplicationMetaInfo` always exports `CorsConfigUpdatedAt`, including zero
baseline and nil tombstone states. It exports `CorsConfig` only for a live
payload.
Status considers sites converged if and only if every site has the same full
CORS state:
```text
(kind, decoded payload bytes, SourceUpdatedAt)
```
Live payload counts remain useful for per-site summary totals, but they do not
determine `CorsCfgMismatch`.
Examples:
| Site A | Site B | Mismatch |
| --- | --- | --- |
| baseline | baseline | no |
| same live bytes at same timestamp | same live bytes at same timestamp | no |
| same live bytes at different timestamps | same live bytes at different timestamps | yes |
| same tombstone timestamp | same tombstone timestamp | no |
| tombstones at different timestamps | tombstones at different timestamps | yes |
| live | tombstone | yes |
| invalid wire state | any state | yes |
## Winner Selection and Heal
Heal computes the maximum non-baseline valid state using the total order.
Selection is independent of Go map iteration. Deployment ID is used only as a
stable log-source choice when two sites already expose exactly equal states.
For each different site:
- the local site delegates to the normal peer CORS transition, preserving the
source timestamp and lock discipline;
- a remote site receives a typed `SRBucketMetaTypeCorsConfig` event with the
winner's canonical payload or nil tombstone and exact timestamp.
Payload equality alone is insufficient. A site with identical bytes at an
older timestamp is healed so it acquires the same future ordering barrier.
If every reported state is baseline, there is no event to propagate. If every
reported state is invalid, status remains mismatched and heal does not select
corrupt input as a source.
## Initial Sync
Initial sync emits:
- a live event when `CorsConfigUpdatedAt` is non-zero and payload is live;
- a tombstone event when `CorsConfigUpdatedAt` is non-zero and payload is nil;
- no event for the zero baseline.
Replaying initial sync is idempotent. A missed DELETE is recoverable because
the tombstone is part of the snapshot rather than being inferred from the
absence of a live payload.
All sites must run the CORS-aware implementation before enabling or mutating
per-bucket CORS. An older receiver can route an unknown typed event through a
legacy path that cannot represent deletion and does not provide this ordering
contract.
## Persistence and Restart
`CorsConfigXML` and `CorsConfigUpdatedAt` are persisted together in
`BucketMetadata` msgpack. Zero time round-trips as zero; CORS is deliberately
not defaulted to `CreatedAt` during load.
`BucketMetadata.Save` parses the live CORS XML before writing and before the
metadata system replaces the local cache. Therefore a rejected payload cannot
poison disk or cache, and a successful peer/heal transition immediately serves
the newly persisted parsed configuration.
After cache removal or process restart:
- a live state restores the same parsed rules and source timestamp;
- a tombstone restores nil payload plus its non-zero timestamp;
- a baseline remains nil plus zero timestamp.
- a legacy-invalid raw document leaves the non-CORS bucket metadata readable,
disables per-bucket CORS fail-closed, and remains repairable through a valid
CORS PUT or DELETE.
## Error Handling
| Error | Behavior |
| --- | --- |
| zero source timestamp on a peer live/delete event | reject the event |
| invalid/non-canonical base64 | reject before locking or saving |
| empty non-nil payload | reject |
| malformed XML | reject before saving |
| semantically invalid CORS rules | reject before saving |
| legacy-invalid CORS already on disk | load other metadata, return a CORS-specific error, and permit CORS replacement or deletion |
| event before bucket `CreatedAt` | ignore and log once per bucket |
| missing bucket metadata | return an error; do not create metadata implicitly |
| exact duplicate or lower state | successful no-op |
| remote heal failure | log the peer error; future heal cycles retry |
## Alternatives Considered
### Timestamp only
Rejected. Ignoring or accepting every equal-timestamp conflict leaves an
already divergent pair without a deterministic repair rule.
### Timestamp plus source deployment ID
Rejected for the current protocol. It is convergent, but requires madmin-go,
wire, and persisted-schema changes without improving convergence over the
selected total order.
### Payload-only status and heal
Rejected. It cannot distinguish ordering barriers and suppresses the exact
heal needed to make later event acceptance consistent.
### Default baseline timestamp to bucket creation
Rejected. It conflates baseline classification with a mutable value that may
come from a different snapshot and can turn a never-configured site into a
false tombstone source.
### Reuse `.metadata.bin` as the transition lock
Rejected. The save path takes the same namespace lock internally; reusing it
would self-deadlock.
### Redesign every bucket metadata type together
Rejected for issue #75. Neighboring metadata types have related inherited
patterns but different delete, validation, and compatibility semantics. They
require focused reproductions under issue #77.
## Invariants
The implementation is acceptable only while all of these invariants hold:
1. Zero timestamp plus nil payload is the only baseline representation.
2. Nil payload plus non-zero timestamp is a durable tombstone.
3. A live payload has canonical base64 on the wire, valid CORS XML, and a
non-zero source timestamp.
4. Every intentional current-version CORS state transition is serialized by
the CORS namespace lock from disk read through state comparison and save;
unrelated whole-record overwrite risk remains explicitly under issue #77.
5. Peer apply and heal never replace local state with a lower or equal state.
6. Local PUT and DELETE create a state strictly greater than the state observed
under the lock.
7. A peer event before local bucket creation cannot modify the new bucket
lineage.
8. Status reports convergence only for identical full states.
9. Heal selects the same maximum regardless of arrival order, site, or map
iteration order.
10. Same-payload/newer-timestamp heal advances the older barrier.
11. Initial sync and retry preserve tombstones and source timestamps.
12. Disk reload and cache reload preserve state kind, payload, and timestamp.
13. A legacy-invalid CORS document cannot activate global fallback, hide other
bucket metadata, or prevent a valid CORS PUT/DELETE repair.
## Test Contract
The required test matrix is:
| Area | Required evidence |
| --- | --- |
| Wire | canonical base64 accepted; case-different decoded bytes differ; malformed and non-canonical base64 rejected |
| Validation | invalid XML and semantically invalid origin/method/rule rejected without mutation |
| Strict wire | standard S3 namespace accepted; trailing root, unknown/nested elements, duplicate singleton fields, lowercase methods, byte-counted Unicode IDs, and invalid MaxAge rejected |
| Ordering | older event ignored; newer event applied; duplicate no-op; equal live/live order-independent; equal PUT/DELETE chooses tombstone |
| Barrier | same payload with newer timestamp is persisted and healed |
| Tombstone | delayed PUT cannot resurrect; missed DELETE wins heal; repeated DELETE is idempotent |
| Status | baseline, live, tombstone, payload mismatch, and timestamp-only mismatch classified correctly |
| Winner | three-site equal-timestamp selection remains deterministic across repeated map iteration |
| Concurrency | concurrent peer and legacy-bulk events converge to the total-order maximum |
| Local concurrency | concurrent local PUT/DELETE timestamps are unique and final state matches the last serialized transition |
| Initial sync | baseline omitted; live and tombstone emitted with exact source timestamp |
| Lineage | pre-creation event ignored; post-creation event applied |
| Restart | cache removal/disk reload preserves tombstone or live state and status timestamp |
| Legacy repair | a lenient historical document loads fail-closed without hiding other metadata and can be deleted or replaced |
| Full seam | signed admin dispatch -> peer apply -> real status collection -> local heal -> cache reload -> remote heal dispatch |
## Local Verification Record
The committed B2 implementation passed:
- the supplied adversarial base64 and same-payload/newer-timestamp tests;
- focused CORS normal tests;
- focused CORS race tests;
- `go test ./internal/bucket/cors` and its race run;
- `go test ./cmd -count=1`;
- `go vet ./...`;
- `go build ./...`;
- repository-configured golangci-lint v2.13.1 with zero issues;
- gofmt and `git diff --check`;
- a signed admin dispatch -> apply -> status -> heal -> cache reload test.
After integrating B3 and resolving overlap, the frozen combination passed:
- focused strict-parser, validation, middleware, replication, namespace,
legacy-repair, and race tests;
- CI-tagged `go test ./...`, full vet/build, module verification, pinned lint,
and rebrand/compatibility checks;
- a real local two-site deployment with two nodes per site, including
bidirectional replacement, a site missing DELETE while offline, restart
heal, and a second restart preserving the tombstone; and
- raw SigV4 wire probes that reject a lowercase method and trailing XML root,
accept a 255-code-point Unicode ID, and replicate the accepted config.
The public EN/ZH design records pass a warning-fatal Hugo build, rendered link
checking, and local browser QA. These results are acceptance evidence, not a
release, deployment, tag, or production claim.
The repository `make lint` bootstrap could not download its private copy of
golangci-lint because the network returned HTTP status 000. The same exact
v2.13.1 binary already installed locally was used with the Makefile's build
tags, timeout, and configuration and reported zero issues.
## Independent Review Record
Four read-only local Claude Code reviews used canonical model
`claude-opus-5` at `max` effort.
The first review rejected the pre-fix candidate and identified the unsafe
CreatedAt-based baseline, missing deterministic tie-break, missing atomic join,
non-monotonic local barrier, timestamp-blind status, and initial-sync tombstone
gap. The selected C-prime model incorporated the valid findings while rejecting
the suggestion to rewrite normal source timestamps.
The second review found no P0. Its `GO WITH FIXES` findings were peer semantic
validation, the legacy/default admin mutation path bypassing the CORS lock and
join, CreatedAt-floor observability, and missing tests for invalid XML, lineage,
and concurrent local transitions. Those required changes and tests are now in
the working tree.
The final review examined this design and the exact dirty diff, independently
reran build, vet, lint, normal tests, and race tests, and found no P0 or P1.
Its verdict was `GO WITH FIXES`: the implementation was explicitly judged GO,
while five design-document statements required correction. It also suggested
an optional status hardening so semantically invalid canonical payloads are
not selected and retransmitted. The hardening and all mandatory documentation
corrections are incorporated in the current tree. The final selected solution
is therefore the C-prime register and invariants recorded in this document.
The fourth review examined the resolved B2+B3 combination. It confirmed that
the C-prime register, strict wire parser, MaxAge presence, wildcard credentials,
Origin-null marker, rejected-preflight `Vary`, checksum classification, and
peer validation can coexist. It found a conflict-resolution test helper typo
and the risk that strict parsing could make all bucket metadata unavailable for
a document accepted by a lenient development build. The helper was corrected;
metadata loading now stashes a CORS-specific error, fails browser behavior
closed, rejects new invalid saves, and allows a valid CORS PUT/DELETE repair.
## Release Gates
An implementation-level GO means only that the local CORS state machine and
tests satisfy this document. It does not authorize a release.
Before closing issue #75 or publishing a server artifact:
1. commit the exact reviewed implementation and design with DCO sign-off;
2. push a focused branch and run remote PR CI;
3. merge and confirm main CI on the merge commit;
4. finish public EN/ZH upgrade, fallback, and downgrade documentation in
`silo.pgsty.com`;
5. run a real two-site process test for PUT, DELETE, simultaneous conflict,
offline peer restart, status, and heal;
6. verify no release tag or image contains an intermediate candidate; and
7. treat package, image, SBOM, signature, canary, and production verification
as separate gates.
+207 -23
View File
@@ -22,10 +22,11 @@ package cors
import (
"encoding/xml"
"errors"
"fmt"
"io"
"strconv"
"strings"
"github.com/minio/pkg/v3/wildcard"
"unicode/utf8"
)
// maxCORSRules is the maximum number of rules allowed per bucket (AWS S3 limit).
@@ -34,6 +35,10 @@ const maxCORSRules = 100
// maxCORSRuleIDLen is the maximum length of a CORSRule <ID> (AWS S3 limit).
const maxCORSRuleIDLen = 255
// maxCORSMaxAgeSeconds is the largest value representable by the int32
// MaxAgeSeconds shape used by the S3 API model.
const maxCORSMaxAgeSeconds = 1<<31 - 1
// supportedMethods are the HTTP methods permitted in an AllowedMethod element.
var supportedMethods = map[string]bool{
"GET": true,
@@ -57,17 +62,153 @@ type Rule struct {
AllowedOrigins []string `xml:"AllowedOrigin"`
ExposeHeaders []string `xml:"ExposeHeader"`
MaxAgeSeconds int `xml:"MaxAgeSeconds"`
maxAgeSecondsSet bool
}
type corsXMLUnknown struct {
XMLName xml.Name
}
type corsXMLValue struct {
Text string `xml:",chardata"`
Unknown []corsXMLUnknown `xml:",any"`
}
type configXML struct {
XMLName xml.Name `xml:"CORSConfiguration"`
CORSRules []ruleXML `xml:"CORSRule"`
Text string `xml:",chardata"`
Unknown []corsXMLUnknown `xml:",any"`
}
type ruleXML struct {
ID []corsXMLValue `xml:"ID"`
AllowedHeaders []corsXMLValue `xml:"AllowedHeader"`
AllowedMethods []corsXMLValue `xml:"AllowedMethod"`
AllowedOrigins []corsXMLValue `xml:"AllowedOrigin"`
ExposeHeaders []corsXMLValue `xml:"ExposeHeader"`
MaxAgeSeconds []corsXMLValue `xml:"MaxAgeSeconds"`
Text string `xml:",chardata"`
Unknown []corsXMLUnknown `xml:",any"`
}
// ParseBucketCorsConfig parses a CORS configuration from the given reader.
func ParseBucketCorsConfig(r io.Reader) (*Config, error) {
var c Config
if err := xml.NewDecoder(r).Decode(&c); err != nil {
var parsed configXML
decoder := xml.NewDecoder(r)
if err := decoder.Decode(&parsed); err != nil {
return nil, err
}
if strings.TrimSpace(parsed.Text) != "" {
return nil, xml.UnmarshalError("unexpected character data in CORSConfiguration")
}
if len(parsed.Unknown) > 0 {
return nil, xml.UnmarshalError(fmt.Sprintf("unexpected element <%s> in CORSConfiguration", parsed.Unknown[0].XMLName.Local))
}
c := Config{
XMLName: parsed.XMLName,
CORSRules: make([]Rule, len(parsed.CORSRules)),
}
for i := range parsed.CORSRules {
rule, err := parseCORSRuleXML(parsed.CORSRules[i])
if err != nil {
return nil, err
}
c.CORSRules[i] = rule
}
// Decode consumes one document element. Only XML whitespace, comments, and
// processing instructions are permitted after it.
for {
token, err := decoder.Token()
if err == io.EOF {
break
}
if err != nil {
return nil, err
}
switch token := token.(type) {
case xml.CharData:
if strings.TrimSpace(string(token)) == "" {
continue
}
case xml.Comment, xml.ProcInst:
continue
}
return nil, errors.New("unexpected XML content after CORSConfiguration")
}
return &c, nil
}
func parseCORSRuleXML(parsed ruleXML) (Rule, error) {
if strings.TrimSpace(parsed.Text) != "" {
return Rule{}, xml.UnmarshalError("unexpected character data in CORSRule")
}
if len(parsed.Unknown) > 0 {
return Rule{}, xml.UnmarshalError(fmt.Sprintf("unexpected element <%s> in CORSRule", parsed.Unknown[0].XMLName.Local))
}
if len(parsed.ID) > 1 {
return Rule{}, xml.UnmarshalError("duplicate ID element in CORSRule")
}
if len(parsed.MaxAgeSeconds) > 1 {
return Rule{}, xml.UnmarshalError("duplicate MaxAgeSeconds element in CORSRule")
}
rule := Rule{}
var err error
if len(parsed.ID) == 1 {
if rule.ID, err = corsXMLText("ID", parsed.ID[0]); err != nil {
return Rule{}, err
}
}
if rule.AllowedHeaders, err = corsXMLTexts("AllowedHeader", parsed.AllowedHeaders); err != nil {
return Rule{}, err
}
if rule.AllowedMethods, err = corsXMLTexts("AllowedMethod", parsed.AllowedMethods); err != nil {
return Rule{}, err
}
if rule.AllowedOrigins, err = corsXMLTexts("AllowedOrigin", parsed.AllowedOrigins); err != nil {
return Rule{}, err
}
if rule.ExposeHeaders, err = corsXMLTexts("ExposeHeader", parsed.ExposeHeaders); err != nil {
return Rule{}, err
}
if len(parsed.MaxAgeSeconds) == 1 {
value, valueErr := corsXMLText("MaxAgeSeconds", parsed.MaxAgeSeconds[0])
if valueErr != nil {
return Rule{}, valueErr
}
age, parseErr := strconv.ParseInt(strings.TrimSpace(value), 10, 32)
if parseErr != nil {
return Rule{}, xml.UnmarshalError("invalid MaxAgeSeconds value")
}
rule.MaxAgeSeconds = int(age)
rule.maxAgeSecondsSet = true
}
return rule, nil
}
func corsXMLTexts(name string, values []corsXMLValue) ([]string, error) {
result := make([]string, len(values))
for i := range values {
value, err := corsXMLText(name, values[i])
if err != nil {
return nil, err
}
result[i] = value
}
return result, nil
}
func corsXMLText(name string, value corsXMLValue) (string, error) {
if len(value.Unknown) > 0 {
return "", xml.UnmarshalError(fmt.Sprintf("element <%s> must not contain child element <%s>", name, value.Unknown[0].XMLName.Local))
}
return value.Text, nil
}
// Validate checks the config against the S3 constraints.
func (c *Config) Validate() error {
if len(c.CORSRules) == 0 {
@@ -77,7 +218,10 @@ func (c *Config) Validate() error {
return errors.New("CORSConfiguration exceeds the maximum number of rules")
}
for _, r := range c.CORSRules {
if len(r.ID) > maxCORSRuleIDLen {
if !utf8.ValidString(r.ID) {
return errors.New("CORSRule ID must contain valid UTF-8")
}
if utf8.RuneCountInString(r.ID) > maxCORSRuleIDLen {
return errors.New("CORSRule ID exceeds the maximum length of 255 characters")
}
if len(r.AllowedOrigins) == 0 {
@@ -87,41 +231,75 @@ func (c *Config) Validate() error {
return errors.New("CORSRule must contain at least one AllowedMethod")
}
for _, o := range r.AllowedOrigins {
if o == "" {
return errors.New("AllowedOrigin must not be empty")
}
if strings.Contains(o, "?") {
return errors.New("AllowedOrigin may not contain wildcard '?': " + o)
}
if strings.Count(o, "*") > 1 {
return errors.New("AllowedOrigin may contain at most one wildcard '*': " + o)
}
}
for _, m := range r.AllowedMethods {
if !supportedMethods[strings.ToUpper(m)] {
if !supportedMethods[m] {
return errors.New("unsupported method in CORSRule: " + m)
}
}
for _, h := range r.AllowedHeaders {
if h == "" {
return errors.New("AllowedHeader must not be empty")
}
if strings.Contains(h, "?") {
return errors.New("AllowedHeader may not contain wildcard '?': " + h)
}
if strings.Count(h, "*") > 1 {
return errors.New("AllowedHeader may contain at most one wildcard '*': " + h)
}
}
for _, h := range r.ExposeHeaders {
if h == "" {
return errors.New("ExposeHeader must not be empty")
}
}
if r.MaxAgeSeconds < 0 {
return errors.New("MaxAgeSeconds must not be negative")
}
if int64(r.MaxAgeSeconds) > maxCORSMaxAgeSeconds {
return errors.New("MaxAgeSeconds exceeds the maximum S3 integer value")
}
}
return nil
}
// HasAllowedOrigin reports whether the rule allows the given origin.
func (r Rule) HasAllowedOrigin(origin string) bool {
for _, o := range r.AllowedOrigins {
if o == "*" || wildcard.MatchSimple(o, origin) {
return true
func matchSingleWildcard(pattern, value string) bool {
prefix, suffix, found := strings.Cut(pattern, "*")
if !found {
return pattern == value
}
return len(value) >= len(prefix)+len(suffix) &&
strings.HasPrefix(value, prefix) && strings.HasSuffix(value, suffix)
}
func (r Rule) matchAllowedOrigin(origin string) (string, bool) {
for _, allowedOrigin := range r.AllowedOrigins {
if matchSingleWildcard(allowedOrigin, origin) {
return allowedOrigin, true
}
}
return false
return "", false
}
// HasAllowedOrigin reports whether the rule allows the given origin.
func (r Rule) HasAllowedOrigin(origin string) bool {
_, ok := r.matchAllowedOrigin(origin)
return ok
}
// HasAllowedMethod reports whether the rule allows the given HTTP method.
func (r Rule) HasAllowedMethod(method string) bool {
for _, m := range r.AllowedMethods {
if strings.EqualFold(m, method) {
if m == method {
return true
}
}
@@ -147,22 +325,24 @@ func (r Rule) FilterAllowedHeaders(reqHeaders []string) ([]string, bool) {
func (r Rule) headerAllowed(header string) bool {
for _, h := range r.AllowedHeaders {
if h == "*" || wildcard.MatchSimple(strings.ToLower(h), strings.ToLower(header)) {
if matchSingleWildcard(strings.ToLower(h), strings.ToLower(header)) {
return true
}
}
return false
}
// MatchRule returns the first rule whose origin and method both match.
func (c *Config) MatchRule(origin, method string) (*Rule, bool) {
// MatchRule returns the first rule whose origin and method both match, along
// with the configured origin pattern that matched.
func (c *Config) MatchRule(origin, method string) (rule *Rule, allowedOrigin string, ok bool) {
for i := range c.CORSRules {
r := &c.CORSRules[i]
if r.HasAllowedOrigin(origin) && r.HasAllowedMethod(method) {
return r, true
matchedOrigin, originOK := r.matchAllowedOrigin(origin)
if originOK && r.HasAllowedMethod(method) {
return r, matchedOrigin, true
}
}
return nil, false
return nil, "", false
}
// MatchPreflight returns the first rule whose origin and method match and
@@ -170,17 +350,21 @@ func (c *Config) MatchRule(origin, method string) (*Rule, bool) {
// 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) {
func (c *Config) MatchPreflight(origin, method string, reqHeaders []string) (rule *Rule, allowedOrigin string, allowedHeaders []string, maxAgeSeconds *int, ok bool) {
for i := range c.CORSRules {
r := &c.CORSRules[i]
if !r.HasAllowedOrigin(origin) || !r.HasAllowedMethod(method) {
matchedOrigin, originOK := r.matchAllowedOrigin(origin)
if !originOK || !r.HasAllowedMethod(method) {
continue
}
allowed, headersOK := r.FilterAllowedHeaders(reqHeaders)
if !headersOK {
continue
}
return r, allowed, true
if r.maxAgeSecondsSet || r.MaxAgeSeconds != 0 {
maxAgeSeconds = &r.MaxAgeSeconds
}
return r, matchedOrigin, allowed, maxAgeSeconds, true
}
return nil, nil, false
return nil, "", nil, nil, false
}
@@ -0,0 +1,222 @@
// Copyright (c) 2015-2021 MinIO, Inc.
//
// This file is part of MinIO Object Storage stack
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
package cors
import (
"strconv"
"strings"
"testing"
)
func TestParseStandardS3Namespace(t *testing.T) {
doc := `<CORSConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><CORSRule><AllowedOrigin>https://app.example.com</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`
cfg, err := ParseBucketCorsConfig(strings.NewReader(doc))
if err != nil {
t.Fatal(err)
}
if err = cfg.Validate(); err != nil {
t.Fatal(err)
}
if _, _, ok := cfg.MatchRule("https://app.example.com", "GET"); !ok {
t.Fatal("standard S3 namespace document did not produce a matching rule")
}
}
const minimalCORSConfig = `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`
func TestParseRejectsTrailingXMLRoot(t *testing.T) {
for name, suffix := range map[string]string{
"second root": `<Extra/>`,
"text": `junk`,
"dangling close": `</Extra>`,
} {
t.Run(name, func(t *testing.T) {
if _, err := ParseBucketCorsConfig(strings.NewReader(minimalCORSConfig + suffix)); err == nil {
t.Fatalf("expected trailing %s to be rejected", name)
}
})
}
}
func TestParseAllowsXMLMiscAfterRoot(t *testing.T) {
for name, suffix := range map[string]string{
"whitespace": " \n\t",
"comment": `<!-- trailing comment -->`,
"processing instruction": `<?cors-test done?>`,
} {
t.Run(name, func(t *testing.T) {
if _, err := ParseBucketCorsConfig(strings.NewReader(minimalCORSConfig + suffix)); err != nil {
t.Fatalf("valid trailing XML misc was rejected: %v", err)
}
})
}
}
func TestValidateCORSRuleIDCountsCharacters(t *testing.T) {
doc := `<CORSConfiguration><CORSRule><ID>` + strings.Repeat("界", 255) + `</ID><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`
cfg, err := ParseBucketCorsConfig(strings.NewReader(doc))
if err != nil {
t.Fatalf("parse failed: %v", err)
}
if err = cfg.Validate(); err != nil {
t.Fatalf("255-character rule ID must be accepted: %v", err)
}
cfg.CORSRules[0].ID += "界"
if err = cfg.Validate(); err == nil {
t.Fatal("256-character rule ID must be rejected")
}
}
func TestValidateRejectsNonCanonicalAllowedMethod(t *testing.T) {
doc := `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>get</AllowedMethod></CORSRule></CORSConfiguration>`
cfg, err := ParseBucketCorsConfig(strings.NewReader(doc))
if err != nil {
t.Fatalf("parse failed: %v", err)
}
if err = cfg.Validate(); err == nil {
t.Fatal("expected lowercase AllowedMethod to be rejected")
}
}
func TestAllowedMethodMatchingIsCaseSensitive(t *testing.T) {
rule := Rule{AllowedMethods: []string{"GET"}}
if !rule.HasAllowedMethod("GET") {
t.Fatal("expected canonical GET to match")
}
if rule.HasAllowedMethod("get") {
t.Fatal("lowercase request method must not match canonical GET")
}
}
func TestParseRejectsElementsOutsideCORSShape(t *testing.T) {
tests := map[string]string{
"unknown root child": `<CORSConfiguration><Unknown/><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
"unknown rule child": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><Unknown/></CORSRule></CORSConfiguration>`,
"nested origin child": `<CORSConfiguration><CORSRule><AllowedOrigin><Unknown/></AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
"duplicate id": `<CORSConfiguration><CORSRule><ID>a</ID><ID>b</ID><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
"duplicate max age": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><MaxAgeSeconds>1</MaxAgeSeconds><MaxAgeSeconds>2</MaxAgeSeconds></CORSRule></CORSConfiguration>`,
"empty max age": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><MaxAgeSeconds/></CORSRule></CORSConfiguration>`,
"overflow max age": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><MaxAgeSeconds>2147483648</MaxAgeSeconds></CORSRule></CORSConfiguration>`,
}
for name, doc := range tests {
t.Run(name, func(t *testing.T) {
if _, err := ParseBucketCorsConfig(strings.NewReader(doc)); err == nil {
t.Fatal("expected parse error")
}
})
}
}
func TestMaxAgeSecondsPresence(t *testing.T) {
tests := []struct {
name string
element string
value int
present bool
}{
{name: "absent"},
{name: "zero", element: `<MaxAgeSeconds>0</MaxAgeSeconds>`, present: true},
{name: "positive", element: `<MaxAgeSeconds>3000</MaxAgeSeconds>`, value: 3000, present: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
doc := `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod>` + tt.element + `</CORSRule></CORSConfiguration>`
cfg, err := ParseBucketCorsConfig(strings.NewReader(doc))
if err != nil {
t.Fatalf("parse failed: %v", err)
}
rule := cfg.CORSRules[0]
_, _, _, maxAgeSeconds, ok := cfg.MatchPreflight("https://example.com", "GET", nil)
if !ok {
t.Fatal("expected rule to match")
}
present := maxAgeSeconds != nil
if rule.MaxAgeSeconds != tt.value || present != tt.present {
t.Fatalf("MaxAgeSeconds = %d, present = %v", rule.MaxAgeSeconds, present)
}
})
}
}
func TestValidateRuleCountBoundary(t *testing.T) {
rule := Rule{AllowedOrigins: []string{"*"}, AllowedMethods: []string{"GET"}}
cfg := Config{CORSRules: make([]Rule, 100)}
for i := range cfg.CORSRules {
cfg.CORSRules[i] = rule
}
if err := cfg.Validate(); err != nil {
t.Fatalf("100 rules must be accepted: %v", err)
}
cfg.CORSRules = append(cfg.CORSRules, rule)
if err := cfg.Validate(); err == nil {
t.Fatal("101 rules must be rejected")
}
}
func TestValidateMaxAgeSecondsBoundary(t *testing.T) {
cfg := Config{CORSRules: []Rule{{
AllowedOrigins: []string{"*"},
AllowedMethods: []string{"GET"},
MaxAgeSeconds: maxCORSMaxAgeSeconds,
}}}
if err := cfg.Validate(); err != nil {
t.Fatalf("MaxAgeSeconds int32 maximum must be accepted: %v", err)
}
if strconv.IntSize > 32 {
overflow := int64(maxCORSMaxAgeSeconds) + 1
cfg.CORSRules[0].MaxAgeSeconds = int(overflow)
if err := cfg.Validate(); err == nil {
t.Fatal("MaxAgeSeconds above int32 maximum must be rejected")
}
}
}
func TestSingleWildcardMatching(t *testing.T) {
tests := []struct {
pattern string
value string
want bool
}{
{"*", "https://example.com", true},
{"https://*.example.com", "https://api.example.com", true},
{"https://*.example.com", "https://.example.com", true},
{"https://*.example.com", "http://api.example.com", false},
{"https://?.example.com", "https://a.example.com", false},
}
for _, tt := range tests {
if got := matchSingleWildcard(tt.pattern, tt.value); got != tt.want {
t.Errorf("matchSingleWildcard(%q, %q) = %v, want %v", tt.pattern, tt.value, got, tt.want)
}
}
}
func TestMatchRuleReturnsMatchedOriginPattern(t *testing.T) {
cfg := Config{CORSRules: []Rule{{
AllowedOrigins: []string{"https://app.example.com", "https://*", "*"},
AllowedMethods: []string{"GET"},
}}}
tests := []struct {
origin string
want string
}{
{"https://app.example.com", "https://app.example.com"},
{"https://other.example.com", "https://*"},
{"http://other.example.com", "*"},
}
for _, tt := range tests {
_, got, ok := cfg.MatchRule(tt.origin, "GET")
if !ok || got != tt.want {
t.Errorf("origin %q matched %q, ok=%v; want %q", tt.origin, got, ok, tt.want)
}
}
}
+43 -4
View File
@@ -55,10 +55,15 @@ func TestValidateRejections(t *testing.T) {
cases := map[string]string{
"bad method": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>TRACE</AllowedMethod></CORSRule></CORSConfiguration>`,
"no origin": `<CORSConfiguration><CORSRule><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
"empty origin": `<CORSConfiguration><CORSRule><AllowedOrigin></AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
"no method": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin></CORSRule></CORSConfiguration>`,
"negative age": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><MaxAgeSeconds>-1</MaxAgeSeconds></CORSRule></CORSConfiguration>`,
"multi wildcard origin": `<CORSConfiguration><CORSRule><AllowedOrigin>https://*.*.example.com</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
"multi wildcard header": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><AllowedHeader>x-*-*</AllowedHeader></CORSRule></CORSConfiguration>`,
"question mark origin": `<CORSConfiguration><CORSRule><AllowedOrigin>https://?.example.com</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
"question mark header": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><AllowedHeader>x-amz-?</AllowedHeader></CORSRule></CORSConfiguration>`,
"empty allowed header": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><AllowedHeader/></CORSRule></CORSConfiguration>`,
"empty expose header": `<CORSConfiguration><CORSRule><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod><ExposeHeader/></CORSRule></CORSConfiguration>`,
"overlong id": `<CORSConfiguration><CORSRule><ID>` + strings.Repeat("a", 256) + `</ID><AllowedOrigin>*</AllowedOrigin><AllowedMethod>GET</AllowedMethod></CORSRule></CORSConfiguration>`,
}
for name, doc := range cases {
@@ -74,14 +79,14 @@ func TestValidateRejections(t *testing.T) {
func TestMatching(t *testing.T) {
c, _ := ParseBucketCorsConfig(strings.NewReader(sampleCORS))
rule, ok := c.MatchRule("https://api.example.org", "GET")
rule, _, ok := c.MatchRule("https://api.example.org", "GET")
if !ok {
t.Fatal("expected origin+method to match")
}
if _, ok := c.MatchRule("http://evil.com", "GET"); ok {
if _, _, ok := c.MatchRule("http://evil.com", "GET"); ok {
t.Fatal("did not expect match for disallowed origin")
}
if _, ok := c.MatchRule("http://www.example.com", "DELETE"); ok {
if _, _, ok := c.MatchRule("http://www.example.com", "DELETE"); ok {
t.Fatal("did not expect match for disallowed method")
}
allowed, ok := rule.FilterAllowedHeaders([]string{"x-amz-date", "x-amz-content-sha256"})
@@ -118,7 +123,7 @@ func TestMatchPreflightFallsThroughToLaterRule(t *testing.T) {
t.Fatalf("parse failed: %v", err)
}
rule, allowed, ok := c.MatchPreflight("https://app.example.com", "GET", []string{"x-custom-header"})
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")
}
@@ -129,3 +134,37 @@ func TestMatchPreflightFallsThroughToLaterRule(t *testing.T) {
t.Fatalf("unexpected allowed headers: %v", allowed)
}
}
func TestMatchAllowedOriginReturnsFirstMatchingPattern(t *testing.T) {
rule := Rule{AllowedOrigins: []string{"https://app.example.com", "https://*", "*"}}
tests := []struct {
origin string
want string
}{
{"https://app.example.com", "https://app.example.com"},
{"https://other.example.com", "https://*"},
{"http://other.example.com", "*"},
}
for _, tt := range tests {
got, ok := rule.matchAllowedOrigin(tt.origin)
if !ok {
t.Fatalf("expected %q to match", tt.origin)
}
if got != tt.want {
t.Fatalf("origin %q matched %q, want %q", tt.origin, got, tt.want)
}
}
}
func TestFilterAllowedHeadersPreservesRequestedNames(t *testing.T) {
rule := Rule{AllowedHeaders: []string{"x-amz-*"}}
allowed, ok := rule.FilterAllowedHeaders([]string{"X-Amz-Date", " X-AMZ-Meta-Test "})
if !ok {
t.Fatal("expected both request headers to match")
}
if got := strings.Join(allowed, ","); got != "X-Amz-Date,X-AMZ-Meta-Test" {
t.Fatalf("allowed headers = %q", got)
}
}