Browse Source

address some comments

pull/6987/head
chrislu 3 months ago
parent
commit
7c22a23964
  1. 16
      test/s3/cors/s3_cors_http_test.go
  2. 9
      test/s3/cors/s3_cors_test.go
  3. 21
      weed/s3api/cors/middleware.go
  4. 14
      weed/s3api/s3api_bucket_config.go
  5. 20
      weed/s3api/s3api_object_handlers.go

16
test/s3/cors/s3_cors_http_test.go

@ -44,7 +44,7 @@ func TestCORSPreflightRequest(t *testing.T) {
httpClient := &http.Client{Timeout: 10 * time.Second}
// Create OPTIONS request
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", defaultConfig.Endpoint, bucketName), nil)
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
require.NoError(t, err, "Should be able to create OPTIONS request")
// Add CORS preflight headers
@ -104,7 +104,7 @@ func TestCORSActualRequest(t *testing.T) {
// Test GET request with CORS headers using raw HTTP
httpClient := &http.Client{Timeout: 10 * time.Second}
req, err := http.NewRequest("GET", fmt.Sprintf("%s/%s/%s", defaultConfig.Endpoint, bucketName, objectKey), nil)
req, err := http.NewRequest("GET", fmt.Sprintf("%s/%s/%s", getDefaultConfig().Endpoint, bucketName, objectKey), nil)
require.NoError(t, err, "Should be able to create GET request")
// Add Origin header to simulate CORS request
@ -189,7 +189,7 @@ func TestCORSOriginMatching(t *testing.T) {
// Test preflight request
httpClient := &http.Client{Timeout: 10 * time.Second}
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", defaultConfig.Endpoint, bucketName), nil)
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
require.NoError(t, err, "Should be able to create OPTIONS request")
req.Header.Set("Origin", tc.requestOrigin)
@ -282,7 +282,7 @@ func TestCORSHeaderMatching(t *testing.T) {
// Test preflight request
httpClient := &http.Client{Timeout: 10 * time.Second}
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", defaultConfig.Endpoint, bucketName), nil)
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
require.NoError(t, err, "Should be able to create OPTIONS request")
req.Header.Set("Origin", "https://example.com")
@ -319,7 +319,7 @@ func TestCORSWithoutConfiguration(t *testing.T) {
// Test preflight request without CORS configuration
httpClient := &http.Client{Timeout: 10 * time.Second}
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", defaultConfig.Endpoint, bucketName), nil)
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
require.NoError(t, err, "Should be able to create OPTIONS request")
req.Header.Set("Origin", "https://example.com")
@ -375,7 +375,7 @@ func TestCORSMethodMatching(t *testing.T) {
t.Run(fmt.Sprintf("method_%s", tc.method), func(t *testing.T) {
httpClient := &http.Client{Timeout: 10 * time.Second}
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", defaultConfig.Endpoint, bucketName), nil)
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
require.NoError(t, err, "Should be able to create OPTIONS request")
req.Header.Set("Origin", "https://example.com")
@ -434,7 +434,7 @@ func TestCORSMultipleRulesMatching(t *testing.T) {
// Test first rule
httpClient := &http.Client{Timeout: 10 * time.Second}
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", defaultConfig.Endpoint, bucketName), nil)
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
require.NoError(t, err, "Should be able to create OPTIONS request")
req.Header.Set("Origin", "https://example.com")
@ -451,7 +451,7 @@ func TestCORSMultipleRulesMatching(t *testing.T) {
assert.Equal(t, "3600", resp.Header.Get("Access-Control-Max-Age"), "Should have first rule's max age")
// Test second rule
req2, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", defaultConfig.Endpoint, bucketName), nil)
req2, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
require.NoError(t, err, "Should be able to create OPTIONS request")
req2.Header.Set("Origin", "https://api.example.com")

9
test/s3/cors/s3_cors_test.go

@ -28,8 +28,10 @@ type S3TestConfig struct {
SkipVerifySSL bool
}
// Default test configuration - should match test_config.json
var defaultConfig = &S3TestConfig{
// getDefaultConfig returns a fresh instance of the default test configuration
// to avoid parallel test issues with global mutable state
func getDefaultConfig() *S3TestConfig {
return &S3TestConfig{
Endpoint: "http://localhost:8333", // Default SeaweedFS S3 port
AccessKey: "some_access_key1",
SecretKey: "some_secret_key1",
@ -38,9 +40,11 @@ var defaultConfig = &S3TestConfig{
UseSSL: false,
SkipVerifySSL: true,
}
}
// getS3Client creates an AWS S3 client for testing
func getS3Client(t *testing.T) *s3.Client {
defaultConfig := getDefaultConfig()
cfg, err := config.LoadDefaultConfig(context.TODO(),
config.WithRegion(defaultConfig.Region),
config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider(
@ -66,6 +70,7 @@ func getS3Client(t *testing.T) *s3.Client {
// createTestBucket creates a test bucket with a unique name
func createTestBucket(t *testing.T, client *s3.Client) string {
defaultConfig := getDefaultConfig()
bucketName := fmt.Sprintf("%s%d", defaultConfig.BucketPrefix, time.Now().UnixNano())
_, err := client.CreateBucket(context.TODO(), &s3.CreateBucketInput{

21
weed/s3api/cors/middleware.go

@ -2,7 +2,6 @@ package cors
import (
"net/http"
"strings"
"github.com/seaweedfs/seaweedfs/weed/glog"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
@ -149,23 +148,3 @@ func (m *Middleware) HandleOptionsRequest(w http.ResponseWriter, r *http.Request
ApplyHeaders(w, corsResp)
w.WriteHeader(http.StatusOK)
}
// ShouldApplyCORS checks if CORS should be applied to the current request
func ShouldApplyCORS(r *http.Request) bool {
// Apply CORS to all bucket and object operations
path := r.URL.Path
// Skip CORS for service-level operations
if path == "/" || path == "" {
return false
}
// Skip CORS for bucket listing
if strings.HasPrefix(path, "/") && !strings.Contains(path[1:], "/") {
// This is a bucket-level operation
return true
}
// This is an object-level operation
return true
}

14
weed/s3api/s3api_bucket_config.go

@ -3,6 +3,8 @@ package s3api
import (
"encoding/json"
"fmt"
"path/filepath"
"strings"
"sync"
"time"
@ -255,6 +257,18 @@ func (s3a *S3ApiServer) removeBucketConfigKey(bucket, key string) s3err.ErrorCod
// loadCORSFromMetadata loads CORS configuration from bucket metadata
func (s3a *S3ApiServer) loadCORSFromMetadata(bucket string) (*cors.CORSConfiguration, error) {
// Validate bucket name to prevent path traversal attacks
if bucket == "" || strings.Contains(bucket, "/") || strings.Contains(bucket, "\\") ||
strings.Contains(bucket, "..") || strings.Contains(bucket, "~") {
return nil, fmt.Errorf("invalid bucket name: %s", bucket)
}
// Clean the bucket name further to prevent any potential path traversal
bucket = filepath.Clean(bucket)
if bucket == "." || bucket == ".." {
return nil, fmt.Errorf("invalid bucket name: %s", bucket)
}
bucketMetadataPath := fmt.Sprintf("%s/%s/.s3metadata", s3a.option.BucketsPath, bucket)
entry, err := s3a.getEntry("", bucketMetadataPath)

20
weed/s3api/s3api_object_handlers.go

@ -20,6 +20,17 @@ import (
util_http "github.com/seaweedfs/seaweedfs/weed/util/http"
)
// corsHeaders defines the CORS headers that need to be preserved
// Package-level constant to avoid repeated allocations
var corsHeaders = []string{
"Access-Control-Allow-Origin",
"Access-Control-Allow-Methods",
"Access-Control-Allow-Headers",
"Access-Control-Expose-Headers",
"Access-Control-Max-Age",
"Access-Control-Allow-Credentials",
}
func mimeDetect(r *http.Request, dataReader io.Reader) io.ReadCloser {
mimeBuffer := make([]byte, 512)
size, _ := dataReader.Read(mimeBuffer)
@ -384,14 +395,7 @@ func setUserMetadataKeyToLowercase(resp *http.Response) {
func passThroughResponse(proxyResponse *http.Response, w http.ResponseWriter) (statusCode int, bytesTransferred int64) {
// Preserve existing CORS headers that may have been set by middleware
existingCORSHeaders := make(map[string]string)
for _, corsHeader := range []string{
"Access-Control-Allow-Origin",
"Access-Control-Allow-Methods",
"Access-Control-Allow-Headers",
"Access-Control-Expose-Headers",
"Access-Control-Max-Age",
"Access-Control-Allow-Credentials",
} {
for _, corsHeader := range corsHeaders {
if value := w.Header().Get(corsHeader); value != "" {
existingCORSHeaders[corsHeader] = value
}

Loading…
Cancel
Save