setopt: fix checking range for CURLOPT_MAXCONNECTS

[MAntoniak]

UINT_MAX can be converted to -1L. In this case, the value_range function will return an incorrect value of 0xFFFFFFFF, because 1 is greater than -1.

Using the upper limit as INT_MAX instead of UINT_MAX.

[jay]

I assume you are speaking about platforms where int and long are the same size, like Windows?

LONG_MAX may be greater than the maximum size of the uint32_t we assign to, how about the way we do it elsewhere:

diff --git a/lib/setopt.c b/lib/setopt.c
index 2fc90c2..5034c4a 100644
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -851,9 +851,9 @@ static CURLcode setopt_long_net(struct Curl_easy *data, CURLoption option,
     s->dns_cache_timeout_ms = -1;
     break;
   case CURLOPT_MAXCONNECTS:
-    result = value_range(&arg, 1, 1, UINT_MAX);
+    result = value_range(&arg, 1, 1, INT_MAX);
     if(!result)
-      s->maxconnects = (unsigned int)arg;
+      s->maxconnects = (uint32_t)arg;
     break;
   case CURLOPT_SERVER_RESPONSE_TIMEOUT:
     return setopt_set_timeout_sec(&s->server_response_timeout, arg);

[MAntoniak]

I assume you are speaking about platforms where int and long are the same size, like Windows?

Yes, I forgot to mention that.

LONG_MAX may be greater than the maximum size of the uint32_t we assign to, how about the way we do it elsewhere:

diff --git a/lib/setopt.c b/lib/setopt.c
index 2fc90c2..5034c4a 100644
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -851,9 +851,9 @@ static CURLcode setopt_long_net(struct Curl_easy *data, CURLoption option,
     s->dns_cache_timeout_ms = -1;
     break;
   case CURLOPT_MAXCONNECTS:
-    result = value_range(&arg, 1, 1, UINT_MAX);
+    result = value_range(&arg, 1, 1, INT_MAX);
     if(!result)
-      s->maxconnects = (unsigned int)arg;
+      s->maxconnects = (uint32_t)arg;
     break;
   case CURLOPT_SERVER_RESPONSE_TIMEOUT:
     return setopt_set_timeout_sec(&s->server_response_timeout, arg);

LGTM! I'll push your version.

[testclutch]

Analysis of PR #20414 at f8df1697:

Test 3027 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Generated by Testclutch

[jay]

Thanks