On Tue, Nov 14, 2023 at 07:34:55PM -0800, Jiří Hruška wrote: > `get_active_slot()` makes sure that the reused cURL handles it gives > out are as good as fresh ones, by resetting all options that other code > might have set on them back to defaults. > > But this does not apply to `CURLOPT_POSTFIELDSIZE_LARGE` yet, which can > stay set from a previous request. For example, an earlier probe request > with just a flush packet "0000" leaves it set to 4. > > The problem seems harmless in practice, but it can be confusing to see > a negative amount of remaining bytes to upload when inspecting libcurl > internals while debugging networking-related issues, for example. > > So reset also this option to its default value (which is -1, not 0). > > Signed-off-by: Jiri Hruska <jirka@xxxxxx> > --- > http.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/http.c b/http.c > index 8f71bf00d8..14f2fbb82e 100644 > --- a/http.c > +++ b/http.c > @@ -1454,6 +1454,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_LARGE, (curl_off_t)-1); > curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0); > curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); > curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1); It feels quite easy for this list to grow stale whenever we start to set a new option somewhere else. Is there a specific reason why we can't instead use `curl_easy_reset()` here? Quoting its description: > Re-initializes all options previously set on a specified CURL handle > to the default values. This puts back the handle to the same state as > it was in when it was just created with curl_easy_init. > > It does not change the following information kept in the handle: live > connections, the Session ID cache, the DNS cache, the cookies, the > shares or the alt-svc cache. From my naive point of view it sounds like exactly what we're after. Most of the code in question was introduced in 9094950d73 (http: prevent segfault during curl handle reuse, 2006-05-31), where we used to support libcurl at least back to v7.7. `curl_easy_reset()` on the other hand had only been introduced with v7.12.1 of libcurl, so maybe that's the reason why it's not used here? I dunno, might as well be that there is a good reason why we don't use it here. But if we can, then I'd argue it would be a great cleanup to convert to `curl_easy_reset()` here instead of piling onto the list of options. Patrick
Attachment:
signature.asc
Description: PGP signature
