On 11/21/2016 10:55 AM, bjt 3 wrote:
>
> I've noticed an application creating a new connection for each request
> in a set when the target is the same. In this set of requests, the
> LOCALPORT/LOCALRANGE has been specified for each request but INTERFACE
> has not. The application cares about the local port range, but really
> doesn't care about the interfaces and is happy to use the default
> value of NULL.
>
>
> Some looking around leads me to:
>
>
> lib/url.c ConnectionExists()
>
>
> if(needle->localdev || needle->localport) {
> /* If we are bound to a specific local end (IP+port), we must not
> re-use a random other one, although if we didn't ask for a
> particular one we can reuse one that was bound.
>
> This comparison is a bit rough and too strict. Since the input
> parameters can be specified in numerous ways and still end
> up the
> same it would take a lot of processing to make it really
> accurate.
> Instead, this matching will assume that re-uses of bound
> connections
> will most likely also re-use the exact same binding
> parameters and
> missing out a few edge cases shouldn't hurt anyone very much.
> */
> if((check->localport != needle->localport) ||
> (check->localportrange != needle->localportrange) ||
> !check->localdev ||
> !needle->localdev ||
> strcmp(check->localdev, needle->localdev))
> continue;
> }
>
> I would have guessed, prior to seeing this code, that a request for
> any interface would have matched anything in the cache that matches
> the other criteria. This code appears to explicitly exclude that
> possibility and is the immediate cause of continual connection
> reconstruction for the case that I'm seeing. Once seeing this, three
> possibilities came to mind. needle->localdev being NULL should match
> anything; needle->localdev and check->localdev both being NULL should
> match; or, finally, that the code is correct and there are other
> considerations at work here that weren't clear to me. Hence, this email.
>
>
> My question is, is this coded as intended or overly restrictive ? If
> the former, I'd very much like to understand why so as to improve my
> understanding of libcurl.
>

interesting. this would loosen it a little:

diff --git a/lib/url.c b/lib/url.c
index 7106d46..5d0e21a 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -3400,9 +3400,9 @@ ConnectionExists(struct Curl_easy *data,
          */
          if((check->localport != needle->localport) ||
             (check->localportrange != needle->localportrange) ||
- !check->localdev ||
- !needle->localdev ||
- strcmp(check->localdev, needle->localdev))
+ ((check->localdev && needle->localdev) ?
+ strcmp(check->localdev, needle->localdev) :
+ (check->localdev || needle->localdev)))
            continue;
        }

-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-11-22