From e364546fb3371ac27f59f897e4c55acce7fc824e Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sun, 1 Mar 2020 16:16:19 +0100 Subject: [PATCH] version: make curl_version* thread-safe without using global context Closes #5010 --- lib/easy.c | 4 - lib/http2.c | 2 +- lib/version.c | 194 +++++++++++++++++++++++---------------------- lib/vquic/ngtcp2.c | 2 +- lib/vquic/quiche.c | 2 +- lib/vtls/vtls.h | 1 - 6 files changed, 101 insertions(+), 104 deletions(-) diff --git a/lib/easy.c b/lib/easy.c index f7a6d5c60..d1c6cedaa 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -83,8 +83,6 @@ #include "curl_memory.h" #include "memdebug.h" -void Curl_version_init(void); - /* true globals -- for curl_global_init() and curl_global_cleanup() */ static unsigned int initialized; static long init_flags; @@ -201,8 +199,6 @@ static CURLcode global_init(long flags, bool memoryfuncs) init_flags = flags; - Curl_version_init(); - return CURLE_OK; fail: diff --git a/lib/http2.c b/lib/http2.c index 72b38a3f6..41d8db685 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -333,7 +333,7 @@ static const struct Curl_handler Curl_handler_http2_ssl = { int Curl_http2_ver(char *p, size_t len) { nghttp2_info *h2 = nghttp2_version(0); - return msnprintf(p, len, " nghttp2/%s", h2->version_str); + return msnprintf(p, len, "nghttp2/%s", h2->version_str); } /* diff --git a/lib/version.c b/lib/version.c index 77aca7cbe..8170106ee 100644 --- a/lib/version.c +++ b/lib/version.c @@ -66,16 +66,6 @@ #include #endif -void Curl_version_init(void); - -/* For thread safety purposes this function is called by global_init so that - the static data in both version functions is initialized. */ -void Curl_version_init(void) -{ - curl_version(); - curl_version_info(CURLVERSION_NOW); -} - #ifdef HAVE_BROTLI static size_t brotli_version(char *buf, size_t bufsz) { @@ -88,95 +78,108 @@ static size_t brotli_version(char *buf, size_t bufsz) } #endif +/* + * curl_version() returns a pointer to a static buffer. + * + * It is implemented to work multi-threaded by making sure repeated invokes + * generate the exact same string and never write any temporary data like + * zeros in the data. + */ char *curl_version(void) { - static bool initialized; - static char version[250]; - char *ptr = version; - size_t len; - size_t left = sizeof(version); - - if(initialized) - return version; - - strcpy(ptr, LIBCURL_NAME "/" LIBCURL_VERSION); - len = strlen(ptr); - left -= len; - ptr += len; - - len = Curl_ssl_version(ptr + 1, left - 1); - - if(len > 0) { - *ptr = ' '; - left -= ++len; - ptr += len; - } - + static char out[250]; + char *outp; + size_t outlen; + const char *src[14]; +#ifdef USE_SSL + char ssl_version[40]; +#endif #ifdef HAVE_LIBZ - len = msnprintf(ptr, left, " zlib/%s", zlibVersion()); - left -= len; - ptr += len; + char z_version[40]; #endif #ifdef HAVE_BROTLI - len = msnprintf(ptr, left, "%s", " brotli/"); - left -= len; - ptr += len; - len = brotli_version(ptr, left); - left -= len; - ptr += len; + char br_version[40] = "brotli/"; #endif #ifdef USE_ARES - /* this function is only present in c-ares, not in the original ares */ - len = msnprintf(ptr, left, " c-ares/%s", ares_version(NULL)); - left -= len; - ptr += len; + char cares_version[40]; +#endif +#if defined(USE_LIBIDN2) || defined(USE_WIN32_IDN) + char idn_version[40]; +#endif +#ifdef USE_LIBPSL + char psl_version[40]; +#endif +#if defined(HAVE_ICONV) && defined(CURL_DOES_CONVERSIONS) + char iconv_version[40]="iconv"; +#endif +#ifdef USE_SSH + char ssh_version[40]; +#endif +#ifdef USE_NGHTTP2 + char h2_version[40]; +#endif +#ifdef ENABLE_QUIC + char h3_version[40]; +#endif +#ifdef USE_LIBRTMP + char rtmp_version[40]; +#endif + int i = 0; + int j; + + src[i++] = LIBCURL_NAME "/" LIBCURL_VERSION; +#ifdef USE_SSL + Curl_ssl_version(ssl_version, sizeof(ssl_version)); + src[i++] = ssl_version; +#endif +#ifdef HAVE_LIBZ + msnprintf(z_version, sizeof(z_version), "zlib/%s", zlibVersion()); + src[i++] = z_version; +#endif +#ifdef HAVE_BROTLI + brotli_version(&br_version[7], sizeof(br_version) - 7); + src[i++] = br_version; +#endif +#ifdef USE_ARES + msnprintf(cares_version, sizeof(cares_version), + "c-ares/%s", ares_version(NULL)); + src[i++] = cares_version; #endif #ifdef USE_LIBIDN2 if(idn2_check_version(IDN2_VERSION)) { - len = msnprintf(ptr, left, " libidn2/%s", idn2_check_version(NULL)); - left -= len; - ptr += len; + msnprintf(idn_version, sizeof(idn_version), + "libidn2/%s", idn2_check_version(NULL)); + src[i++] = idn_version; } +#elif defined(USE_WIN32_IDN) + msnprintf(idn_version, sizeof(idn_version), "WinIDN"); + src[i++] = idn_version; #endif + #ifdef USE_LIBPSL - len = msnprintf(ptr, left, " libpsl/%s", psl_get_version()); - left -= len; - ptr += len; -#endif -#ifdef USE_WIN32_IDN - len = msnprintf(ptr, left, " WinIDN"); - left -= len; - ptr += len; + msnprintf(psl_version, sizeof(psl_version), "libpsl/%s", psl_get_version()); + src[i++] = psl_version; #endif #if defined(HAVE_ICONV) && defined(CURL_DOES_CONVERSIONS) #ifdef _LIBICONV_VERSION - len = msnprintf(ptr, left, " iconv/%d.%d", - _LIBICONV_VERSION >> 8, _LIBICONV_VERSION & 255); + msnprintf(iconv_version, sizeof(iconv_version), "iconv/%d.%d", + _LIBICONV_VERSION >> 8, _LIBICONV_VERSION & 255); #else - /* version unknown */ - len = msnprintf(ptr, left, " iconv"); + /* version unknown, let the default stand */ #endif /* _LIBICONV_VERSION */ - left -= len; - ptr += len; + src[i++] = iconv_version; #endif #ifdef USE_SSH - if(left) { - *ptr++=' '; - left--; - } - len = Curl_ssh_version(ptr, left); - left -= len; - ptr += len; + Curl_ssh_version(ssh_version, sizeof(ssh_version)); + src[i++] = ssh_version; #endif #ifdef USE_NGHTTP2 - len = Curl_http2_ver(ptr, left); - left -= len; - ptr += len; + Curl_http2_ver(h2_version, sizeof(h2_version)); + src[i++] = h2_version; #endif #ifdef ENABLE_QUIC - len = Curl_quic_ver(ptr, left); - left -= len; - ptr += len; + Curl_quic_ver(h3_version, sizeof(h3_version)); + src[i++] = h3_version; #endif #ifdef USE_LIBRTMP { @@ -188,27 +191,32 @@ char *curl_version(void) else suff[0] = '\0'; - msnprintf(ptr, left, " librtmp/%d.%d%s", + msnprintf(rtmp_version, sizeof(rtmp_version), "librtmp/%d.%d%s", RTMP_LIB_VERSION >> 16, (RTMP_LIB_VERSION >> 8) & 0xff, suff); -/* - If another lib version is added below this one, this code would - also have to do: - - len = what msnprintf() returned - - left -= len; - ptr += len; -*/ + src[i++] = rtmp_version; } #endif - /* Silent scan-build even if librtmp is not enabled. */ - (void) left; - (void) ptr; + outp = &out[0]; + outlen = sizeof(out); + for(j = 0; j < i; j++) { + size_t n = strlen(src[j]); + /* we need room for a space, the string and the final zero */ + if(outlen <= (n + 2)) + break; + if(j) { + /* prepend a space if not the first */ + *outp++ = ' '; + outlen--; + } + memcpy(outp, src[j], n); + outp += n; + outlen -= n; + } + *outp = 0; - initialized = true; - return version; + return out; } /* data for curl_version_info @@ -391,7 +399,6 @@ static curl_version_info_data version_info = { curl_version_info_data *curl_version_info(CURLversion stamp) { - static bool initialized; #if defined(USE_SSH) static char ssh_buffer[80]; #endif @@ -406,9 +413,6 @@ curl_version_info_data *curl_version_info(CURLversion stamp) static char brotli_buffer[80]; #endif - if(initialized) - return &version_info; - #ifdef USE_SSL Curl_ssl_version(ssl_buffer, sizeof(ssl_buffer)); version_info.ssl_version = ssl_buffer; @@ -476,7 +480,5 @@ curl_version_info_data *curl_version_info(CURLversion stamp) #endif (void)stamp; /* avoid compiler warnings, we don't use this */ - - initialized = true; return &version_info; } diff --git a/lib/vquic/ngtcp2.c b/lib/vquic/ngtcp2.c index 0788404c0..2f6ee8bdf 100644 --- a/lib/vquic/ngtcp2.c +++ b/lib/vquic/ngtcp2.c @@ -641,7 +641,7 @@ int Curl_quic_ver(char *p, size_t len) { ngtcp2_info *ng2 = ngtcp2_version(0); nghttp3_info *ht3 = nghttp3_version(0); - return msnprintf(p, len, " ngtcp2/%s nghttp3/%s", + return msnprintf(p, len, "ngtcp2/%s nghttp3/%s", ng2->version_str, ht3->version_str); } diff --git a/lib/vquic/quiche.c b/lib/vquic/quiche.c index d09ba7038..c40e5e937 100644 --- a/lib/vquic/quiche.c +++ b/lib/vquic/quiche.c @@ -532,7 +532,7 @@ static ssize_t h3_stream_send(struct connectdata *conn, */ int Curl_quic_ver(char *p, size_t len) { - return msnprintf(p, len, " quiche/%s", quiche_version()); + return msnprintf(p, len, "quiche/%s", quiche_version()); } /* Index where :authority header field will appear in request header diff --git a/lib/vtls/vtls.h b/lib/vtls/vtls.h index f58adee6e..a81b2f22d 100644 --- a/lib/vtls/vtls.h +++ b/lib/vtls/vtls.h @@ -262,7 +262,6 @@ bool Curl_ssl_false_start(void); #define Curl_ssl_send(a,b,c,d,e) -1 #define Curl_ssl_recv(a,b,c,d,e) -1 #define Curl_ssl_initsessions(x,y) CURLE_OK -#define Curl_ssl_version(x,y) 0 #define Curl_ssl_data_pending(x,y) 0 #define Curl_ssl_check_cxn(x) 0 #define Curl_ssl_free_certinfo(x) Curl_nop_stmt