Browse Source

Case Sensitivity, pattern cache, Dead Code Removal

pull/7471/head
chrislu 2 months ago
parent
commit
5d3a3cecae
  1. 52
      weed/s3api/s3_bucket_policy_simple_test.go
  2. 97
      weed/s3api/s3api_bucket_handlers.go

52
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 {

97
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

Loading…
Cancel
Save