* Replace removeDuplicateSlashes with NormalizeObjectKey Use s3_constants.NormalizeObjectKey instead of removeDuplicateSlashes in most places for consistency. NormalizeObjectKey handles both duplicate slash removal and ensures the path starts with '/', providing more complete normalization. * Fix double slash issues after NormalizeObjectKey After using NormalizeObjectKey, object keys have a leading '/'. This commit ensures: - getVersionedObjectDir strips leading slash before concatenation - getEntry calls receive names without leading slash - String concatenation with '/' doesn't create '//' paths This prevents path construction errors like: /buckets/bucket//object (wrong) /buckets/bucket/object (correct) * ensure object key leading "/" * fix compilation * fix: Strip leading slash from object keys in S3 API responses After introducing NormalizeObjectKey, all internal object keys have a leading slash. However, S3 API responses must return keys without leading slashes to match AWS S3 behavior. Fixed in three functions: - addVersion: Strip slash for version list entries - processRegularFile: Strip slash for regular file entries - processExplicitDirectory: Strip slash for directory entries This ensures ListObjectVersions and similar APIs return keys like 'bar' instead of '/bar', matching S3 API specifications. * fix: Normalize keyMarker for consistent pagination comparison The S3 API provides keyMarker without a leading slash (e.g., 'object-001'), but after introducing NormalizeObjectKey, all internal object keys have leading slashes (e.g., '/object-001'). When comparing keyMarker < normalizedObjectKey in shouldSkipObjectForMarker, the ASCII value of '/' (47) is less than 'o' (111), causing all objects to be incorrectly skipped during pagination. This resulted in page 2 and beyond returning 0 results. Fix: Normalize the keyMarker when creating versionCollector so comparisons work correctly with normalized object keys. Fixes pagination tests: - TestVersioningPaginationOver1000Versions - TestVersioningPaginationMultipleObjectsManyVersions * refactor: Change NormalizeObjectKey to return keys without leading slash BREAKING STRATEGY CHANGE: Previously, NormalizeObjectKey added a leading slash to all object keys, which required stripping it when returning keys to S3 API clients and caused complexity in marker normalization for pagination. NEW STRATEGY: - NormalizeObjectKey now returns keys WITHOUT leading slash (e.g., 'foo/bar' not '/foo/bar') - This matches the S3 API format directly - All path concatenations now explicitly add '/' between bucket and object - No need to strip slashes in responses or normalize markers Changes: 1. Modified NormalizeObjectKey to strip leading slash instead of adding it 2. Fixed all path concatenations to use: - BucketsPath + '/' + bucket + '/' + object instead of: - BucketsPath + '/' + bucket + object 3. Reverted response key stripping in: - addVersion() - processRegularFile() - processExplicitDirectory() 4. Reverted keyMarker normalization in findVersionsRecursively() 5. Updated matchesPrefixFilter() to work with keys without leading slash 6. Fixed paths in handlers: - s3api_object_handlers.go (GetObject, HeadObject, cacheRemoteObjectForStreaming) - s3api_object_handlers_postpolicy.go - s3api_object_handlers_tagging.go - s3api_object_handlers_acl.go - s3api_version_id.go (getVersionedObjectDir, getVersionIdFormat) - s3api_object_versioning.go (getObjectVersionList, updateLatestVersionAfterDeletion) All versioning tests pass including pagination stress tests. * adjust format * Update post policy tests to match new NormalizeObjectKey behavior - Update TestPostPolicyKeyNormalization to expect keys without leading slashes - Update TestNormalizeObjectKey to expect keys without leading slashes - Update TestPostPolicyFilenameSubstitution to expect keys without leading slashes - Update path construction in tests to use new pattern: BucketsPath + '/' + bucket + '/' + object * Fix ListObjectVersions prefix filtering Remove leading slash addition to prefix parameter to allow correct filtering of .versions directories when listing object versions with a specific prefix. The prefix parameter should match entry paths relative to bucket root. Adding a leading slash was breaking the prefix filter for paginated requests. Fixes pagination issue where second page returned 0 versions instead of continuing with remaining versions. * no leading slash * Fix urlEscapeObject to add leading slash for filer paths NormalizeObjectKey now returns keys without leading slashes to match S3 API format. However, urlEscapeObject is used for filer paths which require leading slashes. Add leading slash back after normalization to ensure filer paths are correct. Fixes TestS3ApiServer_toFilerPath test failures. * adjust tests * normalize * Fix: Normalize prefixes and markers in LIST operations using NormalizeObjectKey Ensure consistent key normalization across all S3 operations (GET, PUT, LIST). Previously, LIST operations were not applying the same normalization rules (handling backslashes, duplicate slashes, leading slashes) as GET/PUT operations. Changes: - Updated normalizePrefixMarker() to call NormalizeObjectKey for both prefix and marker - This ensures prefixes with leading slashes, backslashes, or duplicate slashes are handled consistently with how object keys are normalized - Fixes Parquet test failures where pads.write_dataset creates implicit directory structures that couldn't be discovered by subsequent LIST operations - Added TestPrefixNormalizationInList and TestListPrefixConsistency tests All existing LIST tests continue to pass with the normalization improvements. * Add debugging logging to LIST operations to track prefix normalization * Fix: Remove leading slash addition from GetPrefix to work with NormalizeObjectKey The NormalizeObjectKey function removes leading slashes to match S3 API format (e.g., 'foo/bar' not '/foo/bar'). However, GetPrefix was adding a leading slash back, which caused LIST operations to fail with incorrect path handling. Now GetPrefix only normalizes duplicate slashes without adding a leading slash, which allows NormalizeObjectKey changes to work correctly for S3 LIST operations. All Parquet integration tests now pass (20/20). * Fix: Handle object paths without leading slash in checkDirectoryObject NormalizeObjectKey() removes the leading slash to match S3 API format. However, checkDirectoryObject() was assuming the object path has a leading slash when processing directory markers (paths ending with '/'). Now we ensure the object has a leading slash before processing it for filer operations. Fixes implicit directory marker test (explicit_dir/) while keeping Parquet integration tests passing (20/20). All tests pass: - Implicit directory tests: 6/6 - Parquet integration tests: 20/20 * Fix: Handle explicit directory markers with trailing slashes Explicit directory markers created with put_object(Key='dir/', ...) are stored in the filer with the trailing slash as part of the name. The checkDirectoryObject() function now checks for both: 1. Explicit directories: lookup with trailing slash preserved (e.g., 'explicit_dir/') 2. Implicit directories: lookup without trailing slash (e.g., 'implicit_dir') This ensures both types of directory markers are properly recognized. All tests pass: - Implicit directory tests: 6/6 (including explicit directory marker test) - Parquet integration tests: 20/20 * Fix: Preserve trailing slash in NormalizeObjectKey NormalizeObjectKey now preserves trailing slashes when normalizing object keys. This is important for explicit directory markers like 'explicit_dir/' which rely on the trailing slash to be recognized as directory objects. The normalization process: 1. Notes if trailing slash was present 2. Removes duplicate slashes and converts backslashes 3. Removes leading slash for S3 API format 4. Restores trailing slash if it was in the original This ensures explicit directory markers created with put_object(Key='dir/', ...) are properly normalized and can be looked up by their exact name. All tests pass: - Implicit directory tests: 6/6 - Parquet integration tests: 20/20 * clean object * Fix: Don't restore trailing slash if result is empty When normalizing paths that are only slashes (e.g., '///', '/'), the function should return an empty string, not a single slash. The fix ensures we only restore the trailing slash if the result is non-empty. This fixes the 'just_slashes' test case: - Input: '///' - Expected: '' - Previous: '/' - Fixed: '' All tests now pass: - Unit tests: TestNormalizeObjectKey (13/13) - Implicit directory tests: 6/6 - Parquet integration tests: 20/20 * prefixEndsOnDelimiter * Update s3api_object_handlers_list.go * Update s3api_object_handlers_list.go * handle create directory
386 lines
11 KiB
Go
386 lines
11 KiB
Go
package s3api
|
|
|
|
import (
|
|
"bytes"
|
|
"io"
|
|
"mime/multipart"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/gorilla/mux"
|
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
// TestPostPolicyKeyNormalization tests that object keys from presigned POST
|
|
// are properly normalized without leading slashes and with duplicate slashes removed.
|
|
// This ensures consistent key handling across the S3 API.
|
|
func TestPostPolicyKeyNormalization(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
key string
|
|
expectedObject string // Expected normalized object key
|
|
}{
|
|
{
|
|
name: "key without leading slash",
|
|
key: "test_image.png",
|
|
expectedObject: "test_image.png",
|
|
},
|
|
{
|
|
name: "key with leading slash",
|
|
key: "/test_image.png",
|
|
expectedObject: "test_image.png",
|
|
},
|
|
{
|
|
name: "key with path without leading slash",
|
|
key: "folder/subfolder/test_image.png",
|
|
expectedObject: "folder/subfolder/test_image.png",
|
|
},
|
|
{
|
|
name: "key with path with leading slash",
|
|
key: "/folder/subfolder/test_image.png",
|
|
expectedObject: "folder/subfolder/test_image.png",
|
|
},
|
|
{
|
|
name: "simple filename",
|
|
key: "file.txt",
|
|
expectedObject: "file.txt",
|
|
},
|
|
{
|
|
name: "key with duplicate slashes",
|
|
key: "folder//subfolder///file.txt",
|
|
expectedObject: "folder/subfolder/file.txt",
|
|
},
|
|
{
|
|
name: "key with leading duplicate slashes",
|
|
key: "//folder/file.txt",
|
|
expectedObject: "folder/file.txt",
|
|
},
|
|
{
|
|
name: "key with trailing slash",
|
|
key: "folder/",
|
|
expectedObject: "folder/",
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
// Use the actual NormalizeObjectKey function
|
|
object := s3_constants.NormalizeObjectKey(tt.key)
|
|
|
|
// Verify the normalized object matches expected
|
|
assert.Equal(t, tt.expectedObject, object,
|
|
"Key should be normalized correctly")
|
|
|
|
// Verify path construction would be correct
|
|
bucket := "my-bucket"
|
|
bucketsPath := "/buckets"
|
|
expectedPath := bucketsPath + "/" + bucket + "/" + tt.expectedObject
|
|
actualPath := bucketsPath + "/" + bucket + "/" + object
|
|
|
|
assert.Equal(t, expectedPath, actualPath,
|
|
"File path should be correctly constructed with slash between bucket and key")
|
|
|
|
// Verify we don't have double slashes (except at the start which is fine)
|
|
assert.NotContains(t, actualPath[1:], "//",
|
|
"Path should not contain double slashes")
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestNormalizeObjectKey tests the NormalizeObjectKey function directly
|
|
func TestNormalizeObjectKey(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
input string
|
|
expected string
|
|
}{
|
|
{"empty string", "", ""},
|
|
{"simple file", "file.txt", "file.txt"},
|
|
{"with leading slash", "/file.txt", "file.txt"},
|
|
{"path without slash", "a/b/c.txt", "a/b/c.txt"},
|
|
{"path with slash", "/a/b/c.txt", "a/b/c.txt"},
|
|
{"duplicate slashes", "a//b///c.txt", "a/b/c.txt"},
|
|
{"leading duplicates", "///a/b.txt", "a/b.txt"},
|
|
{"all duplicates", "//a//b//", "a/b/"},
|
|
{"just slashes", "///", ""},
|
|
{"trailing slash", "folder/", "folder/"},
|
|
{"backslash to forward slash", "folder\\file.txt", "folder/file.txt"},
|
|
{"windows path", "folder\\subfolder\\file.txt", "folder/subfolder/file.txt"},
|
|
{"mixed slashes", "a/b\\c/d", "a/b/c/d"},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
result := s3_constants.NormalizeObjectKey(tt.input)
|
|
assert.Equal(t, tt.expected, result)
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestPostPolicyFilenameSubstitution tests the ${filename} substitution in keys
|
|
func TestPostPolicyFilenameSubstitution(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
keyTemplate string
|
|
uploadedFilename string
|
|
expectedKey string
|
|
}{
|
|
{
|
|
name: "filename at end",
|
|
keyTemplate: "uploads/${filename}",
|
|
uploadedFilename: "photo.jpg",
|
|
expectedKey: "uploads/photo.jpg",
|
|
},
|
|
{
|
|
name: "filename in middle",
|
|
keyTemplate: "user/files/${filename}/original",
|
|
uploadedFilename: "document.pdf",
|
|
expectedKey: "user/files/document.pdf/original",
|
|
},
|
|
{
|
|
name: "no substitution needed",
|
|
keyTemplate: "static/file.txt",
|
|
uploadedFilename: "ignored.txt",
|
|
expectedKey: "static/file.txt",
|
|
},
|
|
{
|
|
name: "filename only",
|
|
keyTemplate: "${filename}",
|
|
uploadedFilename: "myfile.png",
|
|
expectedKey: "myfile.png",
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
// Simulate the substitution logic from PostPolicyBucketHandler
|
|
key := tt.keyTemplate
|
|
if tt.uploadedFilename != "" && strings.Contains(key, "${filename}") {
|
|
key = strings.Replace(key, "${filename}", tt.uploadedFilename, -1)
|
|
}
|
|
|
|
// Normalize using the actual function
|
|
object := s3_constants.NormalizeObjectKey(key)
|
|
|
|
assert.Equal(t, tt.expectedKey, object,
|
|
"Key should be correctly substituted and normalized")
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestExtractPostPolicyFormValues tests the form value extraction
|
|
func TestExtractPostPolicyFormValues(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
key string
|
|
contentType string
|
|
fileContent string
|
|
fileName string
|
|
expectSuccess bool
|
|
}{
|
|
{
|
|
name: "basic upload",
|
|
key: "test.txt",
|
|
contentType: "text/plain",
|
|
fileContent: "hello world",
|
|
fileName: "upload.txt",
|
|
expectSuccess: true,
|
|
},
|
|
{
|
|
name: "upload with path key",
|
|
key: "folder/subfolder/test.txt",
|
|
contentType: "application/octet-stream",
|
|
fileContent: "binary data",
|
|
fileName: "data.bin",
|
|
expectSuccess: true,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
// Create multipart form
|
|
var buf bytes.Buffer
|
|
writer := multipart.NewWriter(&buf)
|
|
|
|
// Add form fields
|
|
writer.WriteField("key", tt.key)
|
|
writer.WriteField("Content-Type", tt.contentType)
|
|
|
|
// Add file
|
|
part, err := writer.CreateFormFile("file", tt.fileName)
|
|
assert.NoError(t, err)
|
|
_, err = io.WriteString(part, tt.fileContent)
|
|
assert.NoError(t, err)
|
|
|
|
err = writer.Close()
|
|
assert.NoError(t, err)
|
|
|
|
// Parse the form
|
|
reader := multipart.NewReader(&buf, writer.Boundary())
|
|
form, err := reader.ReadForm(5 * 1024 * 1024)
|
|
assert.NoError(t, err)
|
|
defer form.RemoveAll()
|
|
|
|
// Extract values using the actual function
|
|
filePart, fileName, fileContentType, fileSize, formValues, err := extractPostPolicyFormValues(form)
|
|
|
|
if tt.expectSuccess {
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, filePart)
|
|
assert.Equal(t, tt.fileName, fileName)
|
|
assert.NotEmpty(t, fileContentType)
|
|
assert.Greater(t, fileSize, int64(0))
|
|
assert.Equal(t, tt.key, formValues.Get("Key"))
|
|
|
|
filePart.Close()
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestPostPolicyPathConstruction is an integration-style test that verifies
|
|
// the complete path construction logic
|
|
func TestPostPolicyPathConstruction(t *testing.T) {
|
|
s3a := &S3ApiServer{
|
|
option: &S3ApiServerOption{
|
|
BucketsPath: "/buckets",
|
|
},
|
|
}
|
|
|
|
tests := []struct {
|
|
name string
|
|
bucket string
|
|
formKey string // Key as it would come from form (may not have leading slash)
|
|
expectedPath string
|
|
}{
|
|
{
|
|
name: "simple key without slash - the bug case",
|
|
bucket: "my-bucket",
|
|
formKey: "test_image.png",
|
|
expectedPath: "/buckets/my-bucket/test_image.png",
|
|
},
|
|
{
|
|
name: "simple key with slash",
|
|
bucket: "my-bucket",
|
|
formKey: "/test_image.png",
|
|
expectedPath: "/buckets/my-bucket/test_image.png",
|
|
},
|
|
{
|
|
name: "nested path without leading slash",
|
|
bucket: "uploads",
|
|
formKey: "2024/01/photo.jpg",
|
|
expectedPath: "/buckets/uploads/2024/01/photo.jpg",
|
|
},
|
|
{
|
|
name: "nested path with leading slash",
|
|
bucket: "uploads",
|
|
formKey: "/2024/01/photo.jpg",
|
|
expectedPath: "/buckets/uploads/2024/01/photo.jpg",
|
|
},
|
|
{
|
|
name: "key with duplicate slashes",
|
|
bucket: "my-bucket",
|
|
formKey: "folder//file.txt",
|
|
expectedPath: "/buckets/my-bucket/folder/file.txt",
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
// Use the actual NormalizeObjectKey function
|
|
object := s3_constants.NormalizeObjectKey(tt.formKey)
|
|
|
|
// Construct path as done in PostPolicyBucketHandler
|
|
filePath := s3a.option.BucketsPath + "/" + tt.bucket + "/" + object
|
|
|
|
assert.Equal(t, tt.expectedPath, filePath,
|
|
"File path should be correctly constructed")
|
|
|
|
// Verify bucket and key are properly separated
|
|
assert.Contains(t, filePath, tt.bucket+"/",
|
|
"Bucket should be followed by a slash")
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestPostPolicyBucketHandlerKeyExtraction tests that the handler correctly
|
|
// extracts and normalizes the key from a POST request
|
|
func TestPostPolicyBucketHandlerKeyExtraction(t *testing.T) {
|
|
// Create a minimal S3ApiServer for testing
|
|
s3a := &S3ApiServer{
|
|
option: &S3ApiServerOption{
|
|
BucketsPath: "/buckets",
|
|
},
|
|
iam: &IdentityAccessManagement{},
|
|
}
|
|
|
|
tests := []struct {
|
|
name string
|
|
bucket string
|
|
key string
|
|
wantPathHas string // substring that must be in the constructed path
|
|
}{
|
|
{
|
|
name: "key without leading slash",
|
|
bucket: "test-bucket",
|
|
key: "simple-file.txt",
|
|
wantPathHas: "/test-bucket/simple-file.txt",
|
|
},
|
|
{
|
|
name: "key with leading slash",
|
|
bucket: "test-bucket",
|
|
key: "/prefixed-file.txt",
|
|
wantPathHas: "/test-bucket/prefixed-file.txt",
|
|
},
|
|
{
|
|
name: "key with duplicate slashes",
|
|
bucket: "test-bucket",
|
|
key: "folder//nested///file.txt",
|
|
wantPathHas: "/test-bucket/folder/nested/file.txt",
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
// Create multipart form body
|
|
var buf bytes.Buffer
|
|
writer := multipart.NewWriter(&buf)
|
|
|
|
// Add required fields
|
|
writer.WriteField("key", tt.key)
|
|
writer.WriteField("Policy", "") // Empty policy for this test
|
|
|
|
// Add file
|
|
part, _ := writer.CreateFormFile("file", "test.txt")
|
|
part.Write([]byte("test content"))
|
|
writer.Close()
|
|
|
|
// Create request
|
|
req := httptest.NewRequest(http.MethodPost, "/"+tt.bucket, &buf)
|
|
req.Header.Set("Content-Type", writer.FormDataContentType())
|
|
|
|
// Set up mux vars (simulating router)
|
|
req = mux.SetURLVars(req, map[string]string{"bucket": tt.bucket})
|
|
|
|
// Parse form to extract key
|
|
reader, _ := req.MultipartReader()
|
|
form, _ := reader.ReadForm(5 * 1024 * 1024)
|
|
defer form.RemoveAll()
|
|
|
|
_, _, _, _, formValues, _ := extractPostPolicyFormValues(form)
|
|
|
|
// Apply the same normalization as PostPolicyBucketHandler
|
|
object := s3_constants.NormalizeObjectKey(formValues.Get("Key"))
|
|
|
|
// Construct path
|
|
filePath := s3a.option.BucketsPath + "/" + tt.bucket + "/" + object
|
|
|
|
assert.Contains(t, filePath, tt.wantPathHas,
|
|
"Path should contain properly separated bucket and key")
|
|
})
|
|
}
|
|
}
|