aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2024-04-02 16:05:17 -0400
committerJunio C Hamano <gitster@pobox.com>2024-04-02 13:27:18 -0700
commit324231174247c70e66908b78924908fbfcebed19 (patch)
tree4f4aa385c549e48f595049b348dc27fcd74ca5cf
parent3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0 (diff)
downloadgit-324231174247c70e66908b78924908fbfcebed19.tar.gz
http: reset POSTFIELDSIZE when clearing curl handle
In get_active_slot(), we return a CURL handle that may have been used before (reusing them is good because it lets curl reuse the same connection across many requests). We set a few curl options back to defaults that may have been modified by previous requests. We reset POSTFIELDS to NULL, but do not reset POSTFIELDSIZE (which defaults to "-1"). This usually doesn't matter because most POSTs will set both fields together anyway. But there is one exception: when handling a large request in remote-curl's post_rpc(), we don't set _either_, and instead set a READFUNCTION to stream data into libcurl. This can interact weirdly with a stale POSTFIELDSIZE setting, because curl will assume it should read only some set number of bytes from our READFUNCTION. However, it has worked in practice because we also manually set a "Transfer-Encoding: chunked" header, which libcurl uses as a clue to set the POSTFIELDSIZE to -1 itself. So everything works, but we're better off resetting the size manually for a few reasons: - there was a regression in curl 8.7.0 where the chunked header detection didn't kick in, causing any large HTTP requests made by Git to fail. This has since been fixed (but not yet released). In the issue, curl folks recommended setting it explicitly to -1: https://github.com/curl/curl/issues/13229#issuecomment-2029826058 and it indeed works around the regression. So even though it won't be strictly necessary after the fix there, this will help folks who end up using the affected libcurl versions. - it's consistent with what a new curl handle would look like. Since get_active_slot() may or may not return a used handle, this reduces the possibility of heisenbugs that only appear with certain request patterns. Note that the recommendation in the curl issue is to actually drop the manual Transfer-Encoding header. Modern libcurl will add the header itself when streaming from a READFUNCTION. However, that code wasn't added until 802aa5ae2 (HTTP: use chunked Transfer-Encoding for HTTP_POST if size unknown, 2019-07-22), which is in curl 7.66.0. We claim to support back to 7.19.5, so those older versions still need the manual header. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--http.c1
1 files changed, 1 insertions, 0 deletions
diff --git a/http.c b/http.c
index e73b136e58..3d80bd6116 100644
--- a/http.c
+++ b/http.c
@@ -1452,6 +1452,7 @@ struct active_request_slot *get_active_slot(void)
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, NULL);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, NULL);
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);
+ curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, -1L);
curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);