cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Windows SSPI patch?

From: Daniel Stenberg <daniel-curl_at_haxx.se>
Date: Fri, 4 Mar 2005 16:56:22 +0100 (CET)

On Thu, 3 Mar 2005, Christopher R. Palmer wrote:

> Great! I wasn't sure if you'd be interested because it's a little painful
> to have very different pieces of code on different systems...

Well, if the additions are useful and appreciated by users, they will be used
and the code will remain functional that way even if I cannot build it myself.

> I've attached the patch to this email. As I said, if you want to
> incorporate it, I'm willing to do some work fixing up things you don't like
> about it.

Thanks a lot. I'm sure we can smoothen out the little quirks and have this
added soon.

Here's a few questions/remarks:

o The indent levels are not done curl-style. You also seem inconsistent even
   with your own levels.

o I think it is enough to depend on the USE_WINDOWS_SSPI define, and not check
   for WIN32 *as well*. Only Win32 users will/should define it.

o Can anyone come up with a use case where both kinds of NTLM auth could be
   good? I mean, both the curl-provided as well as the SSPI-approach? Your
   patch assumes that if you want SSPI, you don't want the code curl offers.

o You should check that strdup() returns a valid pointer or bail out nicely
   otherwise. We make an effort to have libcurl behave nicely even under
   low-memory situations.

o Since assume default user when no user is given, how do we know internally
   that authentication is wanted at all? In the current code, we use
   "conn->bits.user_passwd != NULL" to trigger that. This is already a bad
   assumption for Negotiate (see KNOWN_BUGS issue #10) so it might be about
   time we took care of it.

o Your modification of the #include <ca-bundle.h> to "ca-bundle.h" in
   lib/url.c is not a good idea since it breaks the build for all *nix systems
   (where the ca-bundle.h is generated in the build directory). You should
   instead run buildconf.bat to get your build env setup.

o Considered any docs/notes on how to enable this and even possibly for what
   systems this is assumed to work on?

o There were some absolute path references in there that I suspect you didn't
   intend to be included.

-- 
      Daniel Stenberg -- http://curl.haxx.se -- http://daniel.haxx.se
       Dedicated custom curl help for hire: http://haxx.se/curl.html
Received on 2005-03-04