Fixes for S3 uploading bugs. (#1984)

* Fixes two bugs in uploading.

  First bug prevented the finalisation of the upload in AWS.
  This was caused by not removing two header entries which caused
  the signature to be miscalculated.

  Second was caused by failure to re-check the response code after a
  redirection.

* Adds checks for 307 Temporary Redirect	responses from AWS.

  These	can occur when a bucket has just been created, and its
  name hasn't been added completely to DNS.  They are handled
  in the same way as 301 Moved Permanently.

Author: whitwham

hfile_s3.c

@@ -538,6 +538,8 @@ static int redirect_endpoint(hFILE_s3 *fp, kstring_t *header) {
         }
     }
 
+    if (hts_verbose >= HTS_LOG_INFO) fprintf(stderr, "hfile_s3: redirect_endpoint: return %d\n", ret);
+
     return ret;
 }
 
@@ -1120,9 +1122,9 @@ static int set_region(s3_auth_data *ad, kstring_t *region) {
 // Some common code
 
 #define S3_MOVED_PERMANENTLY 301
+#define S3_TEMPORARY_REDIRECT 307
 #define S3_BAD_REQUEST 400
 
-
 static struct {
     kstring_t useragent;
     CURLSH *share;
@@ -1178,7 +1180,7 @@ static char *stristr(char *haystack, char *needle) {
         char *h = haystack;
         char *n = needle;
 
-        while (toupper(*h) == toupper(*n)) {
+        while (toupper_c(*h) == toupper_c(*n)) {
             h++, n++;
             if (!*h || !*n) break;
         }
@@ -1319,6 +1321,15 @@ static struct curl_slist *set_html_headers(hFILE_s3 *fp, kstring_t *auth, kstrin
     struct curl_slist *headers = NULL;
     int err = 0;
 
+    /* The next two lines have the effect of preventing curl from
+       adding these headers.  If they exist it can lead to conflicts
+       in the signature calculations (not present in all S3 systems).
+    */
+    err = add_header(&headers, "Content-Type:");
+    err |= add_header(&headers, "Expect:");
+
+    if (err) goto error;
+
     if (auth->l)
         if ((err = add_header(&headers, auth->s)))
             goto error;
@@ -1407,11 +1418,13 @@ uploads and abandon the upload process.
 static int abort_upload(hFILE_s3 *fp) {
     kstring_t url = KS_INITIALIZE;
     kstring_t canonical_query_string = KS_INITIALIZE;
-    int ret = -1;
+    int ret = -1, save_errno;
     struct curl_slist *headers = NULL;
     char http_request[] = "DELETE";
     CURLcode err;
 
+    save_errno = errno; // keep the errno that caused the need to abort
+
     clear_authorisation_values(fp);
 
     if (ksprintf(&canonical_query_string, "uploadId=%s", fp->upload_id.s) < 0) {
@@ -1459,6 +1472,7 @@ static int abort_upload(hFILE_s3 *fp) {
     fp->aborted = 1;
     cleanup(fp);
 
+    errno = save_errno;
     return ret;
 }
 
@@ -1497,6 +1511,7 @@ static int complete_upload(hFILE_s3 *fp, kstring_t *resp) {
     curl_easy_reset(fp->curl);
 
     err = curl_easy_setopt(fp->curl, CURLOPT_POST, 1L);
+
     err |= curl_easy_setopt(fp->curl, CURLOPT_POSTFIELDS, fp->completion_message.s);
     err |= curl_easy_setopt(fp->curl, CURLOPT_POSTFIELDSIZE, (long) fp->completion_message.l);
     err |= curl_easy_setopt(fp->curl, CURLOPT_WRITEFUNCTION, response_callback);
@@ -1670,6 +1685,7 @@ static int s3_write_close(hFILE *fpv) {
     kstring_t response = {0, 0, NULL};
     int ret = 0;
     CURLcode cret;
+    long response_code;
 
     if (!fp->aborted) {
 
@@ -1679,7 +1695,6 @@ static int s3_write_close(hFILE *fpv) {
             ret = upload_part(fp, &response);
 
             if (!ret) {
-                long response_code;
                 kstring_t etag = {0, 0, NULL};
 
                 cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
@@ -1715,6 +1730,17 @@ static int s3_write_close(hFILE *fpv) {
             if (!ret) {
                 if (strstr(response.s, "CompleteMultipartUploadResult") == NULL) {
                     ret = -1;
+                    cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
+
+                    if (cret == CURLE_OK) {
+                        if (hts_verbose >= HTS_LOG_INFO) {
+                            if (report_s3_error(&response, response_code)) {
+                                fprintf(stderr, "hfile_s3: warning, unable to report full S3 error status.\n");
+                            }
+                        }
+
+                        errno = http_status_errno(response_code);
+                    }
                 }
             }
         } else {
@@ -1746,6 +1772,8 @@ static int handle_bad_request(hFILE_s3 *fp, kstring_t *resp) {
 
     ks_free(&region);
 
+    if (hts_verbose >= HTS_LOG_INFO) fprintf(stderr, "hfile_s3: handle_bad_request: return %d\n", ret);
+
     return ret;
 }
 
@@ -1852,14 +1880,13 @@ static int get_part(hFILE_s3 *fp, kstring_t *resp) {
     struct curl_slist *headers = NULL;
     int ret = -1;
     char http_request[] = "GET";
-    char canonical_query_string = 0;
     CURLcode err;
 
     ks_clear(&fp->buffer); // reset storage buffer
     clear_authorisation_values(fp);
 
     if (fp->au->is_v4) {
-        if (v4_authorisation(fp, http_request, NULL, &canonical_query_string, 0) != 0) {
+        if (v4_authorisation(fp, http_request, NULL, "", 0) != 0) {
             goto out;
         }
 
@@ -2077,7 +2104,7 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
     hFILE_s3 *fp;
     kstring_t response = {0, 0, NULL};
     kstring_t header   = {0, 0, NULL};
-    int ret, has_user_query = 0;
+    int has_user_query = 0;
     char *query_start;
     const char *env;
     CURLcode cret;
@@ -2125,30 +2152,35 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
         has_user_query = 1;;
     }
 
-    ret = initialise_upload(fp, &header, &response, has_user_query);
+    if (initialise_upload(fp, &header, &response, has_user_query))
+        goto error;
+
     cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
 
-    if (ret == 0) {
-        if (cret == CURLE_OK) {
-            if (response_code == S3_MOVED_PERMANENTLY) {
-                if (redirect_endpoint(fp, &header) == 0) {
-                    ks_clear(&response);
-                    ks_clear(&header);
+    if (cret == CURLE_OK) {
+        if (response_code == S3_MOVED_PERMANENTLY || response_code == S3_TEMPORARY_REDIRECT) {
+            if (redirect_endpoint(fp, &header) == 0) {
+                ks_clear(&response);
+                ks_clear(&header);
 
-                    ret = initialise_upload(fp, &header, &response, has_user_query);
-                }
-            } else if (response_code == S3_BAD_REQUEST) {
-                if (handle_bad_request(fp, &response) == 0) {
-                    ks_clear(&response);
-                    ks_clear(&header);
+                if (initialise_upload(fp, &header, &response, has_user_query))
+                    goto error;
+            }
+        } else if (response_code == S3_BAD_REQUEST) {
+            if (handle_bad_request(fp, &response) == 0) {
+                ks_clear(&response);
+                ks_clear(&header);
 
-                    ret = initialise_upload(fp, &header, &response, has_user_query);
-                }
+                if (initialise_upload(fp, &header, &response, has_user_query))
+                    goto error;
             }
-        } else {
-            // unable to get a response code from curl
-            ret = -1;
         }
+
+        // reget the response code (may not have changed)
+        cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
+    } else {
+        // unable to get a response code from curl
+        goto error;
     }
 
     if (response_code >= 300) {
@@ -2164,11 +2196,9 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
             errno = http_status_errno(response_code);
         }
 
-        ret = -1;
+        goto error;
     }
 
-    if (ret) goto error;
-
     if (get_upload_id(fp, &response)) goto error;
 
     // start the completion message (a formatted list of parts)
@@ -2203,7 +2233,6 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
     const char *env;
     kstring_t response   = {0, 0, NULL};
     kstring_t file_range = {0, 0, NULL};
-    int ret;
     CURLcode cret;
     long response_code = 0;
 
@@ -2240,31 +2269,33 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
 
     kputs(url, &fp->url);
 
-    ret = initialise_download(fp, &response);
-    cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
+    if (initialise_download(fp, &response))
+        goto error;
 
-    if (ret == 0) {
-        if (cret == CURLE_OK) {
-            if (response_code == S3_MOVED_PERMANENTLY) {
-                ks_clear(&response);
+    cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
 
-                if (redirect_endpoint(fp, &response) == 0) {
-                    ret = initialise_download(fp, &response);
-                }
-            } else if (response_code == S3_BAD_REQUEST) {
-                ks_clear(&response);
+    if (cret == CURLE_OK) {
+        if (response_code == S3_MOVED_PERMANENTLY || response_code == S3_TEMPORARY_REDIRECT) {
+            ks_clear(&response);
 
-                if (handle_bad_request(fp, &fp->buffer) == 0) {
-                    ret = initialise_download(fp, &response);
-                }
+            if (redirect_endpoint(fp, &response) == 0) {
+                if (initialise_download(fp, &response))
+                    goto error;
             }
+        } else if (response_code == S3_BAD_REQUEST) {
+            ks_clear(&response);
 
-            // reget the response code (may not have changed)
-            cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
-        } else {
-            // unable to get a response code from curl
-            ret = -1;
+            if (handle_bad_request(fp, &fp->buffer) == 0) {
+                if (initialise_download(fp, &response))
+                    goto error;
+            }
         }
+
+        // reget the response code (may not have changed)
+        cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
+    } else {
+        // unable to get a response code from curl
+        goto error;
     }
 
     if (response_code >= 300) {
@@ -2280,11 +2311,9 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
             errno = http_status_errno(response_code);
         }
 
-        ret = -1;
+        goto error;
     }
 
-    if (ret) goto error;
-
     if (get_entry(response.s, "content-range: bytes ", "\n", &file_range) == EOF) {
         fprintf(stderr, "hfile_s3: warning: failed to read file size.\n");
         fp->file_size = -1;
@@ -2373,7 +2402,7 @@ static hFILE *hopen_s3(const char *url, const char *mode)
 {
     hFILE *fp;
 
-    if (getenv("HTS_S3_V2") == NULL) { // Force the v2 signature code
+    if (getenv("HTS_S3_V2") == NULL) {
         fp = s3_open_v4(url, mode, NULL);
     } else {
         fp = s3_open_v2(url, mode, NULL);