Browse Source

address comments

pull/6987/head
chrislu 3 months ago
parent
commit
b6fbedde4a
  1. 67
      test/s3/cors/s3_cors_http_test.go
  2. 9
      test/s3/cors/test_config.json
  3. 5
      weed/s3api/s3api_bucket_config.go
  4. 3
      weed/s3api/s3api_server.go

67
test/s3/cors/s3_cors_http_test.go

@ -467,3 +467,70 @@ func TestCORSMultipleRulesMatching(t *testing.T) {
assert.Contains(t, resp2.Header.Get("Access-Control-Allow-Headers"), "Authorization", "Should allow Authorization header")
assert.Equal(t, "7200", resp2.Header.Get("Access-Control-Max-Age"), "Should have second rule's max age")
}
// TestServiceLevelCORS tests that service-level endpoints (like /status) get proper CORS headers
func TestServiceLevelCORS(t *testing.T) {
assert := assert.New(t)
endpoints := []string{
"/",
"/status",
"/healthz",
}
for _, endpoint := range endpoints {
t.Run(fmt.Sprintf("endpoint_%s", strings.ReplaceAll(endpoint, "/", "_")), func(t *testing.T) {
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s%s", getDefaultConfig().Endpoint, endpoint), nil)
assert.NoError(err)
// Add Origin header to trigger CORS
req.Header.Set("Origin", "http://example.com")
client := &http.Client{}
resp, err := client.Do(req)
assert.NoError(err)
defer resp.Body.Close()
// Should return 200 OK
assert.Equal(http.StatusOK, resp.StatusCode)
// Should have CORS headers set
assert.Equal("*", resp.Header.Get("Access-Control-Allow-Origin"))
assert.Equal("*", resp.Header.Get("Access-Control-Expose-Headers"))
assert.Equal("*", resp.Header.Get("Access-Control-Allow-Methods"))
assert.Equal("*", resp.Header.Get("Access-Control-Allow-Headers"))
})
}
}
// TestServiceLevelCORSWithoutOrigin tests that service-level endpoints without Origin header don't get CORS headers
func TestServiceLevelCORSWithoutOrigin(t *testing.T) {
assert := assert.New(t)
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/status", getDefaultConfig().Endpoint), nil)
assert.NoError(err)
// No Origin header
client := &http.Client{}
resp, err := client.Do(req)
assert.NoError(err)
defer resp.Body.Close()
// Should return 200 OK
assert.Equal(http.StatusOK, resp.StatusCode)
// Should not have CORS headers set (or have empty values)
corsHeaders := []string{
"Access-Control-Allow-Origin",
"Access-Control-Expose-Headers",
"Access-Control-Allow-Methods",
"Access-Control-Allow-Headers",
}
for _, header := range corsHeaders {
value := resp.Header.Get(header)
// Headers should either be empty or not present
assert.True(value == "" || value == "*", "Header %s should be empty or wildcard, got: %s", header, value)
}
}

9
test/s3/cors/test_config.json

@ -1,9 +0,0 @@
{
"endpoint": "http://localhost:8333",
"access_key": "some_access_key1",
"secret_key": "some_secret_key1",
"region": "us-east-1",
"bucket_prefix": "test-cors-",
"use_ssl": false,
"skip_verify_ssl": true
}

5
weed/s3api/s3api_bucket_config.go

@ -273,26 +273,31 @@ func (s3a *S3ApiServer) loadCORSFromMetadata(bucket string) (*cors.CORSConfigura
entry, err := s3a.getEntry("", bucketMetadataPath)
if err != nil || entry == nil {
glog.V(3).Infof("loadCORSFromMetadata: no metadata found for bucket %s: %v", bucket, err)
return nil, fmt.Errorf("no metadata found")
}
if len(entry.Content) == 0 {
glog.V(3).Infof("loadCORSFromMetadata: empty metadata content for bucket %s", bucket)
return nil, fmt.Errorf("no metadata content")
}
var metadata map[string]json.RawMessage
if err := json.Unmarshal(entry.Content, &metadata); err != nil {
glog.Errorf("loadCORSFromMetadata: failed to unmarshal metadata for bucket %s: %v", bucket, err)
return nil, fmt.Errorf("failed to unmarshal metadata: %v", err)
}
corsData, exists := metadata["cors"]
if !exists {
glog.V(3).Infof("loadCORSFromMetadata: no CORS configuration found for bucket %s", bucket)
return nil, fmt.Errorf("no CORS configuration found")
}
// Directly unmarshal the raw JSON to CORSConfiguration to avoid round-trip allocations
var config cors.CORSConfiguration
if err := json.Unmarshal(corsData, &config); err != nil {
glog.Errorf("loadCORSFromMetadata: failed to unmarshal CORS configuration for bucket %s: %v", bucket, err)
return nil, fmt.Errorf("failed to unmarshal CORS configuration: %v", err)
}

3
weed/s3api/s3api_server.go

@ -186,7 +186,8 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
bucket.Use(corsMiddleware.Handler)
// Bucket-specific OPTIONS handler for CORS preflight requests
bucket.Methods(http.MethodOptions).HandlerFunc(corsMiddleware.HandleOptionsRequest)
// Use PathPrefix to catch all bucket-level preflight routes including /bucket/object
bucket.PathPrefix("/").Methods(http.MethodOptions).HandlerFunc(corsMiddleware.HandleOptionsRequest)
// each case should follow the next rule:
// - requesting object with query must precede any other methods

Loading…
Cancel
Save