http: restrict auth methods to what the server advertises
By default, we tell curl to use CURLAUTH_ANY, which does not limit its set of auth methods. However, this results in an extra round-trip to the server when authentication is required. After we've fed the credential to curl, it wants to probe the server to find its list of available methods before sending an Authorization header. We can shortcut this by limiting our http_auth_methods by what the server told us it supports. In some cases (such as when the server only supports Basic), that lets curl skip the extra probe request. The end result should look the same to the user, but you can use GIT_TRACE_CURL to verify the sequence of requests: GIT_TRACE_CURL=1 \ git ls-remote https://example.com/repo.git \ 2>&1 >/dev/null | egrep '(Send|Recv) header: (GET|HTTP|Auth)' Before this patch, hitting a Basic-only server like github.com results in: Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1 Recv header: HTTP/1.1 401 Authorization Required Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1 Recv header: HTTP/1.1 401 Authorization Required Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1 Send header: Authorization: Basic <redacted> Recv header: HTTP/1.1 200 OK And after: Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1 Recv header: HTTP/1.1 401 Authorization Required Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1 Send header: Authorization: Basic <redacted> Recv header: HTTP/1.1 200 OK The possible downsides are: - This only helps for a Basic-only server; for a server with multiple auth options, curl may still send a probe request to see which ones are available (IOW, there's no way to say "don't probe, I already know what the server will say"). - The http_auth_methods variable is global, so this will apply to all further requests. That's acceptable for Git's usage of curl, though, which also treats the credentials as global. I.e., in any given program invocation we hit only one conceptual server (we may be redirected at the outset, but in that case that's whose auth_avail field we'd see). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
parent
3b9e3c2ced
commit
840398feb8
2
http.c
2
http.c
@ -1340,6 +1340,8 @@ static int handle_curl_result(struct slot_results *results)
|
|||||||
} else {
|
} else {
|
||||||
#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
|
#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
|
||||||
http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
|
http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
|
||||||
|
if (results->auth_avail)
|
||||||
|
http_auth_methods &= results->auth_avail;
|
||||||
#endif
|
#endif
|
||||||
return HTTP_REAUTH;
|
return HTTP_REAUTH;
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user