From d0688dcbdf412273e276dd29135b952f2065cb5b Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 19 Jan 2021 15:30:59 +0100 Subject: [PATCH] socks: use the download buffer instead The SOCKS code now uses the generic download buffer for temporary storage during the connection procedure, instead of having its own private 600 byte buffer that adds to the connectdata struct size. This works fine because this point the buffer is allocated but is not use for download yet since the connection hasn't completed. This reduces the connection struct size by 22% on a 64bit arch! The SOCKS buffer needs to be at least 600 bytes, and the download buffer is guaranteed to never be smaller than 1000 bytes. Closes #6491 --- lib/socks.c | 21 ++++++++++----------- lib/url.h | 16 ---------------- lib/urldata.h | 20 ++++++++++++++++---- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/lib/socks.c b/lib/socks.c index 64b04dd686..9252e2c174 100644 --- a/lib/socks.c +++ b/lib/socks.c @@ -195,7 +195,7 @@ CURLproxycode Curl_SOCKS4(const char *proxy_user, struct connectdata *conn = data->conn; const bool protocol4a = (conn->socks_proxy.proxytype == CURLPROXY_SOCKS4A) ? TRUE : FALSE; - unsigned char *socksreq = &conn->cnnct.socksreq[0]; + unsigned char *socksreq = (unsigned char *)data->state.buffer; CURLcode result; curl_socket_t sockfd = conn->sock[sockindex]; struct connstate *sx = &conn->cnnct; @@ -203,6 +203,9 @@ CURLproxycode Curl_SOCKS4(const char *proxy_user, ssize_t actualread; ssize_t written; + /* make sure that the buffer is at least 600 bytes */ + DEBUGASSERT(READBUFFER_MIN >= 600); + if(!SOCKS_STATE(sx->state) && !*done) sxstate(data, CONNECT_SOCKS_INIT); @@ -319,7 +322,7 @@ CURLproxycode Curl_SOCKS4(const char *proxy_user, socksreq[8] = 0; /* ensure empty userid is NUL-terminated */ if(proxy_user) { size_t plen = strlen(proxy_user); - if(plen >= sizeof(sx->socksreq) - 8) { + if(plen >= (size_t)data->set.buffer_size - 8) { failf(data, "Too long SOCKS proxy user name, can't use!"); return CURLPX_LONG_USER; } @@ -438,8 +441,7 @@ CURLproxycode Curl_SOCKS4(const char *proxy_user, failf(data, "Can't complete SOCKS4 connection to %d.%d.%d.%d:%d. (%d)" ", request rejected or failed.", - (unsigned char)socksreq[4], (unsigned char)socksreq[5], - (unsigned char)socksreq[6], (unsigned char)socksreq[7], + socksreq[4], socksreq[5], socksreq[6], socksreq[7], (((unsigned char)socksreq[2] << 8) | (unsigned char)socksreq[3]), (unsigned char)socksreq[1]); return CURLPX_REQUEST_FAILED; @@ -448,8 +450,7 @@ CURLproxycode Curl_SOCKS4(const char *proxy_user, "Can't complete SOCKS4 connection to %d.%d.%d.%d:%d. (%d)" ", request rejected because SOCKS server cannot connect to " "identd on the client.", - (unsigned char)socksreq[4], (unsigned char)socksreq[5], - (unsigned char)socksreq[6], (unsigned char)socksreq[7], + socksreq[4], socksreq[5], socksreq[6], socksreq[7], (((unsigned char)socksreq[2] << 8) | (unsigned char)socksreq[3]), (unsigned char)socksreq[1]); return CURLPX_IDENTD; @@ -458,8 +459,7 @@ CURLproxycode Curl_SOCKS4(const char *proxy_user, "Can't complete SOCKS4 connection to %d.%d.%d.%d:%d. (%d)" ", request rejected because the client program and identd " "report different user-ids.", - (unsigned char)socksreq[4], (unsigned char)socksreq[5], - (unsigned char)socksreq[6], (unsigned char)socksreq[7], + socksreq[4], socksreq[5], socksreq[6], socksreq[7], (((unsigned char)socksreq[2] << 8) | (unsigned char)socksreq[3]), (unsigned char)socksreq[1]); return CURLPX_IDENTD_DIFFER; @@ -467,8 +467,7 @@ CURLproxycode Curl_SOCKS4(const char *proxy_user, failf(data, "Can't complete SOCKS4 connection to %d.%d.%d.%d:%d. (%d)" ", Unknown.", - (unsigned char)socksreq[4], (unsigned char)socksreq[5], - (unsigned char)socksreq[6], (unsigned char)socksreq[7], + socksreq[4], socksreq[5], socksreq[6], socksreq[7], (((unsigned char)socksreq[2] << 8) | (unsigned char)socksreq[3]), (unsigned char)socksreq[1]); return CURLPX_UNKNOWN_FAIL; @@ -507,7 +506,7 @@ CURLproxycode Curl_SOCKS5(const char *proxy_user, o X'00' succeeded */ struct connectdata *conn = data->conn; - unsigned char *socksreq = &conn->cnnct.socksreq[0]; + unsigned char *socksreq = (unsigned char *)data->state.buffer; char dest[256] = "unknown"; /* printable hostname:port */ int idx; ssize_t actualread; diff --git a/lib/url.h b/lib/url.h index 6c71c1f75b..ce4870ea60 100644 --- a/lib/url.h +++ b/lib/url.h @@ -23,22 +23,6 @@ ***************************************************************************/ #include "curl_setup.h" -#define READBUFFER_SIZE CURL_MAX_WRITE_SIZE -#define READBUFFER_MAX CURL_MAX_READ_SIZE -#define READBUFFER_MIN 1024 - -/* The default upload buffer size, should not be smaller than - CURL_MAX_WRITE_SIZE, as it needs to hold a full buffer as could be sent in - a write callback. - - The size was 16KB for many years but was bumped to 64KB because it makes - libcurl able to do significantly faster uploads in some circumstances. Even - larger buffers can help further, but this is deemed a fair memory/speed - compromise. */ -#define UPLOADBUFFER_DEFAULT 65536 -#define UPLOADBUFFER_MAX (2*1024*1024) -#define UPLOADBUFFER_MIN CURL_MAX_WRITE_SIZE - /* * Prototypes for library-wide functions provided by url.c */ diff --git a/lib/urldata.h b/lib/urldata.h index 1aa5850772..0038e580aa 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -160,6 +160,22 @@ typedef CURLcode (*Curl_datastream)(struct Curl_easy *data, #include #endif /* HAVE_LIBSSH2_H */ +#define READBUFFER_SIZE CURL_MAX_WRITE_SIZE +#define READBUFFER_MAX CURL_MAX_READ_SIZE +#define READBUFFER_MIN 1024 + +/* The default upload buffer size, should not be smaller than + CURL_MAX_WRITE_SIZE, as it needs to hold a full buffer as could be sent in + a write callback. + + The size was 16KB for many years but was bumped to 64KB because it makes + libcurl able to do significantly faster uploads in some circumstances. Even + larger buffers can help further, but this is deemed a fair memory/speed + compromise. */ +#define UPLOADBUFFER_DEFAULT 65536 +#define UPLOADBUFFER_MAX (2*1024*1024) +#define UPLOADBUFFER_MIN CURL_MAX_WRITE_SIZE + #define CURLEASY_MAGIC_NUMBER 0xc0dedbadU #define GOOD_EASY_HANDLE(x) \ ((x) && ((x)->magic == CURLEASY_MAGIC_NUMBER)) @@ -868,13 +884,9 @@ enum connect_t { #define SOCKS_STATE(x) (((x) >= CONNECT_SOCKS_INIT) && \ ((x) < CONNECT_DONE)) -#define SOCKS_REQUEST_BUFSIZE 600 /* room for large user/pw (255 max each) */ struct connstate { enum connect_t state; - unsigned char socksreq[SOCKS_REQUEST_BUFSIZE]; - - /* CONNECT_SOCKS_SEND */ ssize_t outstanding; /* send this many bytes more */ unsigned char *outp; /* send from this pointer */ };