On Thu, 3 May 2012, Joe Mason wrote:

(I changed the subject to better reflect what we're actually discussing!)

> I've put a sample app up at http://pastebin.com/6jsuynGV

Awesome, thanks a lot. Does it work too? =)

> I changed the interface quite a bit from what I proposed below, since I
> decided that calling setopt from inside a callback is too weird. There's too
> much confusion about what you can and can't call.

Cool. Yes, I agree with you that it is a bit of a wormcan that we probably are
better off avoiding.

FYI, the git people have been struggling with a similar issue with auth and
libcurl as they want to figure out which proxy to use at run-time and thus
they cannot set user+password as a fixed value beforehand and they would also
greatly benefit from this kind of callback approach.

Here's git hacker Jeff King's response to my question about how he would like
"an ideal" system to work from their point of view:
http://marc.info/?l=git&m=133611728832402&w=2

> Instead, I'm using this callback signature:
>
> curl_auth_result (*curl_auth_callback)(curl_auth_type type,
> curl_auth_scheme scheme,
> const char *realm,
> unsigned retry_count,
> const char **username,
> const char **password,
> void *userdata);

We're getting an awful lot of arguments. I think we should consider filling
out a struct and pass in a pointer to it, like:

   curl_auth_result (*curl_auth_callback)(struct curl_authcb *info,
                                          void *userdata);

... and then popular the 'curl_authcb' struct properly.

I'm pretty sure we also need to pass in the exact URL that we tried to use
when the need for auth was triggered. I'm thinking in cases where libcurl
follows HTTP redirects and at some point down the line a URL wants auth...

I wonder if it would even be interesting from an application's point of view
to provide a pointer to the URL info deconstructed, like hostname separately.

> Where curl_auth_result can be CURLAUTH_RESULT_CONTINUE,
> CURLAUTH_RESULT_CANCEL or CURLAUTH_RESULT_PAUSE.

I think we also need some more fatal error code like if it runs out of memory
or similarly. Like CURLAUTH_RESULT_ERROR - due to errors it failed to do
anything sensible and this tells libcurl to bail out.

> username and password are in/out parameters: they point to the credentials
> used in the last failed login attempt if there is one, or NULL if the
> callback is being called because there are no credentials set. The user is
> expected to set new credentials with "*username = new_username; *password =
> new_password; return CURLAUTH_RESULT_CONTINUE". (The new strings will be
> strdup'd.)

I see a problem with this concept and I think we need to work on it a bit.

Let me elaborate:

An auth callback that wants to pass back a specific user name would need to
point to (possibly malloced) memory somewhere to hold that name and while
libcurl should indeed strdup that memory, how should the application code free
the memory? It being a callback makes it tricky for an application to store
that and free later, especially if the callback can get called again before
the request is done as then it may need to return back a _second_ name that
also might need to be allocated...

How do I suggest we do it instead?

I don't have a really good solution, but I think we must let the application
copy the data to a buffer libcurl holds so that the ownership gets easier and
it reduces the malloc problem. Possibly the struct mentioned above will point
to two buffers and hold two buffer sizes and then the application can copy at
most that many bytes to the destination. I realize it opens up for some nasty
mistakes and I would like an easier way but I can't really think of any atm.

> If they return CURLAUTH_RESULT_PAUSE, the username and password are ignored
> and the request is paused as if curl_easy_pause were called. It can then be
> resumed with the new function
>
> CURLcode curl_easy_resume_auth(CURL *handle, const char *username, const
> char *password);

I'm having more problems with this pause thing - especially since this adds a
very unique and special-pupose function, but I also realize it might be
necessary. The question is if a paused state is really that important instead
of just saying cancel if the info isn't there and then force a renewed request
later on when the information is indeed available. It would certainly make the
API cleaner and simpler.

An alternative approach (that may requite slightly more internal state
awareness) would make a curl_easy_pause(UNPAUSE) call make the auth callback
get called again if it previously returned CURLAUTH_RESULT_PAUSE and then the
transfer can continue like before.

> If the username and password are NULL, this is treated as if the user had
> returned CURLAUTH_RESULT_CANCEL, otherwise they're strdup'd and the request
> continues as if the user had returned CURLAUTH_RESULT_CONTINUE.

I think CURLAUTH_RESULT_CONTINUE should imply that we use the name and
password as provided and if they are NULL we use them blank.

> What do you think?

I like where this is leading!

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html