Merge pull request #2008 from versity/sis/object-post-boundary-prefix

fix: remove POST object multipart boundary prefix trimming

s3api/utils/multipart_reader.go

@@ -39,8 +39,7 @@ type MultipartParser struct {
 }
 
 // NewMultipartParser creates a new streaming multipart parser.
-// boundary should be the raw boundary value from Content-Type, without the leading "--".
-// If accidentally "--<boundary>" has been passed, it is normalized.
+// boundary should be the raw boundary value from Content-Type,
 func NewMultipartParser(body io.Reader, boundary string, requestContentLength int64) (*MultipartParser, error) {
 	if body == nil {
 		debuglogger.Logf("multipart parser requires non-nil body reader")
@@ -52,7 +51,6 @@ func NewMultipartParser(body io.Reader, boundary string, requestContentLength in
 	}
 
 	boundary = strings.TrimSpace(boundary)
-	boundary = strings.TrimPrefix(boundary, "--")
 	if boundary == "" {
 		debuglogger.Logf("multipart boundary is empty")
 		return nil, s3err.GetAPIError(s3err.ErrMalformedPOSTRequest)

s3api/utils/multipart_reader_test.go

@@ -100,28 +100,28 @@ func TestMultipartParserParseSuccess(t *testing.T) {
 	t.Parallel()
 
 	body := strings.Join([]string{
-		"--abc\r\n",
+		"----abc\r\n",
 		"Content-Disposition: form-data; name=\"key\"\r\n",
 		"\r\n",
 		"uploads/photo.jpg\r\n",
-		"--abc\r\n",
+		"----abc\r\n",
 		"Content-Disposition: form-data; name=\"success_action_status\"\r\n",
 		"\r\n",
 		"201\r\n",
-		"--abc\r\n",
+		"----abc\r\n",
 		"Content-Disposition: form-data; name=\"x-amz-meta-color\"\r\n",
 		"\r\n",
 		"blue\r\n",
-		"--abc\r\n",
+		"----abc\r\n",
 		"Content-Disposition: form-data; name=\"x-amz-meta-color\"\r\n",
 		"\r\n",
 		"green\r\n",
-		"--abc\r\n",
+		"----abc\r\n",
 		"Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n",
 		"Content-Type: image/jpeg\r\n",
 		"\r\n",
 		"file-body-123",
-		"\r\n--abc--\r\n",
+		"\r\n----abc--\r\n",
 	}, "")
 
 	mp := newMultipartParserForTest(t, body, "--abc")

tests/integration/PostObject.go

@@ -20,6 +20,7 @@ import (
 	"encoding/xml"
 	"fmt"
 	"io"
+	"mime/multipart"
 	"net/http"
 	"strings"
 	"time"
@@ -1096,3 +1097,84 @@ func PostObject_checksums_success(s *S3Conf) error {
 		return nil
 	})
 }
+
+func PostObject_success_double_dash_boundary(s *S3Conf) error {
+	testName := "PostObject_success_double_dash_boundary"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		key := "test-object"
+		objData := []byte("hello world")
+		signingDate := time.Now().UTC()
+
+		fields := buildSignedPostFields(bucket, key, s.awsID, s.awsRegion, signingDate)
+		policy, err := encodePostPolicy([]any{}, time.Now().UTC().Add(10*time.Minute), fields, make(map[string]struct{}))
+		if err != nil {
+			return err
+		}
+		fields["policy"] = policy
+		fields["x-amz-signature"] = signPostPolicy(policy, signingDate.Format("20060102"), s.awsRegion, s.awsSecret)
+
+		var buf bytes.Buffer
+		w := multipart.NewWriter(&buf)
+		if err := w.SetBoundary("--custom-post-boundary"); err != nil {
+			return fmt.Errorf("failed to set boundary: %w", err)
+		}
+
+		for k, v := range fields {
+			if err := w.WriteField(k, v); err != nil {
+				return err
+			}
+		}
+
+		fw, err := w.CreateFormFile("file", "upload.bin")
+		if err != nil {
+			return err
+		}
+		if _, err = fw.Write(objData); err != nil {
+			return err
+		}
+		if err := w.Close(); err != nil {
+			return err
+		}
+
+		boundary := w.Boundary()
+		body := buf.Bytes()
+
+		req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/%s", s.endpoint, bucket), bytes.NewReader(body))
+		if err != nil {
+			return err
+		}
+		req.Header.Set("Content-Type", fmt.Sprintf("multipart/form-data; boundary=%s", boundary))
+		req.ContentLength = int64(len(body))
+
+		resp, err := s.httpClient.Do(req)
+		if err != nil {
+			return err
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusNoContent {
+			return fmt.Errorf("expected status 204, got %d", resp.StatusCode)
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		res, err := s3client.GetObject(ctx, &s3.GetObjectInput{
+			Bucket: &bucket,
+			Key:    &key,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+		defer res.Body.Close()
+
+		gotData, err := io.ReadAll(res.Body)
+		if err != nil {
+			return err
+		}
+		if !bytes.Equal(gotData, objData) {
+			return fmt.Errorf("expected object data %q, got %q", objData, gotData)
+		}
+
+		return nil
+	})
+}

tests/integration/group-tests.go

@@ -1175,6 +1175,7 @@ func TestPostObject(ts *TestState) {
 	ts.Run(PostObject_success_with_meta_properties)
 	ts.Run(PostObject_invalid_tagging)
 	ts.Run(PostObject_success_with_tagging)
+	ts.Run(PostObject_success_double_dash_boundary)
 	if !ts.conf.azureTests {
 		ts.Run(PostObject_invalid_checksum_value)
 		ts.Run(PostObject_invalid_checksum_algorithm)
@@ -1981,5 +1982,6 @@ func GetIntTests() IntTests {
 		"PostObject_invalid_checksum_algorithm":                                    PostObject_invalid_checksum_algorithm,
 		"PostObject_multiple_checksum_headers":                                     PostObject_multiple_checksum_headers,
 		"PostObject_checksums_success":                                             PostObject_checksums_success,
+		"PostObject_success_double_dash_boundary":                                  PostObject_success_double_dash_boundary,
 	}
 }