From 7353f0ffd9bfb323dd53d5d3e5480713feeeaf7f Mon Sep 17 00:00:00 2001 From: Jason Zou Date: Tue, 31 Oct 2023 11:21:06 -0700 Subject: [PATCH] replace ms-banned functions part2 --- cpp/FileTransferAgent.cpp | 8 +++-- cpp/SnowflakeAzureClient.cpp | 4 ++- cpp/util/Proxy.cpp | 38 +++++++++++++---------- include/snowflake/Simba_CRTFunctionSafe.h | 13 ++++++-- include/snowflake/platform.h | 8 +++++ lib/client.c | 9 ++++-- lib/logger.c | 4 ++- lib/platform.c | 9 ++++-- 8 files changed, 65 insertions(+), 28 deletions(-) diff --git a/cpp/FileTransferAgent.cpp b/cpp/FileTransferAgent.cpp index b93368ccf5..00f7e316a6 100755 --- a/cpp/FileTransferAgent.cpp +++ b/cpp/FileTransferAgent.cpp @@ -847,16 +847,20 @@ RemoteStorageRequestOutcome Snowflake::Client::FileTransferAgent::downloadSingle } catch (...) { std::string err = "Could not open file " + fileMetadata->destPath + " to downoad"; + char* str_error = sf_strerror(errno); CXX_LOG_DEBUG("Could not open file %s to downoad: %s", - fileMetadata->destPath.c_str(), sf_strerror(errno)); + fileMetadata->destPath.c_str(), str_error); + sf_free_s(str_error); m_executionResults->SetTransferOutCome(outcome, resultIndex); break; } if (!dstFile.is_open()) { std::string err = "Could not open file " + fileMetadata->destPath + " to downoad"; + char* str_error = sf_strerror(errno); CXX_LOG_DEBUG("Could not open file %s to downoad: %s", - fileMetadata->destPath.c_str(), sf_strerror(errno)); + fileMetadata->destPath.c_str(), str_error); + sf_free_s(str_error); m_executionResults->SetTransferOutCome(outcome, resultIndex); break; } diff --git a/cpp/SnowflakeAzureClient.cpp b/cpp/SnowflakeAzureClient.cpp index f8f3e3fa13..2db03df4f6 100755 --- a/cpp/SnowflakeAzureClient.cpp +++ b/cpp/SnowflakeAzureClient.cpp @@ -61,15 +61,17 @@ SnowflakeAzureClient::SnowflakeAzureClient(StageInfo *stageInfo, CXX_LOG_TRACE("ca bundle file from SF_GLOBAL_CA_BUNDLE_FILE *%s*", caBundleFile); } if( caBundleFile[0] == 0 ) { - const char* capath = sf_getenv("SNOWFLAKE_TEST_CA_BUNDLE_FILE"); + char* capath = sf_getenv("SNOWFLAKE_TEST_CA_BUNDLE_FILE"); if (capath) { if (strlen(capath) > MAX_PATH - 1) { + sf_free_s(capath); throw SnowflakeTransferException(TransferError::INTERNAL_ERROR, "CA bundle file path too long."); } if (!sb_strcpy(caBundleFile, (size_t)MAX_PATH, capath)) { caBundleFile[0] = 0; } + sf_free_s(capath); CXX_LOG_TRACE("ca bundle file from SNOWFLAKE_TEST_CA_BUNDLE_FILE *%s*", caBundleFile); } } diff --git a/cpp/util/Proxy.cpp b/cpp/util/Proxy.cpp index e3a648f6f1..cbf629b1de 100644 --- a/cpp/util/Proxy.cpp +++ b/cpp/util/Proxy.cpp @@ -4,6 +4,19 @@ #include "snowflake/Proxy.hpp" #include "snowflake/Simba_CRTFunctionSafe.h" +#include "snowflake/platform.h" + +namespace { + char* get_env_or(const char* name) { + return sf_getenv(name); + } + template + char* get_env_or(const char* prime, Args... fallback) + { + char* value = get_env_or(prime); + return value ? value : get_env_or(fallback...); + } +} Snowflake::Client::Util::Proxy::Proxy(const std::string &proxy_str) { @@ -69,27 +82,18 @@ void Snowflake::Client::Util::Proxy::clearPwd() { } void Snowflake::Client::Util::Proxy::setProxyFromEnv() { - std::string proxy; - - // Get proxy string and set scheme - if (sf_getenv("all_proxy")) { - proxy = sf_getenv("all_proxy"); - } else if (sf_getenv("https_proxy")) { - proxy = sf_getenv("https_proxy"); - } else if (sf_getenv("http_proxy")) { - proxy = sf_getenv("http_proxy"); - } - - if (!proxy.empty()) - { + char* env_value = get_env_or("all_proxy", "https_proxy", "http_proxy"); + if (env_value != nullptr) { + std::string proxy(env_value); + sf_free_s(env_value); stringToProxyParts(proxy); } // Get noproxy string - if (sf_getenv("no_proxy")) { - m_noProxy = sf_getenv("no_proxy"); - } else if (sf_getenv("NO_PROXY")) { - m_noProxy = sf_getenv("NO_PROXY"); + env_value = get_env_or("no_proxy", "NO_PROXY"); + if (env_value != nullptr) { + m_noProxy = env_value; + sf_free_s(env_value); } } diff --git a/include/snowflake/Simba_CRTFunctionSafe.h b/include/snowflake/Simba_CRTFunctionSafe.h index dcc8c9b459..d529b338b0 100644 --- a/include/snowflake/Simba_CRTFunctionSafe.h +++ b/include/snowflake/Simba_CRTFunctionSafe.h @@ -35,6 +35,8 @@ extern char* STDCALL sf_strerror(int); #if defined(WIN32) || defined(_WIN64) // For Windows + #define sf_free_s(x) free(x) + /// @brief Copy a string. /// /// @param out_dest Destination string buffer. (NOT OWN) @@ -244,11 +246,14 @@ extern char* STDCALL sf_strerror(int); } -#define sb_getenv sf_getenv -#define sb_strerror sf_strerror + #define sb_getenv sf_getenv + #define sb_strerror sf_strerror + #else // For all other platforms except Windows + #define sf_free_s(x) + /// @brief Copy a string. /// /// @param out_dest Destination string buffer. (NOT OWN) @@ -404,12 +409,14 @@ extern char* STDCALL sf_strerror(int); static inline FILE* sb_fopen(FILE** out_file, const char* in_filename, const char* in_mode) { #if defined(_WIN32) || defined(WIN32) || defined(_WIN64) - return fopen_s(out_file, in_filename, in_mode) ? NULL : *out_file; + // to simulate fopen, *out_file is guaranteed to be set, fopen_s don't change out_file on error + return fopen_s(out_file, in_filename, in_mode) ? (*out_file = NULL) : *out_file; #else return *out_file = fopen(in_filename, in_mode); #endif } +#define sb_free_s sf_free_s #ifdef __cplusplus diff --git a/include/snowflake/platform.h b/include/snowflake/platform.h index c7e60d2da1..ad113baba0 100755 --- a/include/snowflake/platform.h +++ b/include/snowflake/platform.h @@ -56,14 +56,22 @@ void STDCALL sf_tzset(void); int STDCALL sf_setenv(const char *name, const char *value); +/* on Windows, this function allocate new memory, caller should free it */ char *STDCALL sf_getenv(const char *name); int STDCALL sf_unsetenv(const char *name); int STDCALL sf_mkdir(const char *path); +/* on Windows, this function allocate new memory, caller should free it */ char* STDCALL sf_strerror(int errnum); +#ifdef _WIN32 +#define sf_free_s(x) free(x) +#else +#define sf_free_s(x) +#endif + int STDCALL _thread_init(SF_THREAD_HANDLE *thread, void *(*proc)(void *), void *arg); diff --git a/lib/client.c b/lib/client.c index c1cfdfdabe..d9d4a6a98e 100644 --- a/lib/client.c +++ b/lib/client.c @@ -349,8 +349,9 @@ static sf_bool STDCALL log_init(const char *log_path, SF_LOG_LEVEL log_level) { if (LOG_PATH != NULL) { // Create log file path (if it already doesn't exist) if (mkpath(LOG_PATH) == -1) { - sb_fprintf(stderr, "Error creating log directory. Error code: %s\n", - sf_strerror(errno)); + char* str_error = sf_strerror(errno); + sb_fprintf(stderr, "Error creating log directory. Error code: %s\n", str_error); + sf_free_s(str_error); goto cleanup; } // Set the log path only, the log file will be created when actual log output is needed. @@ -364,6 +365,10 @@ static sf_bool STDCALL log_init(const char *log_path, SF_LOG_LEVEL log_level) { ret = SF_BOOLEAN_TRUE; cleanup: + if (sf_log_path != log_path) { + sf_free_s((char*) sf_log_path); + } + sf_free_s((char*) sf_log_level_str); return ret; } diff --git a/lib/logger.c b/lib/logger.c index 0fa2dd5fc7..346d111a63 100644 --- a/lib/logger.c +++ b/lib/logger.c @@ -147,9 +147,11 @@ log_log_va_list(int level, const char *file, int line, const char *ns, { if (sb_fopen(&L.fp, L.path, "w+") == NULL) { + char* str_error = sf_strerror(errno); sb_fprintf(stderr, "Error opening file from file path: %s\nError code: %s\n", - L.path, sf_strerror(errno)); + L.path, str_error); + sf_free_s(str_error); L.path = NULL; } } diff --git a/lib/platform.c b/lib/platform.c index d8477a2a31..ec18c3d231 100755 --- a/lib/platform.c +++ b/lib/platform.c @@ -268,7 +268,8 @@ int STDCALL sf_setenv(const char *name, const char *value) { char * STDCALL sf_getenv(const char *name) { #ifdef _WIN32 - return getenv(name); + char* result = NULL; + return _dupenv_s(&result, NULL, name) ? NULL : result; # else return getenv(name); # endif @@ -293,7 +294,11 @@ int STDCALL sf_mkdir(const char *path) { char* STDCALL sf_strerror(int in_errNumber) { #ifdef _WIN32 - return strerror(in_errNumber); + char* buf = (char*)malloc(BUFSIZ); + if (buf && 0 == strerror_s(buf, BUFSIZ, in_errNumber)) + return buf; + free(buf); + return NULL; #else return strerror(in_errNumber); #endif