From 5d3a3cecae80df1c6328b9a2876c8a3757ae6b1d Mon Sep 17 00:00:00 2001 From: chrislu Date: Wed, 12 Nov 2025 16:37:08 -0800 Subject: [PATCH] Case Sensitivity, pattern cache, Dead Code Removal --- weed/s3api/s3_bucket_policy_simple_test.go | 52 +++++++++++- weed/s3api/s3api_bucket_handlers.go | 97 +++++++++++++++++----- 2 files changed, 125 insertions(+), 24 deletions(-) diff --git a/weed/s3api/s3_bucket_policy_simple_test.go b/weed/s3api/s3_bucket_policy_simple_test.go index 885580399..5226f767f 100644 --- a/weed/s3api/s3_bucket_policy_simple_test.go +++ b/weed/s3api/s3_bucket_policy_simple_test.go @@ -609,12 +609,62 @@ func TestMatchesPatternRegexEscaping(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := matchesPattern(tt.pattern, tt.str) + result := matchesPattern(tt.pattern, tt.str, true) // case-sensitive for resource tests assert.Equal(t, tt.expected, result, tt.reason) }) } } +// TestActionMatchingCaseInsensitive tests that S3 actions are case-insensitive +func TestActionMatchingCaseInsensitive(t *testing.T) { + tests := []struct { + name string + pattern string + action string + expected bool + }{ + {"Exact match same case", "s3:GetObject", "s3:GetObject", true}, + {"Different case - lowercase", "s3:GetObject", "s3:getobject", true}, + {"Different case - uppercase", "s3:GetObject", "S3:GETOBJECT", true}, + {"Different case - mixed", "s3:GetObject", "S3:getObject", true}, + {"Wildcard with different case", "s3:Get*", "s3:getobject", true}, + {"Wildcard with uppercase", "s3:GET*", "s3:GetObject", true}, + {"No match different action", "s3:GetObject", "s3:PutObject", false}, + {"Question mark wildcard case-insensitive", "s3:?etObject", "s3:GetObject", true}, + {"Question mark wildcard different case", "s3:?etObject", "s3:GETOBJECT", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := matchesPattern(tt.pattern, tt.action, false) // case-insensitive for actions + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestResourceMatchingCaseSensitive tests that resources are case-sensitive +func TestResourceMatchingCaseSensitive(t *testing.T) { + tests := []struct { + name string + pattern string + resource string + expected bool + }{ + {"Exact match same case", "bucket/file.txt", "bucket/file.txt", true}, + {"Different case should NOT match", "bucket/file.txt", "bucket/File.txt", false}, + {"Different case should NOT match - uppercase", "bucket/file.txt", "BUCKET/FILE.TXT", false}, + {"Wildcard pattern case-sensitive", "bucket/*.txt", "bucket/file.txt", true}, + {"Wildcard pattern different case should NOT match", "bucket/*.txt", "bucket/File.TXT", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := matchesPattern(tt.pattern, tt.resource, true) // case-sensitive for resources + assert.Equal(t, tt.expected, result) + }) + } +} + // TestActionMatchingWithRegexChars tests action matching with regex special characters func TestActionMatchingWithRegexChars(t *testing.T) { tests := []struct { diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index 4e2bd5e49..c96c87cd2 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -14,6 +14,7 @@ import ( "sort" "strconv" "strings" + "sync" "time" "github.com/seaweedfs/seaweedfs/weed/iam/policy" @@ -36,6 +37,12 @@ import ( util_http "github.com/seaweedfs/seaweedfs/weed/util/http" ) +// Pattern cache for regex compilation performance +var ( + patternCache = make(map[string]*regexp.Regexp) + patternCacheMu sync.RWMutex +) + func (s3a *S3ApiServer) ListBucketsHandler(w http.ResponseWriter, r *http.Request) { glog.V(3).Infof("ListBucketsHandler") @@ -678,12 +685,6 @@ func principalMatchesAnonymous(principal interface{}) bool { return true } } - case []string: - for _, v := range awsVal { - if v == "*" { - return true - } - } } } } @@ -692,9 +693,10 @@ func principalMatchesAnonymous(principal interface{}) bool { } // actionMatches checks if the requested action matches any action in the statement +// S3 actions are case-insensitive per AWS specification func actionMatches(actions []string, requestedAction string) bool { for _, action := range actions { - if matchesPattern(action, requestedAction) { + if matchesPattern(action, requestedAction, false) { // case-insensitive return true } } @@ -712,26 +714,75 @@ func resourceMatches(resources []string, requestedResource string) bool { } // matchesPattern checks if a pattern matches a string (supports wildcards) -func matchesPattern(pattern, str string) bool { - if pattern == "*" || pattern == str { +// caseSensitive determines if the match should be case-sensitive +func matchesPattern(pattern, str string, caseSensitive bool) bool { + // Fast path for common cases + if pattern == "*" { return true } + if caseSensitive { + if pattern == str { + return true + } + } else { + if strings.EqualFold(pattern, str) { + return true + } + } + + // If no wildcards, we already checked equality above + if !strings.ContainsAny(pattern, "*?") { + return false + } + + // Build cache key + cacheKey := pattern + if !caseSensitive { + cacheKey = "(?i)" + pattern + } + + // Check cache first + patternCacheMu.RLock() + re, exists := patternCache[cacheKey] + patternCacheMu.RUnlock() + + if !exists { + // Escape regex metacharacters before expanding wildcards + // This prevents patterns like "*.json" from matching "filexjson" (no dot) + escaped := regexp.QuoteMeta(pattern) + + // Convert S3 wildcard pattern to regex + // * matches any sequence of characters + // ? matches any single character + escaped = strings.ReplaceAll(strings.ReplaceAll(escaped, "\\*", ".*"), "\\?", ".") + regexPattern := "^" + escaped + "$" + + // Add case-insensitive flag if needed + if !caseSensitive { + regexPattern = "(?i)" + regexPattern + } + + var err error + re, err = regexp.Compile(regexPattern) + if err != nil { + return false // Invalid patterns do not match + } + + // Cache the compiled regex (with double-check locking) + patternCacheMu.Lock() + if re2, ok := patternCache[cacheKey]; ok { + re = re2 // Another goroutine already cached it + } else { + patternCache[cacheKey] = re + } + patternCacheMu.Unlock() + } - // Escape regex metacharacters before expanding wildcards - // This prevents patterns like "*.json" from matching "filexjson" (no dot) - escaped := regexp.QuoteMeta(pattern) - - // Convert S3 wildcard pattern to regex - // * matches any sequence of characters - // ? matches any single character - escaped = strings.ReplaceAll(strings.ReplaceAll(escaped, "\\*", ".*"), "\\?", ".") - regexPattern := "^" + escaped + "$" - - matched, err := regexp.MatchString(regexPattern, str) - return err == nil && matched + return re.MatchString(str) } // matchesResourcePattern checks if a resource pattern matches a resource ARN +// Resources are case-sensitive per AWS S3 specification func matchesResourcePattern(pattern, resource string) bool { if pattern == "*" || pattern == resource { return true @@ -746,8 +797,8 @@ func matchesResourcePattern(pattern, resource string) bool { return true } - // Check wildcard match - return matchesPattern(normalizedPattern, normalizedResource) + // Check wildcard match (case-sensitive for resources) + return matchesPattern(normalizedPattern, normalizedResource, true) } // normalizeResourceARN normalizes a resource ARN to a consistent format