On Sun, 17 Nov 2013, Christian Grothoff wrote:

> > Wouldn't option 2 be better as a) it is more in keeping with the
> > the existing API where the application owns the memory and 2)
> > application can then destroy it at its own leisure - potentially
> > isn't there a scenario where libcurl destroys the connection and
> > the application still has a pointer to the tlsinfo address that has
> > now been destroyed?
>
> Right now, the tlsinfo pointer's destination would be freed (on
> curl_easy_cleanup), in the other case, the TLS session information
> reachable via the application-owned tlsinfo memory would be
> freed (on curl_easy_cleanup). Thus, in both cases doing anything
> with that memory will lead to disaster.

Exactly - neither one is great but I feel option 2 is the lesser of two
evils so to speak.

From an API point of view I can view one as memory that libcurl owns (option
1) whilst I can picture the other (option 2) where the application owns the
structure and libcurl owns the void* (the handle if you like) ;-)

> Yes, it is unsafe to use this information after curl_easy_cleanup;
> but there is no good way to make it safe.

I appreciate this has been discussed at length prior to me joining the
conversation a) Partly because I was busy with other libcurl features and 2)
It didn't quite make sense until I reviewed the patch ;-)

> > At present I'm not sure whether I would use a dedicated "capability"
> > structure for the information I want to get hold of or the existing
> > slist - but if it is the former then I would like to follow suit (for
> > consistency) and if it is the latter then the application owns the
> > memory and has to call
> > curl_slist_free_all() when it is done - a practice that libcurl
> > programmers are familiar with ;-)
>
> Note that the information returned is not an slist.

Indeed - I wasn't trying to say it was. I was simply trying to demonstrate
that libcurl programmers are used to either owning the memory completely or
telling libcurl to free the memory afterwards - either way they are in
control of when the memory is destroyed ;-)

> > Additionally, is the SSL backend type needed in this structure or
> > should we consider having it separate?
>
> I left is separate, as otherwise we need to include the header of
> the SSL backend in curl.h, which would be ugly given that there are
> many different backends.

Excuse my ignorance here (sorry) but I got slightly confused with your point
there - could you please elaborate?

> You are assuming that there will always be a 1:1 correspondence
> between SSL backends and returned void *-types. However, it might
> be that at some point we decide to return a different type depending
> on say the OpenSSL version in use (say OpeenSSL v2 offers a completely
> different kind of session handle). Then, with the enum, we can indicate
> this change easily by defining a second enum value for OpenSSL2, while
> possibly still returning the same name for the backend in curl_version
> and (if it is added) CURLINFO_SSL_BACKEND. I also don't like having
> another option just to get the backend, but that's a lesser reason for me.

Partly because we have a 1:1 relationship at the moment... but even if we
add OpenSSL2 to the enum, wouldn't CURLINFO_SSL_BACKEND still return that
enum (and as such "OpenSSL2") whilst CURLINFO_TLS_SESSION would return a
pointer to a void* that would then point to the new tlsinfo2 structure?

Even if CURLINFO_TLS_SESSION returned a pointer to the proposed tlsinfo
structure, that would then need modifying for the second handle, or whatever
it may be, and that would break ABI would it? I hope I haven't misunderstood
you there.

If that is the case then alternatively we could have either a) a union
contained within the structure containing each of the backend specific
variables or b) separate variables for each of the backends rather than a
simple "internals" (for example: ctx, session, handle, etc...). Both would
allow us to add to the structure at some point in the future.

If we went down this route, then I can see a reason for having the backend
as a member variable but I would still recommend that we have a lightweight
CURLINFO_SSL_BACKEND option so an application can easily find out what the
backend is without having to use CURLINFO_TLS_SESSION.

Thanks for your efforts so far and sorry if I have made things more
complicated ;-)

Kind Regards

Steve
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-11-18