Make C S3 requests use the same host logic as Perl.

The C code was passing the host (if specified) with the request which could force the server into path-style URLs, which are not supported. Instead, use the Perl logic of always passing bucket.endpoint in the request no matter what host is used for the HTTPS connection. It's an open question whether we should support path-style URLs but since we don't it's useless to tell the server otherwise. Note that Amazon S3 has deprecated path-style URLs and they are no longer supported on newly created buckets.
pgbackrest · Jun 4, 2019 · 15b8e3b · 15b8e3b
1 parent 5f92c36
commit 15b8e3b
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/src/storage/s3/storage.c b/src/storage/s3/storage.c
@@ -768,14 +768,14 @@ storageS3New(
         driver->secretAccessKey = strDup(secretAccessKey);
         driver->securityToken = strDup(securityToken);
         driver->partSize = partSize;
-        driver->host = host == NULL ? strNewFmt("%s.%s", strPtr(bucket), strPtr(endPoint)) : strDup(host);
+        driver->host = strNewFmt("%s.%s", strPtr(bucket), strPtr(endPoint));
         driver->port = port;
 
         // Force the signing key to be generated on the first run
         driver->signingKeyDate = YYYYMMDD_STR;
 
         // Create the http client used to service requests
-        driver->httpClient = httpClientNew(driver->host, driver->port, timeout, verifyPeer, caFile, caPath);
+        driver->httpClient = httpClientNew(host == NULL ? driver->host : host, driver->port, timeout, verifyPeer, caFile, caPath);
         driver->headerRedactList = strLstAdd(strLstNew(), S3_HEADER_AUTHORIZATION_STR);
 
         this = storageNewP(

diff --git a/test/src/module/storage/s3Test.c b/test/src/module/storage/s3Test.c
@@ -9,6 +9,7 @@ Test S3 Storage
 /***********************************************************************************************************************************
 Test server
 ***********************************************************************************************************************************/
+#define S3_TEST_HOST                                                "bucket.s3.amazonaws.com"
 #define DATE_REPLACE                                                "????????"
 #define DATETIME_REPLACE                                            "????????T??????Z"
 #define SHA256_REPLACE                                                                                                             \
@@ -41,7 +42,7 @@ testS3ServerRequest(const char *verb, const char *uri, const char *content)
 
     strCatFmt(
         request,
-        "host:" TLS_TEST_HOST "\r\n"
+        "host:" S3_TEST_HOST "\r\n"
         "x-amz-content-sha256:%s\r\n"
         "x-amz-date:" DATETIME_REPLACE "\r\n"
         "\r\n",
@@ -506,7 +507,9 @@ testRun(void)
         TEST_ASSIGN(storage, storageRepoGet(strNew(STORAGE_TYPE_S3), false), "get S3 repo storage with options");
         TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->bucket), strPtr(bucket), "    check bucket");
         TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->region), strPtr(region), "    check region");
-        TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->host), strPtr(host), "    check host");
+        TEST_RESULT_STR(
+            strPtr(((StorageS3 *)storage->driver)->host), strPtr(strNewFmt("%s.%s", strPtr(bucket), strPtr(endPoint))),
+            "    check host");
         TEST_RESULT_UINT(((StorageS3 *)storage->driver)->port, 443, "    check port");
         TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->accessKey), strPtr(accessKey), "    check access key");
         TEST_RESULT_STR(
@@ -565,7 +568,9 @@ testRun(void)
         TEST_ASSIGN(storage, storageRepoGet(strNew(STORAGE_TYPE_S3), false), "get S3 repo storage with options");
         TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->bucket), strPtr(bucket), "    check bucket");
         TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->region), strPtr(region), "    check region");
-        TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->host), strPtr(host), "    check host");
+        TEST_RESULT_STR(
+            strPtr(((StorageS3 *)storage->driver)->host), strPtr(strNewFmt("%s.%s", strPtr(bucket), strPtr(endPoint))),
+            "    check host");
         TEST_RESULT_UINT(((StorageS3 *)storage->driver)->port, 7777, "    check port");
         TEST_RESULT_STR(strPtr(((StorageS3 *)storage->driver)->accessKey), strPtr(accessKey), "    check access key");
         TEST_RESULT_STR(
@@ -688,7 +693,7 @@ testRun(void)
             "*** Request Headers ***:\n"
             "authorization: <redacted>\n"
             "content-length: 0\n"
-            "host: " TLS_TEST_HOST "\n"
+            "host: " S3_TEST_HOST "\n"
             "x-amz-content-sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855\n"
             "x-amz-date: <redacted>\n"
             "*** Response Headers ***:\n"
@@ -748,7 +753,7 @@ testRun(void)
             "*** Request Headers ***:\n"
             "authorization: <redacted>\n"
             "content-length: 0\n"
-            "host: " TLS_TEST_HOST "\n"
+            "host: " S3_TEST_HOST "\n"
             "x-amz-content-sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855\n"
             "x-amz-date: <redacted>");