Fix bug in V4 where copy object wasn't working for some special characters (#3710)

Fix copy object to work for all special characters

Author: peterrsongg

generator/.DevConfigs/1667b220-e786-4bc9-9852-a85e630ac8e0.json

@@ -0,0 +1,11 @@
+{
+    "services": [
+        {
+            "serviceName": "S3",
+            "type": "patch",
+            "changeLogMessages": [
+                "Fix: Fix a bug where copy object wasn't working with some special characters."
+            ]
+        }
+    ]
+}

sdk/src/Services/S3/Custom/AmazonS3Client.Extensions.cs

@@ -389,7 +389,7 @@ private static SigningResult ReturnSigningResult(SignatureVersion signatureVersi
                     Amazon.S3.Internal.S3Signer.SignRequest(iRequest, metrics, immutableCredentials.AccessKey, immutableCredentials.SecretKey);
                     signingResult.Authorization = iRequest.Headers[HeaderKeys.AuthorizationHeader];
                     signingResult.Authorization = signingResult.Authorization.Substring(signingResult.Authorization.IndexOf(":", StringComparison.Ordinal) + 1);
-                    signingResult.Authorization = "&Signature=" + AmazonS3Util.UrlEncode(signingResult.Authorization, false);
+                    signingResult.Authorization = "&Signature=" + AWSSDKUtils.UrlEncode(signingResult.Authorization, false);
                     signingResult.Result = ComposeUrl(iRequest).AbsoluteUri + signingResult.Authorization;
                     break;
             }

sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/CopyObjectRequestMarshaller.cs

@@ -164,15 +164,16 @@ static string ConstructCopySourceHeaderValue(string bucket, string key, string v
             {
                 var isAccessPoint = S3ArnUtils.IsS3AccessPointsArn(bucket) || S3ArnUtils.IsS3OutpostsArn(bucket);
                 // 'object/' needed appended to key for copy header with access points
-                source = AmazonS3Util.UrlEncode(String.Concat(bucket, isAccessPoint ? "/object/" : "/", key), !isAccessPoint);
+
+                source = AWSSDKUtils.UrlEncode(String.Concat(bucket, isAccessPoint ? "/object/" : "/", key), false);
                 if (!String.IsNullOrEmpty(version))
                 {
-                    source = string.Format(CultureInfo.InvariantCulture, "{0}?versionId={1}", source, AmazonS3Util.UrlEncode(version, true));
+                    source = string.Format(CultureInfo.InvariantCulture, "{0}?versionId={1}", source, AWSSDKUtils.UrlEncode(version, true));
                 }
             }
             else
             {
-                source = AmazonS3Util.UrlEncode(bucket, true);
+                source = AWSSDKUtils.UrlEncode(bucket, true);
             }
 
             return source;

sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/CopyPartRequestMarshaller.cs

@@ -120,15 +120,15 @@ static string ConstructCopySourceHeaderValue(string bucket, string key, string v
             {
                 var isAccessPoint = S3ArnUtils.IsS3AccessPointsArn(bucket) || S3ArnUtils.IsS3OutpostsArn(bucket);
                 // 'object/' needed appended to key for copy header with access points
-                source = AmazonS3Util.UrlEncode(String.Concat(bucket, isAccessPoint ? "/object/" : "/", key), !isAccessPoint);
+                source = AWSSDKUtils.UrlEncode(String.Concat(bucket, isAccessPoint ? "/object/" : "/", key), false);
                 if (!String.IsNullOrEmpty(version))
                 {
-                    source = string.Format(CultureInfo.InvariantCulture, "{0}?versionId={1}", source, AmazonS3Util.UrlEncode(version, true));
+                    source = string.Format(CultureInfo.InvariantCulture, "{0}?versionId={1}", source, AWSSDKUtils.UrlEncode(version, true));
                 }
             }
             else
             {
-                source = AmazonS3Util.UrlEncode(bucket, true);
+                source = AWSSDKUtils.UrlEncode(bucket, true);
             }
 
             return source;

sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/S3Transforms.cs

@@ -35,7 +35,7 @@ internal static string ToURLEncodedValue(string value, bool path)
             if (value == null)
                 return string.Empty;
 
-            return AmazonS3Util.UrlEncode(value, path);
+            return AWSSDKUtils.UrlEncode(value, path);
         }
 
         internal static string ToURLEncodedValue(int value, bool path)

sdk/src/Services/S3/Custom/Util/AmazonS3Util.cs

@@ -241,17 +241,6 @@ public static string MimeTypeFromExtension(string ext)
             }
         }
 
-        /// <summary>
-        /// URL encodes a string. If the path property is specified,
-        /// the accepted path characters {/+:} are not encoded.
-        /// </summary>
-        /// <param name="data">The string to encode</param>
-        /// <param name="path">Whether the string is a URL path or not</param>
-        /// <returns></returns>
-        public static string UrlEncode(string data, bool path)
-        {
-            return AWSSDKUtils.UrlEncode(data, path);
-        }
 
         /// <summary>
         /// Converts a non-seekable stream into a System.IO.MemoryStream.

sdk/test/Services/S3/IntegrationTests/CopyObjectTests.cs

@@ -72,18 +72,40 @@ public void TestCleanup()
 
             BaseClean();
         }
-
-        [TestMethod]
+        [DataTestMethod]
+        [DataRow(testKey, testKey)]
+        [DataRow("ObjectWithAllSpecialCharacters/'()!*$+,;=&", "DestinationObjectWithAllSpecialCharacters/'()!*$+,;=&")]
         [TestCategory("S3")]
-        public void TestCopyObject()
+        public void TestCopyObject(string sourceKey, string destinationKey)
         {
+            var putObjectResponse = usEastClient.PutObject(new PutObjectRequest
+            {
+                BucketName = eastBucketName,
+                Key = sourceKey,
+                ContentBody = testContent
+            });
             var response = usEastClient.CopyObject(new CopyObjectRequest
             {
                 SourceBucket = eastBucketName,
-                SourceKey = testKey,
+                SourceKey = sourceKey,
                 DestinationBucket = westBucketName,
-                DestinationKey = testKey
+                DestinationKey = destinationKey
             });
+
+            Assert.AreEqual(HttpStatusCode.OK, response.HttpStatusCode);
+
+            var getObjectResponse = usWestClient.GetObject(new GetObjectRequest
+            {
+                BucketName = westBucketName,
+                Key = destinationKey
+            });
+
+            using (getObjectResponse.ResponseStream)
+            using (var reader = new StreamReader(getObjectResponse.ResponseStream))
+            {
+                var actualText = reader.ReadToEnd();
+                Assert.AreEqual(testContent, actualText);
+            }
         }
 
         [TestMethod]