On 04/01/2015 05:51 AM, Wenlong Dong wrote:
> Linus, thanks a lot for making the change! The change overall looks
> great to me. I was not able to patch it locally somehow (not sure
> whether I have to find the exact commit to sync to) and here are some
> minor comments:
>
> *) lib/http_negotiate.c:
> /+ char *spn = Curl_sasl_build_gssapi_spn(/
> /+ proxy ? "HTTP" : data->set.str[STRING_SERVICE_NAME],/
> /+ proxy ? conn->proxy.name <http://proxy.name> : conn->host.name
> <http://host.name>);/
>
> *Q1: Could data->set.str[STRING_SERVICE_NAME] be null or is it defaulted
> to "http" when it's not set (from lib/url.c)?*

The default should be HTTP, look at CURL_DEFAULT_SERVICE_NAME below.

> *Q2: When proxy is true, should you use
> data->set.str[STRING_PROXY_SERVICE_NAME] instead of "HTTP"?*

I was a little bit hesitant about that, since PROXY_SERVICE_NAME
defaults to "rcmd" at the moment, since it replaces the
SOCKS5_GSSAPI_SERVICE option. Maybe replacing it wasn't such a good idea
after all.

> /+ neg_ctx->server_name =/
> /+ Curl_sasl_build_spn(proxy ? "HTTP" :
> data->set.str[STRING_SERVICE_NAME],/
> /+ proxy ? conn->proxy.name <http://proxy.name>
> : conn->host.name <http://host.name>);/
>
> *Q3: Same questions as above.*
>
> *) lib/url.h
> /+#define CURL_DEFAULT_SERVICE_NAME "http" /* default negotiate service *//
>
> *Q4: Is the service name case sensitive? If it is, should it be "HTTP"?*

Good catch. It is. Use the attached patch instead. It should be applied
to the latest git master.

Linus

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