SNOW-1997074 handle HTTP 307&308 redirect (#941)

sfc-gh-ext-simba-hx · web-flow · commit 747ced354ed2 · 2025-11-27T15:56:01.000-08:00
diff --git a/ci/test/test.sh b/ci/test/test.sh
@@ -11,6 +11,9 @@ set -o pipefail
 source $SCRIPTS_DIR/_init.sh -t $BUILD_TYPE
 echo "CMAKE: $CMAKE, CTEST: $CTEST"
 source $SCRIPTS_DIR/utils.sh
+if [ "$(uname)" == "Linux" ]; then
+    source $SCRIPTS_DIR/setup_wiremock.sh
+fi
 
 init_git_variables
 set_parameters $CLOUD_PROVIDER
diff --git a/cpp/StatementPutGet.cpp b/cpp/StatementPutGet.cpp
@@ -192,7 +192,7 @@ bool StatementPutGet::http_put(std::string const& url,
   sf_bool success = SF_BOOLEAN_FALSE;
 
   success = http_perform(curl, PUT_REQUEST_TYPE, urlbuf, &reqHeaders, NULL, &putPayload, NULL,
-                         NULL, &respHeaders, get_retry_timeout(sf),
+                         NULL, &respHeaders, get_retry_timeout(sf), get_retry_timeout(sf),
                          SF_BOOLEAN_FALSE, &m_stmt->error, sf->insecure_mode,sf->ocsp_fail_open,
                          sf->crl_check, sf->crl_advisory, sf->crl_allow_no_crl,
                          sf->crl_disk_caching, sf->crl_memory_caching,
@@ -255,7 +255,7 @@ bool StatementPutGet::http_get(std::string const& url,
   sf_bool success = SF_BOOLEAN_FALSE;
 
   success = http_perform(curl, reqType, urlbuf, &reqHeaders, NULL, NULL, NULL,
-                         &resp, &respHeaders, get_retry_timeout(sf),
+                         &resp, &respHeaders, get_retry_timeout(sf), get_retry_timeout(sf),
                          SF_BOOLEAN_FALSE, &m_stmt->error, sf->insecure_mode, sf->ocsp_fail_open,
                          sf->crl_check, sf->crl_advisory, sf->crl_allow_no_crl,
                          sf->crl_disk_caching, sf->crl_memory_caching,
diff --git a/cpp/lib/Authenticator.cpp b/cpp/lib/Authenticator.cpp
@@ -568,7 +568,7 @@ namespace Client
           if (parseJSON) 
           {
               isHttpSuccess = http_perform(curl, GET_REQUEST_TYPE, (char*)destination.c_str(), httpExtraHeaders, NULL, NULL, &resp_data,
-                                           raw_resp, NULL, curlTimeout, SF_BOOLEAN_FALSE, err,
+                                           raw_resp, NULL, m_retryTimeout, curlTimeout, SF_BOOLEAN_FALSE, err,
                                            m_connection->insecure_mode, m_connection->ocsp_fail_open,
                                            m_connection->crl_check, m_connection->crl_advisory, m_connection->crl_allow_no_crl,
                                            m_connection->crl_disk_caching, m_connection->crl_memory_caching,
@@ -579,7 +579,7 @@ namespace Client
           else
           {
               isHttpSuccess = http_perform(curl, GET_REQUEST_TYPE, (char*)destination.c_str(), httpExtraHeaders, NULL, NULL, NULL,
-                                           raw_resp, NULL, curlTimeout, SF_BOOLEAN_FALSE, err,
+                                           raw_resp, NULL, m_retryTimeout, curlTimeout, SF_BOOLEAN_FALSE, err,
                                            m_connection->insecure_mode, m_connection->ocsp_fail_open,
                                            m_connection->crl_check, m_connection->crl_advisory, m_connection->crl_allow_no_crl,
                                            m_connection->crl_disk_caching, m_connection->crl_memory_caching,
diff --git a/cpp/util/CurlDesc.cpp b/cpp/util/CurlDesc.cpp
@@ -64,6 +64,9 @@ namespace Client
       curl_easy_reset(m_curl);
       CXX_LOG_TRACE("CurDesc::reset(): curl_easy_reset %p", m_curl);
     }
+
+    // allow redirect
+    curl_easy_setopt(m_curl, CURLOPT_FOLLOWLOCATION, true);
   }
 
 }
diff --git a/include/snowflake/client.h b/include/snowflake/client.h
@@ -82,6 +82,11 @@ extern "C" {
  */
 #define SF_RETRY_TIMEOUT 300
 
+ /**
+ * network timeout in seconds
+ */
+#define SF_NETWORK_TIMEOUT 90
+
     /**
      * CRL download timeout in seconds
      */
diff --git a/lib/chunk_downloader.c b/lib/chunk_downloader.c
@@ -212,14 +212,14 @@ sf_bool STDCALL download_chunk(char *url, SF_HEADER *headers,
                                long crl_download_timeout,
                                const char *proxy,
                                const char *no_proxy,
-                               int64 network_timeout, int8 retry_max_count) {
+                               int64 retry_timeout, int8 retry_max_count) {
     sf_bool ret = SF_BOOLEAN_FALSE;
     void* curl_desc = get_curl_desc_from_pool(url, proxy, no_proxy);
     CURL *curl = get_curl_from_desc(curl_desc);
 
     if (!curl ||
         !http_perform(curl, GET_REQUEST_TYPE, url, headers, NULL, NULL, chunk,
-                      non_json_resp, NULL, network_timeout,
+                      non_json_resp, NULL, retry_timeout, retry_timeout,
                       SF_BOOLEAN_TRUE, error, insecure_mode, fail_open,
                       crl_check, crl_advisory, crl_allow_no_crl,
                       crl_disk_caching, crl_memory_caching,
@@ -255,7 +255,7 @@ SF_CHUNK_DOWNLOADER *STDCALL chunk_downloader_init(const char *qrmk,
                                                    NON_JSON_RESP* (*callback_create_resp)(void),
                                                    const char *proxy,
                                                    const char *no_proxy,
-                                                   int64 network_timeout,
+                                                   int64 retry_timeout,
                                                    int8 retry_max_count) {
     struct SF_CHUNK_DOWNLOADER *chunk_downloader = NULL;
     const char *error_msg = NULL;
@@ -299,7 +299,7 @@ SF_CHUNK_DOWNLOADER *STDCALL chunk_downloader_init(const char *qrmk,
     chunk_downloader->callback_create_resp = callback_create_resp;
     chunk_downloader->proxy = NULL;
     chunk_downloader->no_proxy = NULL;
-    chunk_downloader->network_timeout = network_timeout;
+    chunk_downloader->retry_timeout = retry_timeout;
     chunk_downloader->retry_max_count = retry_max_count;
 
     // Initialize chunk_headers or qrmk
@@ -507,7 +507,7 @@ static void * chunk_downloader_thread(void *downloader) {
                             chunk_downloader->crl_check, chunk_downloader->crl_advisory, chunk_downloader->crl_allow_no_crl,
                             chunk_downloader->crl_disk_caching, chunk_downloader-> crl_memory_caching,
                             chunk_downloader->crl_download_timeout, chunk_downloader->proxy,
-                            chunk_downloader->no_proxy, chunk_downloader->network_timeout, chunk_downloader->retry_max_count)) {
+                            chunk_downloader->no_proxy, chunk_downloader->retry_timeout, chunk_downloader->retry_max_count)) {
             _rwlock_wrlock(&chunk_downloader->attr_lock);
             if (!chunk_downloader->has_error) {
                 copy_snowflake_error(chunk_downloader->sf_error, &err);
diff --git a/lib/chunk_downloader.h b/lib/chunk_downloader.h
@@ -79,7 +79,7 @@ struct SF_CHUNK_DOWNLOADER {
     char *no_proxy;
 
     // retry settings
-    int64 network_timeout;
+    int64 retry_timeout;
     int8 retry_max_count;
 };
 
@@ -100,7 +100,7 @@ SF_CHUNK_DOWNLOADER *STDCALL chunk_downloader_init(const char *qrmk,
                                                    NON_JSON_RESP* (*callback_create_resp)(void),
                                                    const char *proxy,
                                                    const char *no_proxy,
-                                                   int64 network_timeout,
+                                                   int64 retry_timeout,
                                                    int8 retry_max_count);
 sf_bool STDCALL chunk_downloader_term(SF_CHUNK_DOWNLOADER *chunk_downloader);
 sf_bool STDCALL get_shutdown_or_error(SF_CHUNK_DOWNLOADER *chunk_downloader);
diff --git a/lib/client.c b/lib/client.c
@@ -1046,7 +1046,7 @@ SF_CONNECT *STDCALL snowflake_init() {
         sf->token = NULL;
         sf->master_token = NULL;
         sf->login_timeout = SF_LOGIN_TIMEOUT;
-        sf->network_timeout = 0;
+        sf->network_timeout = SF_NETWORK_TIMEOUT;
         sf->browser_response_timeout = SF_BROWSER_RESPONSE_TIMEOUT;
         sf->retry_timeout = SF_RETRY_TIMEOUT;
         sf->sequence_counter = 0;
@@ -1522,7 +1522,7 @@ SF_STATUS STDCALL snowflake_set_attribute(
             sf->login_timeout = value ? *((int64 *) value) : SF_LOGIN_TIMEOUT;
             break;
         case SF_CON_NETWORK_TIMEOUT:
-            sf->network_timeout = value ? *((int64 *) value) : SF_LOGIN_TIMEOUT;
+            sf->network_timeout = value ? *((int64 *) value) : SF_NETWORK_TIMEOUT;
             break;
         case SF_CON_BROWSER_RESPONSE_TIMEOUT:
             sf->browser_response_timeout = value ? *((int64*)value) : SF_BROWSER_RESPONSE_TIMEOUT;
diff --git a/lib/connection.c b/lib/connection.c
@@ -287,7 +287,7 @@ sf_bool STDCALL curl_post_call(SF_CONNECT *sf,
 
     do {
         if (!http_perform(curl, POST_REQUEST_TYPE, url, header, body, NULL, json, NULL, NULL,
-                          retry_timeout, SF_BOOLEAN_FALSE, error,
+                          retry_timeout, sf->network_timeout, SF_BOOLEAN_FALSE, error,
                           sf->insecure_mode, sf->ocsp_fail_open,
                           sf->crl_check, sf->crl_advisory, sf->crl_allow_no_crl,
                           sf->crl_disk_caching, sf->crl_memory_caching,
@@ -432,7 +432,7 @@ sf_bool STDCALL curl_get_call(SF_CONNECT *sf,
 
     do {
         if (!http_perform(curl, GET_REQUEST_TYPE, url, header, NULL, NULL, json, NULL, NULL,
-                          get_retry_timeout(sf), SF_BOOLEAN_FALSE, error,
+                          get_retry_timeout(sf), sf->network_timeout, SF_BOOLEAN_FALSE, error,
                           sf->insecure_mode, sf->ocsp_fail_open,
                           sf->crl_check, sf->crl_advisory, sf->crl_allow_no_crl,
                           sf->crl_disk_caching, sf->crl_memory_caching,
@@ -1295,7 +1295,7 @@ int64 get_login_timeout(SF_CONNECT *sf)
 
 int64 get_retry_timeout(SF_CONNECT *sf)
 {
-  return get_less_one(sf->network_timeout, sf->retry_timeout);
+  return sf->retry_timeout;
 }
 
 int8 get_login_retry_count(SF_CONNECT *sf)
diff --git a/lib/connection.h b/lib/connection.h
@@ -451,6 +451,7 @@ size_t char_resp_cb(char *data, size_t size, size_t nmemb, RAW_CHAR_BUFFER *raw_
  *                      Used only when json is set to NULL.
  * @param resp_headers A reference to retrieve response headers. Needs to be freed with SF_FREE.
  *                     Set to NULL if it's not needed.
+ * @param retry_timeout The retry timeout to use for all retry attempts of one request.
  * @param network_timeout The network request timeout to use for each request try.
  * @param chunk_downloader A boolean value determining whether or not we are running this request from the chunk
  *                         downloader. Each chunk that we download from AWS is invalid JSON so we need to add an
@@ -482,7 +483,7 @@ size_t char_resp_cb(char *data, size_t size, size_t nmemb, RAW_CHAR_BUFFER *raw_
  */
 sf_bool STDCALL http_perform(CURL *curl, SF_REQUEST_TYPE request_type, char *url, SF_HEADER *header,
                              char *body, PUT_PAYLOAD* put_payload, cJSON **json, NON_JSON_RESP* non_json_resp,
-                             char** resp_headers, int64 network_timeout, sf_bool chunk_downloader,
+                             char** resp_headers, int64 retry_timeout, int64 network_timeout, sf_bool chunk_downloader,
                              SF_ERROR_STRUCT* error, sf_bool insecure_mode, sf_bool fail_open,
                              sf_bool crl_check, sf_bool crl_advisory, sf_bool crl_allow_no_crl,
                              sf_bool crl_disk_caching, sf_bool crl_memory_caching,
diff --git a/lib/http_perform.c b/lib/http_perform.c
@@ -139,6 +139,7 @@ sf_bool STDCALL http_perform(CURL *curl,
                              cJSON **json,
                              NON_JSON_RESP *non_json_resp,
                              char** resp_headers,
+                             int64 retry_timeout,
                              int64 network_timeout,
                              sf_bool chunk_downloader,
                              SF_ERROR_STRUCT *error,
@@ -174,17 +175,30 @@ sf_bool STDCALL http_perform(CURL *curl,
       djb.cap = SF_NEW_STRATEGY_BACKOFF_CAP;
     }
 
-    network_timeout = (network_timeout > 0) ? network_timeout : SF_RETRY_TIMEOUT;
+    retry_timeout = (retry_timeout > 0) ? retry_timeout : SF_RETRY_TIMEOUT;
+    if (elapsed_time) {
+        retry_timeout -= *elapsed_time;
+        if (retry_timeout <= 0) {
+            retry_timeout = 1;
+        }
+    }
+
+    network_timeout = (network_timeout > 0) ? network_timeout : SF_NETWORK_TIMEOUT;
     if (elapsed_time) {
         network_timeout -= *elapsed_time;
         if (network_timeout <= 0) {
             network_timeout = 1;
         }
     }
+    if (retry_timeout < network_timeout)
+    {
+        network_timeout = retry_timeout;
+    }
+
     RETRY_CONTEXT curl_retry_ctx = {
             retried_count ? *retried_count : 0,      //retry_count
             0,      // retry reason
-            network_timeout,
+            retry_timeout,
             djb.base,      // time to sleep
             &djb,    // Decorrelate jitter
             sf_get_current_time_millis() // start time
@@ -490,6 +504,7 @@ sf_bool STDCALL http_perform(CURL *curl,
 
         /* Check for errors */
         if (res != CURLE_OK) {
+          char msg[1024];
           if (curl_error_buffer[0] != '\0') {
             log_error("curl error buffer: %s", curl_error_buffer);
           }
@@ -504,9 +519,35 @@ sf_bool STDCALL http_perform(CURL *curl,
                       curl_retry_ctx.retry_count,
                       next_sleep_in_secs);
               sf_sleep_ms(next_sleep_in_secs*1000);
-            } else if ((res == CURLE_OPERATION_TIMEDOUT) && (renew_timeout > 0)) {
-               retry = SF_BOOLEAN_TRUE;
-            } else {
+            } else if (res == CURLE_OPERATION_TIMEDOUT) {
+              // retry directly without backoff when timeout is triggered by renew
+              if ((renew_timeout > 0) && (curl_timeout == renew_timeout))
+              {
+                retry = SF_BOOLEAN_TRUE;
+              }
+              // otherwise retry with backoff
+              else if (((uint64)(time(NULL) - elapsedRetryTime) < curl_retry_ctx.retry_timeout) &&
+                       ((retry_max_count <= 0) || (curl_retry_ctx.retry_count < (unsigned)retry_max_count)))
+              {
+                uint32 next_sleep_in_secs = retry_ctx_next_sleep(&curl_retry_ctx);
+                log_debug(
+                    "retry on network timeout, retry count  %d "
+                    "will retry after %d seconds", res,
+                    curl_retry_ctx.retry_count,
+                    next_sleep_in_secs);
+                sf_sleep_ms(next_sleep_in_secs * 1000);
+                retry = SF_BOOLEAN_TRUE;
+              }
+              else {
+                sf_sprintf(msg, sizeof(msg),
+                          "Exceeded the retry_timeout , curl code: [%d]",
+                          res);
+                SET_SNOWFLAKE_ERROR(error, SF_STATUS_ERROR_RETRY,
+                                    msg,
+                                    SF_SQLSTATE_UNABLE_TO_CONNECT);
+              }
+            }
+            else {
               char msg[1024];
               if (res == CURLE_SSL_CACERT_BADFILE) {
                 sf_sprintf(msg, sizeof(msg), "curl_easy_perform() failed. err: %s, CA Cert file: %s",
@@ -647,6 +688,7 @@ sf_bool STDCALL __wrap_http_perform(CURL *curl,
                                     cJSON **json,
                                     NON_JSON_RESP *non_json_resp,
                                     char** resp_headers,
+                                    int64 retry_timeout,
                                     int64 network_timeout,
                                     sf_bool chunk_downloader,
                                     SF_ERROR_STRUCT *error,
diff --git a/lib/mock_http_perform.h b/lib/mock_http_perform.h
@@ -13,7 +13,7 @@ extern "C" {
 // The parameters for this are identical to http_perform located in connection.h
 // This is just the mock interface
 sf_bool STDCALL __wrap_http_perform(CURL *curl, SF_REQUEST_TYPE request_type, char *url, SF_HEADER *header,
-                                    char *body, PUT_PAYLOAD* put_payload, cJSON **json, NON_JSON_RESP *non_json_resp, char** resp_headers, int64 network_timeout, sf_bool chunk_downloader,
+                                    char *body, PUT_PAYLOAD* put_payload, cJSON **json, NON_JSON_RESP *non_json_resp, char** resp_headers, int64 retry_timeout, int64 network_timeout, sf_bool chunk_downloader,
                                     SF_ERROR_STRUCT *error, sf_bool insecure_mode, sf_bool fail_open);
 
 #endif
diff --git a/scripts/setup_wiremock.sh b/scripts/setup_wiremock.sh
@@ -0,0 +1,41 @@
+#!/usr/bin/env bash
+set -o pipefail
+
+WIREMOCK_VERSION="3.8.0"
+WIREMOCK_JAR="${HOME}/.m2/repository/org/wiremock/wiremock-standalone/${WIREMOCK_VERSION}/wiremock-standalone-${WIREMOCK_VERSION}.jar"
+WIREMOCK_URL="https://repo1.maven.org/maven2/org/wiremock/wiremock-standalone/${WIREMOCK_VERSION}/wiremock-standalone-${WIREMOCK_VERSION}.jar"
+
+mkdir -p "$(dirname "$WIREMOCK_JAR")"
+
+echo "WireMock JAR is downlaoding${WIREMOCK_URL}"
+curl -L -o "${WIREMOCK_JAR}" "${WIREMOCK_URL}"
+CURL_STATUS=$? 
+
+if [ "$CURL_STATUS" -ne 0 ]; then
+    echo "ERROR: Failed to download WireMock JAR. (curl code: $CURL_STATUS)"
+    exit 1
+fi
+
+chmod +r "$WIREMOCK_JAR"
+
+echo "SUCCESS: WireMock JAR (${WIREMOCK_VERSION}) downloaded successfully."
+echo "   - location: $WIREMOCK_JAR"
+echo "   - size: ${FILE_SIZE} bytes"
+
+JAVA_URL="https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.12%2B7/OpenJDK17U-jdk_x64_linux_hotspot_17.0.12_7.tar.gz"
+INSTALL_DIR="${HOME}/.java17"
+DOWNLOAD_DIR="${HOME}/java17.tar.gz"
+
+mkdir -p $INSTALL_DIR
+
+curl -L --fail $JAVA_URL -o "${DOWNLOAD_DIR}"
+
+tar -xzf "${DOWNLOAD_DIR}" -C $INSTALL_DIR --strip-components=1
+
+rm "${DOWNLOAD_DIR}"
+
+export JAVA_HOME=$INSTALL_DIR
+export PATH=$JAVA_HOME/bin:$PATH
+
+echo "JAVA_HOME : $JAVA_HOME"
+echo "PATH:$PATH"
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -396,6 +396,10 @@ else()
         ENDFOREACH ()
     endif ()
 
+    if (LINUX)
+        add_subdirectory(wiremock)
+        add_subdirectory(test_redirect)
+    endif()
 
 endif()
 
diff --git a/tests/test_connect_negative.c b/tests/test_connect_negative.c
@@ -123,10 +123,10 @@ void test_connect_login_timeout(void** unused) {
   unsigned long end_time = (unsigned long)time(NULL);
   assert_int_not_equal(ret, SF_STATUS_SUCCESS); // must fail
   SF_ERROR_STRUCT* sferr = snowflake_error(sf);
-  if (sferr->error_code != SF_STATUS_ERROR_CURL) {
+  if (sferr->error_code != SF_STATUS_ERROR_RETRY) {
     dump_error(sferr);
   }
-  assert_int_equal(sferr->error_code, SF_STATUS_ERROR_CURL);
+  assert_int_equal(sferr->error_code, SF_STATUS_ERROR_RETRY);
   assert_in_range(end_time - start_time, timeout - delta, timeout + delta);
   snowflake_term(sf);
 }
@@ -157,7 +157,9 @@ void test_chunk_downloading_timeout(void** unused) {
   /* query */
   sfstmt = snowflake_stmt(sf);
 
-  snowflake_set_attribute(sf, SF_CON_NETWORK_TIMEOUT, &timeout);
+  // can't set retry timeout lower than 300 through snowflake_set_attribute
+  sf->retry_timeout = timeout;
+
   // sometime this could get timeout
   for (int i = 0; i < 3; i++)
   {
diff --git a/tests/test_redirect/CMakeLists.txt b/tests/test_redirect/CMakeLists.txt
@@ -0,0 +1,21 @@
+project(test_redirect)
+
+configure_file(http_307_retry.json . COPYONLY)
+configure_file(http_308_retry.json . COPYONLY)
+
+add_executable(test_redirect
+    ../wiremock/wiremock.cpp
+    ../wiremock/wiremock.hpp
+    ../utils/test_setup.h
+    ../utils/TestSetup.hpp
+    ../utils/test_setup.c
+    ../utils/TestSetup.cpp
+    test_redirect.cpp
+)
+
+add_compile_options(-Wall -Wno-error=ignored-attributes)
+
+target_link_libraries(test_redirect ${TESTLIB_OPTS_C})
+
+add_test(test_redirect test_redirect)
+
diff --git a/tests/test_redirect/http_307_retry.json b/tests/test_redirect/http_307_retry.json
diff --git a/tests/test_redirect/http_308_retry.json b/tests/test_redirect/http_308_retry.json
diff --git a/tests/test_redirect/test_redirect.cpp b/tests/test_redirect/test_redirect.cpp
diff --git a/tests/wiremock/CMakeLists.txt b/tests/wiremock/CMakeLists.txt
diff --git a/tests/wiremock/ca-cert.jks b/tests/wiremock/ca-cert.jks
diff --git a/tests/wiremock/wiremock.cpp b/tests/wiremock/wiremock.cpp
diff --git a/tests/wiremock/wiremock.hpp b/tests/wiremock/wiremock.hpp

Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,9 @@ namespace Client`
`64`	`64`	`curl_easy_reset(m_curl);`
`65`	`65`	`CXX_LOG_TRACE("CurDesc::reset(): curl_easy_reset %p", m_curl);`
`66`	`66`	`}`
	`67`	`+`
	`68`	`+ // allow redirect`
	`69`	`+ curl_easy_setopt(m_curl, CURLOPT_FOLLOWLOCATION, true);`
`67`	`70`	`}`
`68`	`71`
`69`	`72`	`}`