s3: fix presigned POST upload missing slash between bucket and key (#7714)

* s3: fix presigned POST upload missing slash between bucket and key

When uploading a file using presigned POST (e.g., boto3.generate_presigned_post),
the file was saved with the bucket name and object key concatenated without a
slash (e.g., 'my-bucketfilename' instead of 'my-bucket/filename').

The issue was that PostPolicyBucketHandler retrieved the object key from form
values without ensuring it had a leading slash, unlike GetBucketAndObject()
which normalizes the key.

Fixes #7713

* s3: add tests for presigned POST key normalization

Add comprehensive tests for PostPolicyBucketHandler to ensure:
- Object keys without leading slashes are properly normalized
- ${filename} substitution works correctly with normalization
- Path construction correctly separates bucket and key
- Form value extraction works properly

These tests would have caught the bug fixed in the previous commit
where keys like 'test_image.png' were concatenated with bucket
without a separator, resulting in 'my-buckettest_image.png'.

* s3: create normalizeObjectKey function for robust key normalization

Address review feedback by creating a reusable normalizeObjectKey function
that both adds a leading slash and removes duplicate slashes, aligning with
how other handlers process paths (e.g., toFilerPath uses removeDuplicateSlashes).

The function handles edge cases like:
- Keys without leading slashes (the original bug)
- Keys with duplicate slashes (e.g., 'a//b' -> '/a/b')
- Keys with leading duplicate slashes (e.g., '///a' -> '/a')

Updated tests to use the new function and added TestNormalizeObjectKey
for comprehensive coverage of the new function.

* s3: move NormalizeObjectKey to s3_constants for shared use

Move the NormalizeObjectKey function to the s3_constants package so it can
be reused by:
- GetBucketAndObject() - now normalizes all object keys from URL paths
- GetPrefix() - now normalizes prefix query parameters
- PostPolicyBucketHandler - normalizes keys from form values

This ensures consistent object key normalization across all S3 API handlers,
handling both missing leading slashes and duplicate slashes.

Benefits:
- Single source of truth for key normalization
- GetBucketAndObject now removes duplicate slashes (previously only added leading slash)
- All handlers benefit from the improved normalization automatically

1 files changed, 383 insertions, 0 deletions

diff --git a/weed/s3api/s3api_object_handlers_postpolicy_test.go b/weed/s3api/s3api_object_handlers_postpolicy_test.go
new file mode 100644
index 000000000..357fb9c7c
--- /dev/null
+++ b/weed/s3api/s3api_object_handlers_postpolicy_test.go
@@ -0,0 +1,383 @@
+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 with a leading slash and duplicate slashes removed.
+// This addresses issue #7713 where keys without leading slashes caused
+// bucket and key to be concatenated without a separator.
+func TestPostPolicyKeyNormalization(t *testing.T) {
+	tests := []struct {
+		name           string
+		key            string
+		expectedPrefix string // Expected path prefix after bucket
+	}{
+		{
+			name:           "key without leading slash",
+			key:            "test_image.png",
+			expectedPrefix: "/test_image.png",
+		},
+		{
+			name:           "key with leading slash",
+			key:            "/test_image.png",
+			expectedPrefix: "/test_image.png",
+		},
+		{
+			name:           "key with path without leading slash",
+			key:            "folder/subfolder/test_image.png",
+			expectedPrefix: "/folder/subfolder/test_image.png",
+		},
+		{
+			name:           "key with path with leading slash",
+			key:            "/folder/subfolder/test_image.png",
+			expectedPrefix: "/folder/subfolder/test_image.png",
+		},
+		{
+			name:           "simple filename",
+			key:            "file.txt",
+			expectedPrefix: "/file.txt",
+		},
+		{
+			name:           "key with duplicate slashes",
+			key:            "folder//subfolder///file.txt",
+			expectedPrefix: "/folder/subfolder/file.txt",
+		},
+		{
+			name:           "key with leading duplicate slashes",
+			key:            "//folder/file.txt",
+			expectedPrefix: "/folder/file.txt",
+		},
+		{
+			name:           "key with trailing slash",
+			key:            "folder/",
+			expectedPrefix: "/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 has the expected prefix
+			assert.Equal(t, tt.expectedPrefix, object,
+				"Key should be normalized correctly")
+
+			// Verify path construction would be correct
+			bucket := "my-bucket"
+			bucketsPath := "/buckets"
+			expectedPath := bucketsPath + "/" + bucket + tt.expectedPrefix
+			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/"},
+	}
+
+	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")
+		})
+	}
+}