git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] http: reset POSTFIELDSIZE when clearing curl handle

From
Jeff King <peff@peff.net>
Date
Apr 2, 2024, 20:05 UTC
Message-ID
<20240402200517.GA875182@coredump.intra.peff.net>
In-Reply-To
<20240402200254.GA874754@coredump.intra.peff.net>

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>
---
 http.c | 1 +
 1 file changed, 1 insertion(+)
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);
-- 
2.44.0.789.g5ea01f6724
Previous: Jeff KingNext: Junio C Hamano
Message 4 of 15 in “tests broken with curl-8.7.0”
  1. Jeff KingMar 30, 2024
  2. Daniel StenbergMar 30, 2024
  3. 0/2 git+curl 8.7.0 workaroundJeff King, Apr 2, 2024
  4. 1/2 http: reset POSTFIELDSIZE when clearing curl handleJeff King, Apr 2, 2024
  5. Junio C HamanoApr 2, 2024
  6. Jeff KingApr 3, 2024
  7. Patrick SteinhardtApr 3, 2024
  8. Patrick SteinhardtApr 3, 2024
  9. Jeff KingApr 3, 2024
  10. 2/2 INSTALL: bump libcurl version to 7.21.3Jeff King, Apr 2, 2024
  11. rsbecker@nexbridge.comApr 2, 2024
  12. Jeff KingApr 2, 2024
  13. 3/2 remote-curl: add Transfer-Encoding header only for older curlJeff King, Apr 5, 2024
  14. Daniel StenbergApr 5, 2024
  15. Junio C HamanoApr 5, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.