Skip to content
  • Jeff King's avatar
    http: reset POSTFIELDSIZE when clearing curl handle · 32423117
    Jeff King authored and Junio C Hamano's avatar Junio C Hamano committed
    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: default avatarJeff King <peff@peff.net>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    32423117