unset tagging when directive is unspecified

Author: Sean O'Brien

features/multipart/s3.feature

@@ -66,7 +66,7 @@ Feature: S3 Multipart Uploads
   Scenario: Caller-supplied Tagging does not trigger REPLACE
     Given I have an s3 client and an uploaded file named "tags-no-auto" with tags
     When I call multipartCopy on "tags-no-auto" with caller-supplied Tagging "k=v&Project=X" only
-    Then the copied file "tags-no-auto-copy" should have tags "k=v&Project=X"
+    Then the copied file "tags-no-auto-copy" should have no tags
 
   @s3annotations
   Scenario: tags_directive=COPY copies tags to the destination

src/S3/MultipartCopy.php

@@ -115,11 +115,11 @@ class MultipartCopy extends AbstractUploadManager
      *   and any matching caller-supplied fields are dropped. When 'REPLACE',
      *   no source metadata is read and caller-supplied params are used as-is.
      * - tags_directive: (string, default='UNSPECIFIED') 'UNSPECIFIED', 'COPY',
-     *   or 'REPLACE'. UNSPECIFIED means no tag work. COPY reads source tags
-     *   via GetObjectTagging and writes to the destination via
-     *   PutObjectTagging after CompleteMultipartUpload. REPLACE skips the
-     *   read and writes caller-supplied `params['Tagging']` to the
-     *   destination.
+     *   or 'REPLACE'. UNSPECIFIED means no tag work and any caller-supplied
+     *   `params['Tagging']` is dropped. COPY reads source tags via GetObjectTagging
+     *   and writes them to the destination via PutObjectTagging after
+     *   CompleteMultipartUpload. REPLACE skips the read and writes
+     *   caller-supplied `params['Tagging']` to the destination.
      * - annotations_directive: (string, default='UNSPECIFIED') 'UNSPECIFIED',
      *   'COPY', or 'EXCLUDE'. UNSPECIFIED and EXCLUDE both skip annotation
      *   work. COPY reads source annotations via ListObjectAnnotations and
@@ -417,13 +417,11 @@ protected function getInitiateParams()
             }
         }
 
-        // Tags are written separately in Phase 3 to avoid x-amz-tagging header bloat.
-        $tagsDir = $this->resolveTagsDirective();
-        if ($tagsDir === self::TAGS_DIRECTIVE_COPY
-            || $tagsDir === self::TAGS_DIRECTIVE_REPLACE
-        ) {
-            unset($params['Tagging']);
-        }
+        // When tags_directive is UNSPECIFIED, no tag work
+        // happens at all and any caller-supplied
+        // params['Tagging'] is dropped here too — callers who need their
+        // tags applied must opt in via tags_directive='REPLACE'.
+        unset($params['Tagging']);
 
         return $params;
     }

tests/S3/MultipartCopyTest.php

@@ -1040,10 +1040,8 @@ function (CommandInterface $cmd) use (&$captured) {
     public function testCallerSuppliedTaggingDoesNotTriggerReplace()
     {
         // Caller-supplied params['Tagging'] does NOT auto-flip tags_directive.
-        // With no explicit tags_directive (the default), the resolver stays
-        // at UNSPECIFIED. No Phase 1 read,
-        // no Phase 3 PUT. Caller's Tagging rides on CreateMultipartUpload as
-        // before, preserving legacy initiate-time tagging behavior.
+        // Callers who need their Tagging applied must opt in via
+        // tags_directive='REPLACE'.
         $client = $this->getTestClient('s3');
         $this->addMockResults($client, [
             new Result(['UploadId' => 'baz']),
@@ -1075,7 +1073,11 @@ function (CommandInterface $cmd) use (&$observed, &$initiate) {
         $this->assertNotContains('GetObjectTagging', $observed);
         $this->assertNotContains('PutObjectTagging', $observed);
         $this->assertNotNull($initiate);
-        $this->assertSame('Project=Override&Env=prod', $initiate['Tagging']);
+        $this->assertNull(
+            $initiate['Tagging'] ?? null,
+            'UNSPECIFIED must drop caller-supplied Tagging from '
+            . 'CreateMultipartUpload — MPU never carries Tagging on initiate.'
+        );
     }
 
     public function testAnnotationPutTransientFailureIsRetried()
@@ -1739,14 +1741,13 @@ function (CommandInterface $cmd) use (&$initiate, &$putTagging) {
         );
     }
 
-    public function testTagsDirectiveUnspecifiedLeavesCallerTaggingOnInitiate()
+    public function testTagsDirectiveUnspecifiedDropsCallerTaggingFromInitiate()
     {
-        // When tags_directive resolves to UNSPECIFIED (legacy default with
-        // no caller --tagging), there is
-        // no Phase 3 tag write. Caller-supplied params['Tagging'], if any, is
-        // left on CreateMultipartUpload exactly as before, preserving
-        // backwards compatibility for callers that relied on the original
-        // initiate-time tagging behavior.
+       // When tags_directive resolves to UNSPECIFIED
+        // (the default), there is no Phase 3 tag write AND any caller-supplied
+        // params['Tagging'] is dropped from the initiate. Callers who need
+        // their Tagging applied to the destination must opt in via
+        // tags_directive='REPLACE'.
         $client = $this->getTestClient('s3');
         $this->addMockResults($client, [
             new Result(['UploadId' => 'baz']),
@@ -1772,13 +1773,18 @@ function (CommandInterface $cmd) use (&$initiate, &$observed) {
             'key'             => 'bar',
             'source_metadata' => $this->srcMeta(),
             'tags_directive'  => 'UNSPECIFIED',
-            'params'          => ['Tagging' => 'preserved=yes'],
+            'params'          => ['Tagging' => 'dropped=yes'],
         ]);
         $uploader->upload();
 
         $this->assertNotNull($initiate);
-        $this->assertSame('preserved=yes', $initiate['Tagging']);
-        // No Phase 3 tag writes.
+        $this->assertNull(
+            $initiate['Tagging'] ?? null,
+            'UNSPECIFIED must drop caller-supplied Tagging from '
+            . 'CreateMultipartUpload — MPU never carries Tagging on initiate.'
+        );
+        // No Phase 1 read, no Phase 3 write.
+        $this->assertNotContains('GetObjectTagging', $observed);
         $this->assertNotContains('PutObjectTagging', $observed);
     }
 
@@ -2108,7 +2114,7 @@ function (CommandInterface $cmd) use (&$putAnnotCmds) {
         $this->assertCount(3, $putAnnotCmds);
 
         // First attempt: no @http.delay (cold path).
-        $this->assertNull($putAnnotCmds[0]['@http']['delay'] ?? null);
+        $this->assertEmpty($putAnnotCmds[0]['@http']['delay'] ?? null);
 
         // Subsequent retries: @http.delay set to a non-negative integer
         // within the 5000ms ceiling. (Full-jitter formula allows 0 as a