From 0e0ac20fef8f5b83bb783e6dcadef5aa93f7fd22 Mon Sep 17 00:00:00 2001
From: Chris Lu <chris.lu@gmail.com>
Date: Thu, 30 Oct 2025 10:41:23 -0700
Subject: [PATCH] Refactor validateResourceForBucket to reduce code duplication

Simplified the validation logic by stripping ARN prefixes first,
then performing validation on the remaining resource path.
This reduces code duplication and improves maintainability while
maintaining identical functionality.

Addresses review feedback from Gemini Code Assist.
---
 weed/s3api/s3api_bucket_policy_handlers.go | 47 ++++++++--------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/weed/s3api/s3api_bucket_policy_handlers.go b/weed/s3api/s3api_bucket_policy_handlers.go
index e1fcf8203..2ed95b53a 100644
--- a/weed/s3api/s3api_bucket_policy_handlers.go
+++ b/weed/s3api/s3api_bucket_policy_handlers.go
@@ -288,39 +288,24 @@ func (s3a *S3ApiServer) validateResourceForBucket(resource, bucket string) bool
 	//   bucket-name/*
 	//   bucket-name/path/to/object
 
-	// Check AWS-style ARN
-	awsBucketArn := fmt.Sprintf("arn:aws:s3:::%s", bucket)
-	awsBucketWildcard := fmt.Sprintf("arn:aws:s3:::%s/*", bucket)
-	awsBucketPath := fmt.Sprintf("arn:aws:s3:::%s/", bucket)
-
-	if resource == awsBucketArn ||
-		resource == awsBucketWildcard ||
-		strings.HasPrefix(resource, awsBucketPath) {
-		return true
+	var resourcePath string
+	const awsPrefix = "arn:aws:s3:::"
+	const seaweedPrefix = "arn:seaweed:s3:::"
+
+	// Strip the optional ARN prefix to get the resource path
+	if strings.HasPrefix(resource, awsPrefix) {
+		resourcePath = resource[len(awsPrefix):]
+	} else if strings.HasPrefix(resource, seaweedPrefix) {
+		resourcePath = resource[len(seaweedPrefix):]
+	} else {
+		resourcePath = resource
 	}
 
-	// Check SeaweedFS-style ARN
-	seaweedBucketArn := fmt.Sprintf("arn:seaweed:s3:::%s", bucket)
-	seaweedBucketWildcard := fmt.Sprintf("arn:seaweed:s3:::%s/*", bucket)
-	seaweedBucketPath := fmt.Sprintf("arn:seaweed:s3:::%s/", bucket)
-
-	if resource == seaweedBucketArn ||
-		resource == seaweedBucketWildcard ||
-		strings.HasPrefix(resource, seaweedBucketPath) {
-		return true
-	}
-
-	// Check simplified format (bucket name without ARN prefix)
-	simplifiedBucketWildcard := fmt.Sprintf("%s/*", bucket)
-	simplifiedBucketPath := fmt.Sprintf("%s/", bucket)
-
-	if resource == bucket ||
-		resource == simplifiedBucketWildcard ||
-		strings.HasPrefix(resource, simplifiedBucketPath) {
-		return true
-	}
-
-	return false
+	// After stripping the optional ARN prefix, the resource path must
+	// either match the bucket name exactly, or be a path within the bucket.
+	return resourcePath == bucket ||
+		resourcePath == bucket+"/*" ||
+		strings.HasPrefix(resourcePath, bucket+"/")
 }
 
 // IAM integration functions