deps: update libcurl to 8.19.0#435
Open
santigimeno wants to merge 1 commit intonode-v24.x-nsolid-v6.xfrom
Open
deps: update libcurl to 8.19.0#435santigimeno wants to merge 1 commit intonode-v24.x-nsolid-v6.xfrom
santigimeno wants to merge 1 commit intonode-v24.x-nsolid-v6.xfrom
Conversation
WalkthroughUpdates curl library from version 8.18.0 to 8.19.0 with extensive architectural refactoring. Major changes include: transitioning from handler-centric to scheme-centric protocol organization, consolidating memory helpers and includes under curlx/ namespace, standardizing parameter names across public APIs, adjusting verbose output gating, and enhancing timeout and socket handling mechanisms. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deps/curl/lib/cfilters.c (1)
601-612:⚠️ Potential issue | 🟡 MinorGuard
conn->schemebefore using non-network fallback.
Curl_conn_is_connected()dereferencesconn->scheme->flagswithout a null check. If this is called on a partially initialized connection, it can crash.💡 Suggested fix
bool Curl_conn_is_connected(struct connectdata *conn, int sockindex) { struct Curl_cfilter *cf; - if(!CONN_SOCK_IDX_VALID(sockindex)) + if(!conn || !CONN_SOCK_IDX_VALID(sockindex)) return FALSE; cf = conn->cfilter[sockindex]; if(cf) return (bool)cf->connected; - else if(conn->scheme->flags & PROTOPT_NONETWORK) + else if(conn->scheme && (conn->scheme->flags & PROTOPT_NONETWORK)) return TRUE; return FALSE; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deps/curl/lib/cfilters.c` around lines 601 - 612, Curl_conn_is_connected dereferences conn->scheme->flags without checking conn->scheme; update the function Curl_conn_is_connected to guard access to conn->scheme before using PROTOPT_NONETWORK (e.g. only evaluate (conn->scheme->flags & PROTOPT_NONETWORK) when conn->scheme is non-NULL) and keep existing behavior for conn->cfilter/ cf->connected; if conn->scheme is NULL, treat it as not having PROTOPT_NONETWORK (return FALSE).deps/curl/lib/doh.c (1)
1254-1278:⚠️ Potential issue | 🟠 MajorPropagate DNS-entry creation/cache failures instead of returning success.
If
Curl_dnscache_mk_entry()returnsNULL, this block still reaches Line 1283 withresult == CURLE_OKand reports a completed lookup with no entry. ACurl_dnscache_add()failure also leavesdata->state.async.dnslive on the error path.Suggested fix
/* we got a response, create a dns entry. */ dns = Curl_dnscache_mk_entry(data, &ai, dohp->host, 0, dohp->port, FALSE); - if(dns) { + if(!dns) { + result = CURLE_OUT_OF_MEMORY; + goto error; + } + { /* Now add and HTTPSRR information if we have */ `#ifdef` USE_HTTPSRR if(de.numhttps_rrs > 0 && result == CURLE_OK) { struct Curl_https_rrinfo *hrr = NULL; result = doh_resp_decode_httpsrr(data, de.https_rrs->val, @@ `#endif` /* USE_HTTPSRR */ /* and add the entry to the cache */ data->state.async.dns = dns; result = Curl_dnscache_add(data, dns); + if(result) { + Curl_resolv_unlink(data, &data->state.async.dns); + goto error; + } *dnsp = data->state.async.dns; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deps/curl/lib/doh.c` around lines 1254 - 1278, If Curl_dnscache_mk_entry() returns NULL or Curl_dnscache_add() fails you must not report success; check the return of Curl_dnscache_mk_entry (dns) and set an appropriate error (e.g. result = CURLE_OUT_OF_MEMORY or CURLE_COULDNT_RESOLVE_HOST) and goto error when NULL, and after calling Curl_dnscache_add detect non-OK result, clear/unlink data->state.async.dns (use Curl_resolv_unlink or free the dns entry) so it isn't left live, and then propagate the error (goto error); update handling around the symbols Curl_dnscache_mk_entry, dns, data->state.async.dns and Curl_dnscache_add to implement these checks and cleanup.
🧹 Nitpick comments (1)
deps/curl/lib/Makefile.in (1)
1034-1034: Align variable usage across Makefile.am and regenerate Makefile.in.The declaration
CLANG_TIDY =@CLANG_TIDY@exists in Makefile.in (line 1034), but the recipe at line 5933 uses@CLANG_TIDY@directly instead of$(CLANG_TIDY). More importantly, the source fileMakefile.amlacks this variable declaration entirely and also uses@CLANG_TIDY@in its recipe (line 178).To enable straightforward
make CLANG_TIDY=... tidyoverrides, add the variable declaration toMakefile.amand update the recipe to use$(CLANG_TIDY), then regenerateMakefile.infrom it rather than editing the generated file directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deps/curl/lib/Makefile.in` at line 1034, Add a declarative variable for CLANG_TIDY in Makefile.am (e.g., CLANG_TIDY = `@CLANG_TIDY`@) and update any recipes in Makefile.am that currently use the literal `@CLANG_TIDY`@ (the tidy recipe referenced) to use $(CLANG_TIDY) instead so overrides like `make CLANG_TIDY=... tidy` work; then regenerate Makefile.in from the updated Makefile.am (do not edit Makefile.in directly) so the generated CLANG_TIDY = `@CLANG_TIDY`@ line in Makefile.in and all recipe usages are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deps/curl/lib/altsvc.c`:
- Around line 312-323: The function Curl_altsvc_ctrl currently rejects ctrl == 0
which prevents resetting data->asi->flags; change the input validation in
Curl_altsvc_ctrl so zero is accepted as a valid "reset/disable" value and is
assigned to data->asi->flags, and instead validate only against unknown bits
(e.g., compute a ALTSVC_CTRL_VALID mask and return CURLE_BAD_FUNCTION_ARGUMENT
if (ctrl & ~ALTSVC_CTRL_VALID) != 0); ensure data->asi is still allocated as
before and that data->asi->flags = ctrl handles the zero case.
In `@deps/curl/lib/asyn-ares.c`:
- Around line 329-335: The OOM path after Curl_dnscache_mk_entry currently jumps
to the early "out" label and skips async_ares_cleanup, leaking ares->hinfo and
nested allocations; change the control flow so that when Curl_dnscache_mk_entry
returns NULL you set result = CURLE_OUT_OF_MEMORY and jump to the common cleanup
that runs async_ares_cleanup (or call async_ares_cleanup before jumping out)
instead of jumping directly to out. Locate the block around
Curl_dnscache_mk_entry and replace the direct goto out with a goto or call that
ensures async_ares_cleanup(data) runs (referencing Curl_dnscache_mk_entry,
async_ares_cleanup, and ares->hinfo).
In `@deps/curl/lib/cf-ip-happy.c`:
- Around line 490-493: The final trace loop uses a local index i (set by
VERBOSE(i = 0)) but never increments it, so every trace prints "baller 0";
inside the loop that iterates for(a = bs->running; a; a = a->next) in
cf-ip-happy.c, increment i each iteration (e.g., i++ at the end of the loop
body) before or after calling CURL_TRC_CF so the trace message shows the correct
attempt index for each entry.
In `@deps/curl/lib/cf-socket.c`:
- Around line 315-317: The code mutates addr->socktype to add SOCK_CLOEXEC
before invoking the user fopen socket callback, which exposes a flag-augmented
value to that callback; change this so the public fopen socket callback
(data->set.fopensocket / the fopensocket invocation at the call site) sees the
original plain socket type (e.g., SOCK_STREAM/SOCK_DGRAM). Either apply
SOCK_CLOEXEC only after checking if data->set.fopensocket is set (like the
SOCK_NONBLOCK pattern) or pass a local copy of addr->socktype to the callback
and set the CLOEXEC flag on the actual addr->socktype only when creating the
socket; ensure references to addr->socktype and the fopensocket callback name
are used to locate and adjust the logic.
- Around line 357-374: Replace the direct sclose(*sockfd) calls in the socket
setup failure paths (inside the Curl_sock_nosigpipe and fcntl error blocks) with
the callback-aware close helper socket_close(data, data->conn, TRUE, *sockfd) so
any registered fclosesocket callback gets invoked; after invoking socket_close
set *sockfd = CURL_SOCKET_BAD and return CURLE_COULDNT_CONNECT as before. Ensure
you update both failure branches where sclose is used (the block around
Curl_sock_nosigpipe and the block around fcntl(F_SETFD, FD_CLOEXEC)) and keep
the existing failf messages and error returns unchanged.
In `@deps/curl/lib/curl_threads.h`:
- Line 44: The macro Curl_mutex_init currently calls
InitializeCriticalSectionEx(m, 0, 1) with an invalid Flags value; change the
third argument to a valid flag (either 0 or
CRITICAL_SECTION_NO_DEBUG_INFO/0x01000000) so initialization uses a supported
flag. Update the Curl_mutex_init definition to pass the proper flag constant
(and include or ensure CRITICAL_SECTION_NO_DEBUG_INFO is available) instead of
the literal 1 to avoid silent failures.
In `@deps/curl/lib/curl_trc.c`:
- Around line 204-225: The Curl_trc_feat_* symbols (Curl_trc_feat_multi,
Curl_trc_feat_read, Curl_trc_feat_write, Curl_trc_feat_dns, Curl_trc_feat_timer)
are only defined under `#ifdef` CURLVERBOSE but are referenced by code paths that
can be compiled when CURL_DISABLE_VERBOSE_STRINGS is defined (and CURLVERBOSE is
not), causing undefined symbols; fix by aligning the guards so consumers and
providers match: either move the definitions in curl_trc.c out of the
CURLVERBOSE guard and into a `#ifndef` CURL_DISABLE_VERBOSE_STRINGS block (so the
symbols exist whenever verbose strings are enabled), or change the header guard
at the consumer block to require CURLVERBOSE as well—update the preprocessor
condition around the Curl_trc_feat_* definitions (and any corresponding
declarations) so both definition and use are controlled by the same macro to
eliminate the linker error.
In `@deps/curl/lib/curlx/basename.c`:
- Around line 54-71: The curlx_basename fallback (function curlx_basename) must
first implement POSIX basename semantics: if path is NULL or empty return ".",
strip trailing slashes (while preserving root) before finding the last
separator, and if the remaining string consists only of slashes return "/"
(root); otherwise return the final component after the last '/' or '\\'. Modify
curlx_basename to check for NULL/empty, remove trailing separators with care to
detect root-only inputs, then locate the last separator via strrchr and return
the correct component per POSIX.
In `@deps/curl/lib/curlx/snprintf.h`:
- Around line 30-37: The Windows wrapper curlx_win32_snprintf currently has a
void return type while SNPRINTF is aliased to it under WITHOUT_LIBCURL, causing
a signature mismatch with snprintf (which returns int); change the declaration
of curlx_win32_snprintf to return int (matching snprintf) and update its
implementation to return the number of characters written (or a compatible
snprintf-style value), then keep the macro SNPRINTF mapped to that function so
all branches share the same int return contract.
In `@deps/curl/lib/http_aws_sigv4.c`:
- Around line 150-197: The split_to_dyn_array function currently treats reaching
MAX_QUERY_COMPONENTS as overflow because it checks for equality after
incrementing num_splits; change both overflow checks (the one inside the for
loop after storing a segment and the one after the final segment handling) to
trigger only when num_splits exceeds MAX_QUERY_COMPONENTS (i.e., use
if(++num_splits > MAX_QUERY_COMPONENTS) result = CURLE_TOO_LARGE) so exactly
MAX_QUERY_COMPONENTS entries are allowed; update references where num_splits is
incremented to use the > check accordingly.
In `@deps/curl/lib/http.c`:
- Around line 1435-1441: In Curl_compareheader the loop only runs once because
len is initialized and compared to the same curlx_strlen(&val) value; compute
the total length once (e.g. size_t total = curlx_strlen(&val)), set len = total
and iterate while len >= clen, advancing p and decrementing len so you test
every possible start position where remaining length >= clen; update uses of
curlx_strlen(&val) in that block to use the cached total and keep the comparison
using clen, referencing variables val, clen, p, len and the Curl_compareheader
function.
- Around line 516-527: The 401 handling in http_should_fail() treats any missing
data->state.aptr.user as terminal and will trigger CURLOPT_FAILONERROR before
Bearer auth can be selected; update the 401 check to allow continuation when a
Bearer credential is present by also checking the bearer string (e.g.
data->state.aptr.bearer or the STRING_BEARER storage used for CURLAUTH_BEARER)
so change the condition in the if((httpcode == 401) && !data->state.aptr.user)
branch to also verify there is no bearer token (e.g. &&
!data->state.aptr.bearer) so Curl_http_auth_act() can still pick
CURLAUTH_BEARER.
In `@deps/curl/lib/http2.c`:
- Around line 263-266: The problem is that H2_STREAM_CTX() looks up stream state
by data->mid, but h2_duphandle() calls http2_data_setup() on a duplicated push
handle before Curl_multi_add_perform() assigns newhandle->mid, which creates an
orphaned key-0 h2_stream_ctx; to fix, stop creating/looking up a stream context
keyed by mid while mid==0: either (A) make H2_STREAM_CTX() return NULL when
(data)->mid is zero and ensure http2_data_setup() is not invoked in
h2_duphandle() until after Curl_multi_add_perform() has assigned newhandle->mid,
or (B) defer the call to http2_data_setup() from h2_duphandle() to the
post-Curl_multi_add_perform() path where newhandle->mid is non-zero (references:
H2_STREAM_CTX, h2_duphandle(), http2_data_setup(), Curl_multi_add_perform(),
data->mid, newhandle->mid).
In `@deps/curl/lib/http2.h`:
- Around line 39-42: Add a direct include of nghttp2's public header so the
nghttp2_frame type is declared where Curl_nghttp2_fr_print is declared: modify
deps/curl/lib/http2.h to `#include` <nghttp2/nghttp2.h> (preferably guarded by the
existing macro that enables nghttp2, e.g. surround the include and the
Curl_nghttp2_fr_print prototype with the same USE_NGHTTP2/CURL_DISABLE_HTTP2
conditional) so callers of http2.h (like cf-https-connect.c, curl_trc.c,
multi.c) get the nghttp2_frame typedef when CURLVERBOSE is enabled.
In `@deps/curl/lib/imap.c`:
- Around line 1179-1214: The matcher currently only inspects the message-set and
thus treats any FETCH with a range/comma as a "listing", which causes BODY[]
responses to be swallowed; update is_custom_fetch_listing() (and its helper
is_custom_fetch_listing_match()) to not just recognize the message-set but also
verify the fetch data-item token that follows and only return true for
listing-style queries (e.g. when the data-item is FLAGS/ (FLAGS ...)), otherwise
return FALSE so UID/FETCH ... BODY[] responses are handled normally by
imap_state_listsearch_resp().
- Around line 219-236: The dynbuf initialized by curlx_dyn_init(&line, 2000)
must be freed on all error paths in imap_atom(): when any curlx_dyn_addn() fails
you should call curlx_dyn_free(&line) before returning NULL (or refactor to a
single cleanup label and jump there on error), ensuring curlx_dyn_free(&line) is
called for every early return and only return curlx_dyn_ptr(&line) on success.
- Around line 1697-1709: The imap_easy_reset function currently frees string
fields but doesn't clear the UIDVALIDITY tracking, so subsequent requests on the
same easy handle inherit the previous request's UIDVALIDITY constraint; update
imap_easy_reset to reset the UIDVALIDITY state by clearing imap->uidvalidity_set
(and reset imap->uidvalidity as well) so the next transfer starts without the
previous UIDVALIDITY constraint, referencing the imap_easy_reset function and
the imap->uidvalidity_set/imap->uidvalidity fields.
In `@deps/curl/lib/mprintf.c`:
- Around line 558-564: The va_arg usage for %ll boundaries is incorrect: in the
switch cases MTYPE_LONGLONGU and MTYPE_LONGLONG replace va_arg(arglist,
uint64_t)/va_arg(arglist, int64_t) with va_arg(arglist, unsigned long
long)/va_arg(arglist, long long) and then cast the returned long long/unsigned
long long to the union fields iptr->val.numu and iptr->val.nums respectively;
additionally, where FLAGS_LONGLONG causes a pointer to be stored/retrieved (the
code that currently casts to int64_t *), change that cast to long long * (or
unsigned long long * as appropriate) before assigning/copying so the varargs
boundary uses long long types per the %ll specifiers.
---
Outside diff comments:
In `@deps/curl/lib/cfilters.c`:
- Around line 601-612: Curl_conn_is_connected dereferences conn->scheme->flags
without checking conn->scheme; update the function Curl_conn_is_connected to
guard access to conn->scheme before using PROTOPT_NONETWORK (e.g. only evaluate
(conn->scheme->flags & PROTOPT_NONETWORK) when conn->scheme is non-NULL) and
keep existing behavior for conn->cfilter/ cf->connected; if conn->scheme is
NULL, treat it as not having PROTOPT_NONETWORK (return FALSE).
In `@deps/curl/lib/doh.c`:
- Around line 1254-1278: If Curl_dnscache_mk_entry() returns NULL or
Curl_dnscache_add() fails you must not report success; check the return of
Curl_dnscache_mk_entry (dns) and set an appropriate error (e.g. result =
CURLE_OUT_OF_MEMORY or CURLE_COULDNT_RESOLVE_HOST) and goto error when NULL, and
after calling Curl_dnscache_add detect non-OK result, clear/unlink
data->state.async.dns (use Curl_resolv_unlink or free the dns entry) so it isn't
left live, and then propagate the error (goto error); update handling around the
symbols Curl_dnscache_mk_entry, dns, data->state.async.dns and Curl_dnscache_add
to implement these checks and cleanup.
---
Nitpick comments:
In `@deps/curl/lib/Makefile.in`:
- Line 1034: Add a declarative variable for CLANG_TIDY in Makefile.am (e.g.,
CLANG_TIDY = `@CLANG_TIDY`@) and update any recipes in Makefile.am that currently
use the literal `@CLANG_TIDY`@ (the tidy recipe referenced) to use $(CLANG_TIDY)
instead so overrides like `make CLANG_TIDY=... tidy` work; then regenerate
Makefile.in from the updated Makefile.am (do not edit Makefile.in directly) so
the generated CLANG_TIDY = `@CLANG_TIDY`@ line in Makefile.in and all recipe
usages are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6c3bb3d-bc7e-47c6-95c2-adb3ff1c16fb
📒 Files selected for processing (300)
deps/curl/.clang-tidy.ymldeps/curl/COPYINGdeps/curl/curl.gypdeps/curl/include/Makefile.indeps/curl/include/curl/Makefile.amdeps/curl/include/curl/Makefile.indeps/curl/include/curl/curl.hdeps/curl/include/curl/curlver.hdeps/curl/include/curl/easy.hdeps/curl/include/curl/header.hdeps/curl/include/curl/multi.hdeps/curl/include/curl/system.hdeps/curl/include/curl/typecheck-gcc.hdeps/curl/include/curl/urlapi.hdeps/curl/lib/CMakeLists.txtdeps/curl/lib/Makefile.amdeps/curl/lib/Makefile.indeps/curl/lib/Makefile.incdeps/curl/lib/altsvc.cdeps/curl/lib/altsvc.hdeps/curl/lib/amigaos.cdeps/curl/lib/arpa_telnet.hdeps/curl/lib/asyn-ares.cdeps/curl/lib/asyn-base.cdeps/curl/lib/asyn-thrdd.cdeps/curl/lib/asyn.hdeps/curl/lib/bufq.hdeps/curl/lib/bufref.cdeps/curl/lib/cf-h1-proxy.cdeps/curl/lib/cf-h1-proxy.hdeps/curl/lib/cf-h2-proxy.cdeps/curl/lib/cf-haproxy.cdeps/curl/lib/cf-https-connect.cdeps/curl/lib/cf-ip-happy.cdeps/curl/lib/cf-socket.cdeps/curl/lib/cf-socket.hdeps/curl/lib/cfilters.cdeps/curl/lib/cfilters.hdeps/curl/lib/config-mac.hdeps/curl/lib/config-os400.hdeps/curl/lib/config-plan9.hdeps/curl/lib/config-riscos.hdeps/curl/lib/config-win32.hdeps/curl/lib/conncache.cdeps/curl/lib/conncache.hdeps/curl/lib/connect.cdeps/curl/lib/connect.hdeps/curl/lib/content_encoding.cdeps/curl/lib/cookie.cdeps/curl/lib/cookie.hdeps/curl/lib/cshutdn.cdeps/curl/lib/cshutdn.hdeps/curl/lib/curl_addrinfo.cdeps/curl/lib/curl_addrinfo.hdeps/curl/lib/curl_config-cmake.h.indeps/curl/lib/curl_config.h.indeps/curl/lib/curl_fnmatch.cdeps/curl/lib/curl_fopen.cdeps/curl/lib/curl_get_line.cdeps/curl/lib/curl_gssapi.cdeps/curl/lib/curl_gssapi.hdeps/curl/lib/curl_hmac.hdeps/curl/lib/curl_ldap.hdeps/curl/lib/curl_md5.hdeps/curl/lib/curl_ntlm_core.cdeps/curl/lib/curl_ntlm_core.hdeps/curl/lib/curl_printf.hdeps/curl/lib/curl_range.cdeps/curl/lib/curl_rtmp.cdeps/curl/lib/curl_rtmp.hdeps/curl/lib/curl_sasl.cdeps/curl/lib/curl_sasl.hdeps/curl/lib/curl_setup.hdeps/curl/lib/curl_setup_once.hdeps/curl/lib/curl_sha256.hdeps/curl/lib/curl_sha512_256.cdeps/curl/lib/curl_share.cdeps/curl/lib/curl_share.hdeps/curl/lib/curl_sspi.cdeps/curl/lib/curl_sspi.hdeps/curl/lib/curl_threads.hdeps/curl/lib/curl_trc.cdeps/curl/lib/curl_trc.hdeps/curl/lib/curlx/base64.cdeps/curl/lib/curlx/basename.cdeps/curl/lib/curlx/basename.hdeps/curl/lib/curlx/curlx.hdeps/curl/lib/curlx/dynbuf.cdeps/curl/lib/curlx/fopen.cdeps/curl/lib/curlx/fopen.hdeps/curl/lib/curlx/inet_ntop.cdeps/curl/lib/curlx/inet_ntop.hdeps/curl/lib/curlx/inet_pton.cdeps/curl/lib/curlx/inet_pton.hdeps/curl/lib/curlx/multibyte.cdeps/curl/lib/curlx/multibyte.hdeps/curl/lib/curlx/nonblock.cdeps/curl/lib/curlx/snprintf.cdeps/curl/lib/curlx/snprintf.hdeps/curl/lib/curlx/strcopy.cdeps/curl/lib/curlx/strdup.cdeps/curl/lib/curlx/strdup.hdeps/curl/lib/curlx/strerr.cdeps/curl/lib/curlx/strparse.cdeps/curl/lib/curlx/strparse.hdeps/curl/lib/curlx/timediff.cdeps/curl/lib/curlx/timediff.hdeps/curl/lib/curlx/timeval.cdeps/curl/lib/curlx/timeval.hdeps/curl/lib/curlx/version_win32.cdeps/curl/lib/curlx/version_win32.hdeps/curl/lib/curlx/wait.cdeps/curl/lib/curlx/wait.hdeps/curl/lib/curlx/warnless.cdeps/curl/lib/curlx/warnless.hdeps/curl/lib/curlx/winapi.cdeps/curl/lib/cw-out.cdeps/curl/lib/cw-pause.cdeps/curl/lib/dict.cdeps/curl/lib/dict.hdeps/curl/lib/doh.cdeps/curl/lib/doh.hdeps/curl/lib/dynhds.cdeps/curl/lib/dynhds.hdeps/curl/lib/easy.cdeps/curl/lib/easy_lock.hdeps/curl/lib/escape.cdeps/curl/lib/escape.hdeps/curl/lib/fake_addrinfo.cdeps/curl/lib/fake_addrinfo.hdeps/curl/lib/file.cdeps/curl/lib/file.hdeps/curl/lib/formdata.cdeps/curl/lib/formdata.hdeps/curl/lib/ftp.cdeps/curl/lib/ftp.hdeps/curl/lib/ftplistparser.hdeps/curl/lib/functypes.hdeps/curl/lib/getinfo.cdeps/curl/lib/gopher.cdeps/curl/lib/gopher.hdeps/curl/lib/hash.cdeps/curl/lib/hash.hdeps/curl/lib/headers.cdeps/curl/lib/headers.hdeps/curl/lib/hmac.cdeps/curl/lib/hostip.cdeps/curl/lib/hostip.hdeps/curl/lib/hostip4.cdeps/curl/lib/hostip6.cdeps/curl/lib/hsts.cdeps/curl/lib/hsts.hdeps/curl/lib/http.cdeps/curl/lib/http.hdeps/curl/lib/http1.cdeps/curl/lib/http1.hdeps/curl/lib/http2.cdeps/curl/lib/http2.hdeps/curl/lib/http_aws_sigv4.cdeps/curl/lib/http_chunks.cdeps/curl/lib/http_digest.cdeps/curl/lib/http_negotiate.cdeps/curl/lib/http_proxy.cdeps/curl/lib/httpsrr.cdeps/curl/lib/idn.cdeps/curl/lib/idn.hdeps/curl/lib/if2ip.cdeps/curl/lib/imap.cdeps/curl/lib/imap.hdeps/curl/lib/ldap.cdeps/curl/lib/libcurl.rcdeps/curl/lib/llist.hdeps/curl/lib/md4.cdeps/curl/lib/md5.cdeps/curl/lib/memdebug.cdeps/curl/lib/mime.cdeps/curl/lib/mime.hdeps/curl/lib/mprintf.cdeps/curl/lib/mqtt.cdeps/curl/lib/mqtt.hdeps/curl/lib/multi.cdeps/curl/lib/multi_ev.cdeps/curl/lib/multi_ntfy.cdeps/curl/lib/multihandle.hdeps/curl/lib/multiif.hdeps/curl/lib/netrc.cdeps/curl/lib/netrc.hdeps/curl/lib/noproxy.cdeps/curl/lib/openldap.cdeps/curl/lib/parsedate.cdeps/curl/lib/parsedate.hdeps/curl/lib/pingpong.cdeps/curl/lib/pingpong.hdeps/curl/lib/pop3.cdeps/curl/lib/pop3.hdeps/curl/lib/progress.cdeps/curl/lib/rand.cdeps/curl/lib/rand.hdeps/curl/lib/ratelimit.cdeps/curl/lib/ratelimit.hdeps/curl/lib/request.cdeps/curl/lib/request.hdeps/curl/lib/rtsp.cdeps/curl/lib/rtsp.hdeps/curl/lib/select.cdeps/curl/lib/select.hdeps/curl/lib/sendf.cdeps/curl/lib/sendf.hdeps/curl/lib/setopt.cdeps/curl/lib/setopt.hdeps/curl/lib/setup-os400.hdeps/curl/lib/setup-vms.hdeps/curl/lib/setup-win32.hdeps/curl/lib/sha256.cdeps/curl/lib/sigpipe.hdeps/curl/lib/slist.cdeps/curl/lib/smb.cdeps/curl/lib/smb.hdeps/curl/lib/smtp.cdeps/curl/lib/smtp.hdeps/curl/lib/socketpair.cdeps/curl/lib/socketpair.hdeps/curl/lib/socks.cdeps/curl/lib/socks_gssapi.cdeps/curl/lib/socks_sspi.cdeps/curl/lib/splay.cdeps/curl/lib/splay.hdeps/curl/lib/strcase.cdeps/curl/lib/strcase.hdeps/curl/lib/strequal.cdeps/curl/lib/strerror.cdeps/curl/lib/system_win32.cdeps/curl/lib/system_win32.hdeps/curl/lib/telnet.cdeps/curl/lib/telnet.hdeps/curl/lib/tftp.cdeps/curl/lib/tftp.hdeps/curl/lib/transfer.cdeps/curl/lib/uint-hash.cdeps/curl/lib/uint-table.hdeps/curl/lib/url.cdeps/curl/lib/url.hdeps/curl/lib/urlapi-int.hdeps/curl/lib/urlapi.cdeps/curl/lib/urldata.hdeps/curl/lib/vauth/cleartext.cdeps/curl/lib/vauth/cram.cdeps/curl/lib/vauth/digest.cdeps/curl/lib/vauth/digest.hdeps/curl/lib/vauth/digest_sspi.cdeps/curl/lib/vauth/gsasl.cdeps/curl/lib/vauth/krb5_gssapi.cdeps/curl/lib/vauth/krb5_sspi.cdeps/curl/lib/vauth/ntlm.cdeps/curl/lib/vauth/ntlm_sspi.cdeps/curl/lib/vauth/oauth2.cdeps/curl/lib/vauth/spnego_gssapi.cdeps/curl/lib/vauth/spnego_sspi.cdeps/curl/lib/vauth/vauth.cdeps/curl/lib/vauth/vauth.hdeps/curl/lib/version.cdeps/curl/lib/vquic/curl_ngtcp2.cdeps/curl/lib/vquic/curl_ngtcp2.hdeps/curl/lib/vquic/curl_osslq.cdeps/curl/lib/vquic/curl_quiche.cdeps/curl/lib/vquic/curl_quiche.hdeps/curl/lib/vquic/vquic-tls.cdeps/curl/lib/vquic/vquic-tls.hdeps/curl/lib/vquic/vquic.cdeps/curl/lib/vquic/vquic.hdeps/curl/lib/vquic/vquic_int.hdeps/curl/lib/vssh/libssh.cdeps/curl/lib/vssh/libssh2.cdeps/curl/lib/vssh/ssh.hdeps/curl/lib/vssh/vssh.cdeps/curl/lib/vssh/vssh.hdeps/curl/lib/vtls/apple.cdeps/curl/lib/vtls/apple.hdeps/curl/lib/vtls/cipher_suite.cdeps/curl/lib/vtls/cipher_suite.hdeps/curl/lib/vtls/gtls.cdeps/curl/lib/vtls/gtls.hdeps/curl/lib/vtls/hostcheck.cdeps/curl/lib/vtls/hostcheck.hdeps/curl/lib/vtls/keylog.cdeps/curl/lib/vtls/keylog.hdeps/curl/lib/vtls/mbedtls.cdeps/curl/lib/vtls/mbedtls.hdeps/curl/lib/vtls/openssl.cdeps/curl/lib/vtls/openssl.hdeps/curl/lib/vtls/rustls.cdeps/curl/lib/vtls/rustls.hdeps/curl/lib/vtls/schannel.cdeps/curl/lib/vtls/schannel.hdeps/curl/lib/vtls/schannel_int.hdeps/curl/lib/vtls/schannel_verify.cdeps/curl/lib/vtls/vtls.cdeps/curl/lib/vtls/vtls.hdeps/curl/lib/vtls/vtls_int.hdeps/curl/lib/vtls/vtls_scache.c
💤 Files with no reviewable changes (6)
- deps/curl/lib/cf-haproxy.c
- deps/curl/lib/curl_share.c
- deps/curl/lib/config-plan9.h
- deps/curl/lib/dynhds.c
- deps/curl/lib/curl_setup_once.h
- deps/curl/lib/config-riscos.h
Comment on lines
+312
to
+323
| CURLcode Curl_altsvc_ctrl(struct Curl_easy *data, const long ctrl) | ||
| { | ||
| DEBUGASSERT(asi); | ||
| asi->flags = ctrl; | ||
| DEBUGASSERT(data); | ||
| if(!ctrl) | ||
| return CURLE_BAD_FUNCTION_ARGUMENT; | ||
|
|
||
| if(!data->asi) { | ||
| data->asi = Curl_altsvc_init(); | ||
| if(!data->asi) | ||
| return CURLE_OUT_OF_MEMORY; | ||
| } | ||
| data->asi->flags = ctrl; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
rg -n -C2 '\bCURLOPT_ALTSVC_CTRL\b|\bCurl_altsvc_ctrl\b|CURLALTSVC_'Repository: nodesource/nsolid
Length of output: 3907
🏁 Script executed:
# Search for documentation, tests, and usage patterns of CURLOPT_ALTSVC_CTRL
rg -i 'altsvc|alt-svc' --type md --type txt -A 3 -B 1 | head -100Repository: nodesource/nsolid
Length of output: 5953
🏁 Script executed:
# Look for test files that might test CURLOPT_ALTSVC_CTRL
fd -e c -e h -e txt test | xargs rg -l 'ALTSVC_CTRL' 2>/dev/null || echo "No test files found with ALTSVC_CTRL"Repository: nodesource/nsolid
Length of output: 97
🏁 Script executed:
# Check if there are any comments or documentation in the curl headers explaining the behavior
rg -B5 -A5 'CURLOPT_ALTSVC_CTRL' deps/curl/include/Repository: nodesource/nsolid
Length of output: 1477
🏁 Script executed:
# Search for any curl documentation files mentioning altsvc or reset behavior
find deps/curl -type f \( -name "*.md" -o -name "*.txt" -o -name "*.rst" \) | xargs rg -l -i 'altsvc' 2>/dev/null | head -5Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
# Look at how other control options handle 0
rg -B5 -A8 'case CURLOPT_' deps/curl/lib/setopt.c | grep -A8 'BITMASK\|_CTRL' | head -60Repository: nodesource/nsolid
Length of output: 535
🏁 Script executed:
# Check if there are other similar control setters and how they handle zero
ast-grep --pattern 'if(!$VAR)
return CURLE_BAD_FUNCTION_ARGUMENT;' deps/curl/lib/Repository: nodesource/nsolid
Length of output: 5241
🏁 Script executed:
# Look at the full context of the altsvc.c file to see if there are any comments about expected usage
sed -n '300,330p' deps/curl/lib/altsvc.cRepository: nodesource/nsolid
Length of output: 701
🏁 Script executed:
# Search curl's official documentation or CHANGELOG in the repo
find deps/curl -type f -name "CHANGELOG*" -o -name "NEWS*" | xargs grep -i altsvc 2>/dev/null | head -20Repository: nodesource/nsolid
Length of output: 43
Reconsider the zero-mask check to provide a disable/reset path.
The current code rejects ctrl == 0 with CURLE_BAD_FUNCTION_ARGUMENT, which prevents resetting data->asi->flags once initialized. This deviates from the pattern used by similar bitmask controls like CURLOPT_HSTS_CTRL, which checks for specific bits rather than rejecting zero outright. Either allow zero to reset the flags to a neutral state, or document explicitly why zero is invalid. Without a reset path, callers cannot disable alt-svc after enabling it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/altsvc.c` around lines 312 - 323, The function Curl_altsvc_ctrl
currently rejects ctrl == 0 which prevents resetting data->asi->flags; change
the input validation in Curl_altsvc_ctrl so zero is accepted as a valid
"reset/disable" value and is assigned to data->asi->flags, and instead validate
only against unknown bits (e.g., compute a ALTSVC_CTRL_VALID mask and return
CURLE_BAD_FUNCTION_ARGUMENT if (ctrl & ~ALTSVC_CTRL_VALID) != 0); ensure
data->asi is still allocated as before and that data->asi->flags = ctrl handles
the zero case.
Comment on lines
329
to
+335
| data->state.async.dns = | ||
| Curl_dnscache_mk_entry(data, ares->temp_ai, | ||
| Curl_dnscache_mk_entry(data, &ares->temp_ai, | ||
| data->state.async.hostname, 0, | ||
| data->state.async.port, FALSE); | ||
| if(data->state.async.dns) | ||
| ares->temp_ai = NULL; /* temp_ai now owned by entry */ | ||
| if(!data->state.async.dns) { | ||
| result = CURLE_OUT_OF_MEMORY; | ||
| goto out; |
There was a problem hiding this comment.
Keep the OOM exit on the shared cleanup path.
Line 335 jumps past the async_ares_cleanup(data) call at Line 363. If HTTPS RR state was already accumulated, ares->hinfo stays live until a later lookup overwrites it, leaking the nested allocations on this failure path.
Suggested fix
data->state.async.dns =
Curl_dnscache_mk_entry(data, &ares->temp_ai,
data->state.async.hostname, 0,
data->state.async.port, FALSE);
if(!data->state.async.dns) {
result = CURLE_OUT_OF_MEMORY;
+ async_ares_cleanup(data);
goto out;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data->state.async.dns = | |
| Curl_dnscache_mk_entry(data, ares->temp_ai, | |
| Curl_dnscache_mk_entry(data, &ares->temp_ai, | |
| data->state.async.hostname, 0, | |
| data->state.async.port, FALSE); | |
| if(data->state.async.dns) | |
| ares->temp_ai = NULL; /* temp_ai now owned by entry */ | |
| if(!data->state.async.dns) { | |
| result = CURLE_OUT_OF_MEMORY; | |
| goto out; | |
| data->state.async.dns = | |
| Curl_dnscache_mk_entry(data, &ares->temp_ai, | |
| data->state.async.hostname, 0, | |
| data->state.async.port, FALSE); | |
| if(!data->state.async.dns) { | |
| result = CURLE_OUT_OF_MEMORY; | |
| async_ares_cleanup(data); | |
| goto out; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/asyn-ares.c` around lines 329 - 335, The OOM path after
Curl_dnscache_mk_entry currently jumps to the early "out" label and skips
async_ares_cleanup, leaking ares->hinfo and nested allocations; change the
control flow so that when Curl_dnscache_mk_entry returns NULL you set result =
CURLE_OUT_OF_MEMORY and jump to the common cleanup that runs async_ares_cleanup
(or call async_ares_cleanup before jumping out) instead of jumping directly to
out. Locate the block around Curl_dnscache_mk_entry and replace the direct goto
out with a goto or call that ensures async_ares_cleanup(data) runs (referencing
Curl_dnscache_mk_entry, async_ares_cleanup, and ares->hinfo).
Comment on lines
+490
to
493
| VERBOSE(i = 0); | ||
| for(a = bs->running; a; a = a->next) { | ||
| CURL_TRC_CF(data, cf, "baller %d: result=%d", i, a->result); | ||
| if(a->result) |
There was a problem hiding this comment.
Increment the attempt index in this final trace dump.
Line 492 will log every entry as baller 0 because i is initialized once and never advanced in this loop.
🔧 Suggested fix
- VERBOSE(i = 0);
+ VERBOSE(i = -1);
for(a = bs->running; a; a = a->next) {
+ VERBOSE(++i);
CURL_TRC_CF(data, cf, "baller %d: result=%d", i, a->result);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| VERBOSE(i = 0); | |
| for(a = bs->running; a; a = a->next) { | |
| CURL_TRC_CF(data, cf, "baller %d: result=%d", i, a->result); | |
| if(a->result) | |
| VERBOSE(i = -1); | |
| for(a = bs->running; a; a = a->next) { | |
| VERBOSE(++i); | |
| CURL_TRC_CF(data, cf, "baller %d: result=%d", i, a->result); | |
| if(a->result) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/cf-ip-happy.c` around lines 490 - 493, The final trace loop
uses a local index i (set by VERBOSE(i = 0)) but never increments it, so every
trace prints "baller 0"; inside the loop that iterates for(a = bs->running; a; a
= a->next) in cf-ip-happy.c, increment i each iteration (e.g., i++ at the end of
the loop body) before or after calling CURL_TRC_CF so the trace message shows
the correct attempt index for each entry.
Comment on lines
+315
to
+317
| #ifdef SOCK_CLOEXEC | ||
| addr->socktype |= SOCK_CLOEXEC; | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full socket_open function context around lines 315-317
cat -n deps/curl/lib/cf-socket.c | sed -n '300,380p'Repository: nodesource/nsolid
Length of output: 3474
🏁 Script executed:
# Also search for where fopensocket callback is invoked
rg -n "fopensocket" deps/curl/lib/cf-socket.c -A 5 -B 2Repository: nodesource/nsolid
Length of output: 1964
🏁 Script executed:
# Check for SOCK_NONBLOCK handling to verify the claim about it avoiding internal flags
rg -n "SOCK_NONBLOCK" deps/curl/lib/cf-socket.c -B 3 -A 3Repository: nodesource/nsolid
Length of output: 1473
Keep SOCK_CLOEXEC out of the public opensocket callback input.
Line 315 mutates addr->socktype before the fopensocket callback at line 332 sees it. This changes the callback-visible value from plain SOCK_STREAM/SOCK_DGRAM to a flag-augmented one on SOCK_CLOEXEC platforms, breaking existing callbacks that switch or compare on address->socktype. The nearby SOCK_NONBLOCK handling (lines 1072–1073) explicitly avoids this pattern with if(!data->set.fopensocket) before setting the flag.
Suggested fix
static CURLcode socket_open(struct Curl_easy *data,
struct Curl_sockaddr_ex *addr,
curl_socket_t *sockfd)
{
char errbuf[STRERROR_LEN];
+ int socktype = addr->socktype;
-
-#ifdef SOCK_CLOEXEC
- addr->socktype |= SOCK_CLOEXEC;
-#endif
DEBUGASSERT(data);
DEBUGASSERT(data->conn);
if(data->set.fopensocket) {
@@
}
else {
/* opensocket callback not set, so create the socket now */
+#ifdef SOCK_CLOEXEC
+ socktype |= SOCK_CLOEXEC;
+#endif
`#ifdef` DEBUGBUILD
if((addr->family == AF_INET6) && getenv("CURL_DBG_SOCK_FAIL_IPV6")) {
failf(data, "CURL_DBG_SOCK_FAIL_IPV6: failed to open socket");
return CURLE_COULDNT_CONNECT;
}
`#endif`
- *sockfd = CURL_SOCKET(addr->family, addr->socktype, addr->protocol);
+ *sockfd = CURL_SOCKET(addr->family, socktype, addr->protocol);
if((*sockfd == CURL_SOCKET_BAD) && (SOCKERRNO == SOCKENOMEM))
return CURLE_OUT_OF_MEMORY;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #ifdef SOCK_CLOEXEC | |
| addr->socktype |= SOCK_CLOEXEC; | |
| #endif | |
| static CURLcode socket_open(struct Curl_easy *data, | |
| struct Curl_sockaddr_ex *addr, | |
| curl_socket_t *sockfd) | |
| { | |
| char errbuf[STRERROR_LEN]; | |
| int socktype = addr->socktype; | |
| DEBUGASSERT(data); | |
| DEBUGASSERT(data->conn); | |
| if(data->set.fopensocket) { | |
| /* opensocket callback provided */ | |
| *sockfd = data->set.fopensocket(data->set.opensocket_client, | |
| (struct sockaddr *) addr, sockfd); | |
| } | |
| else { | |
| /* opensocket callback not set, so create the socket now */ | |
| `#ifdef` SOCK_CLOEXEC | |
| socktype |= SOCK_CLOEXEC; | |
| `#endif` | |
| `#ifdef` DEBUGBUILD | |
| if((addr->family == AF_INET6) && getenv("CURL_DBG_SOCK_FAIL_IPV6")) { | |
| failf(data, "CURL_DBG_SOCK_FAIL_IPV6: failed to open socket"); | |
| return CURLE_COULDNT_CONNECT; | |
| } | |
| `#endif` | |
| *sockfd = CURL_SOCKET(addr->family, socktype, addr->protocol); | |
| if((*sockfd == CURL_SOCKET_BAD) && (SOCKERRNO == SOCKENOMEM)) | |
| return CURLE_OUT_OF_MEMORY; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/cf-socket.c` around lines 315 - 317, The code mutates
addr->socktype to add SOCK_CLOEXEC before invoking the user fopen socket
callback, which exposes a flag-augmented value to that callback; change this so
the public fopen socket callback (data->set.fopensocket / the fopensocket
invocation at the call site) sees the original plain socket type (e.g.,
SOCK_STREAM/SOCK_DGRAM). Either apply SOCK_CLOEXEC only after checking if
data->set.fopensocket is set (like the SOCK_NONBLOCK pattern) or pass a local
copy of addr->socktype to the callback and set the CLOEXEC flag on the actual
addr->socktype only when creating the socket; ensure references to
addr->socktype and the fopensocket callback name are used to locate and adjust
the logic.
Comment on lines
+357
to
374
| #ifdef USE_SO_NOSIGPIPE | ||
| if(Curl_sock_nosigpipe(*sockfd) < 0) { | ||
| failf(data, "setsockopt enable SO_NOSIGPIPE: %s", | ||
| curlx_strerror(SOCKERRNO, errbuf, sizeof(errbuf))); | ||
| sclose(*sockfd); | ||
| *sockfd = CURL_SOCKET_BAD; | ||
| return CURLE_COULDNT_CONNECT; | ||
| } | ||
| #endif /* USE_SO_NOSIGPIPE */ | ||
|
|
||
| #if defined(HAVE_FCNTL) && !defined(SOCK_CLOEXEC) | ||
| if(fcntl(*sockfd, F_SETFD, FD_CLOEXEC) < 0) { | ||
| failf(data, "fcntl set CLOEXEC: %s", | ||
| curlx_strerror(SOCKERRNO, errbuf, sizeof(errbuf))); | ||
| close(*sockfd); | ||
| sclose(*sockfd); | ||
| *sockfd = CURL_SOCKET_BAD; | ||
| return CURLE_COULDNT_CONNECT; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n deps/curl/lib/cf-socket.c | sed -n '340,390p'Repository: nodesource/nsolid
Length of output: 2070
🏁 Script executed:
cat -n deps/curl/lib/cf-socket.c | sed -n '300,400p'Repository: nodesource/nsolid
Length of output: 4162
🏁 Script executed:
rg "fopensocket" deps/curl/lib/cf-socket.c -A 3 -B 3Repository: nodesource/nsolid
Length of output: 1657
🏁 Script executed:
rg "fclosesocket" deps/curl/lib/cf-socket.c -A 2 -B 2Repository: nodesource/nsolid
Length of output: 330
🏁 Script executed:
wc -l deps/curl/lib/cf-socket.cRepository: nodesource/nsolid
Length of output: 91
🏁 Script executed:
rg "^[a-zA-Z_].*\(.*\)\s*$" deps/curl/lib/cf-socket.c --before-context 0 | head -20Repository: nodesource/nsolid
Length of output: 588
🏁 Script executed:
cat -n deps/curl/lib/cf-socket.c | sed -n '320,380p'Repository: nodesource/nsolid
Length of output: 2829
🏁 Script executed:
cat -n deps/curl/lib/cf-socket.c | sed -n '250,375p' | grep -E "^[[:space:]]*[0-9]+\s+[a-zA-Z_].*\{|^[[:space:]]*[0-9]+\s+static|^[[:space:]]*[0-9]+\s+sclose|^[[:space:]]*[0-9]+\s+fclosesocket"Repository: nodesource/nsolid
Length of output: 554
🏁 Script executed:
python3 << 'EOF'
with open('deps/curl/lib/cf-socket.c', 'r') as f:
lines = f.readlines()
# Print lines 250-380 with line numbers
for i in range(249, min(380, len(lines))):
print(f"{i+1:4d}: {lines[i]}", end='')
EOFRepository: nodesource/nsolid
Length of output: 5271
🏁 Script executed:
rg "sclose" deps/curl/lib/cf-socket.c -B 3 -A 1 | head -60Repository: nodesource/nsolid
Length of output: 583
🏁 Script executed:
rg "define fclosesocket|fclosesocket\s*\(" deps/curl/lib/ -B 2 -A 2Repository: nodesource/nsolid
Length of output: 382
🏁 Script executed:
rg "fclosesocket|sclose" deps/curl/include/ -B 1 -A 1Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
rg "fclosesocket" --type c | head -20Repository: nodesource/nsolid
Length of output: 575
🏁 Script executed:
fd "sockrc\|socket" deps/curl/lib/ -type fRepository: nodesource/nsolid
Length of output: 231
🏁 Script executed:
rg "define.*fclosesocket|fclosesocket.*=" deps/curl/ --type c --type hRepository: nodesource/nsolid
Length of output: 214
🏁 Script executed:
rg -l "fclosesocket" deps/curl/ | xargs rg "sclose" -l | head -5Repository: nodesource/nsolid
Length of output: 86
🏁 Script executed:
rg "fclosesocket" --type c --type hRepository: nodesource/nsolid
Length of output: 575
🏁 Script executed:
sed -n '350,380p' deps/curl/lib/cf-socket.c | cat -nRepository: nodesource/nsolid
Length of output: 1333
🏁 Script executed:
sed -n '300,420p' deps/curl/lib/cf-socket.c | cat -nRepository: nodesource/nsolid
Length of output: 4769
🏁 Script executed:
rg "socket_open|Curl_socket_open" deps/curl/lib/cf-socket.c -B 5 -A 20 | head -100Repository: nodesource/nsolid
Length of output: 3152
🏁 Script executed:
rg "sclose\(" deps/curl/lib/cf-socket.c -B 5 -A 0 | head -80Repository: nodesource/nsolid
Length of output: 608
🏁 Script executed:
ast-grep --pattern 'static $_ $func($_) {
$$$
}' --lang c deps/curl/lib/cf-socket.c | head -50Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
head -n 380 deps/curl/lib/cf-socket.c | tail -n 80Repository: nodesource/nsolid
Length of output: 2883
🏁 Script executed:
grep -n "^[a-zA-Z_]" deps/curl/lib/cf-socket.c | awk -F: '$1 < 360 {line=$1; name=$0} END {print line, name}'Repository: nodesource/nsolid
Length of output: 120
🏁 Script executed:
awk 'NR>=250 && NR<=380' deps/curl/lib/cf-socket.cRepository: nodesource/nsolid
Length of output: 4485
🏁 Script executed:
rg "socket\(" deps/curl/lib/cf-socket.c -B 2 -A 5 | head -80Repository: nodesource/nsolid
Length of output: 1360
🏁 Script executed:
find deps/curl -name "*.h" -o -name "*.c" | xargs grep -l "fclosesocket" 2>/dev/nullRepository: nodesource/nsolid
Length of output: 153
🏁 Script executed:
rg "fopensocket|fclosesocket" deps/curl/include/curl.h -B 2 -A 2Repository: nodesource/nsolid
Length of output: 125
🏁 Script executed:
head -c 15000 deps/curl/lib/cf-socket.c | tail -c 2000 | cat -nRepository: nodesource/nsolid
Length of output: 2607
🏁 Script executed:
python3 << 'PYEOF'
with open('deps/curl/lib/cf-socket.c', 'r') as f:
lines = f.readlines()
# Print lines 280-390
for i in range(279, min(390, len(lines))):
print(f"{i+1:4d}: {lines[i]}", end='')
PYEOFRepository: nodesource/nsolid
Length of output: 4299
🏁 Script executed:
rg "fopensocket" deps/curl/ -B 2 -A 2 | head -100Repository: nodesource/nsolid
Length of output: 3104
🏁 Script executed:
rg "fclosesocket" deps/curl/ -B 2 -A 2 | head -100Repository: nodesource/nsolid
Length of output: 1826
🏁 Script executed:
rg "typedef.*socket\|callback.*socket\|socket.*callback" deps/curl/include/curl.h -B 1 -A 3Repository: nodesource/nsolid
Length of output: 125
🏁 Script executed:
sed -n '355,365p' deps/curl/lib/cf-socket.c | nlRepository: nodesource/nsolid
Length of output: 103
🏁 Script executed:
rg "USE_SO_NOSIGPIPE" deps/curl/lib/cf-socket.c -B 10 -A 15Repository: nodesource/nsolid
Length of output: 2004
🏁 Script executed:
rg "USE_SO_NOSIGPIPE" deps/curl/lib/cf-socket.c -B 50 | head -80Repository: nodesource/nsolid
Length of output: 2691
🏁 Script executed:
rg "Curl_socket\|socket\(" deps/curl/lib/cf-socket.c -B 3 -A 3 | head -80Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
rg "fclosesocket\|sclose" deps/curl/lib/ --type c | grep -v "^Binary" | head -40Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
rg "^#define\s+fclosesocket|^fclosesocket" deps/curl/lib/ -B 1 -A 3Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
find deps/curl/lib -name "*.h" -o -name "*.c" | xargs grep -n "fclosesocket\|socket.*callback" | head -40Repository: nodesource/nsolid
Length of output: 2985
🏁 Script executed:
ls -la deps/curl/lib/*.hRepository: nodesource/nsolid
Length of output: 9755
🏁 Script executed:
rg "fclosesocket" deps/curl/lib/url.h -B 3 -A 3Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
rg "fopensocket" deps/curl/lib/ -B 5 -A 5 | head -100Repository: nodesource/nsolid
Length of output: 6165
🏁 Script executed:
cat deps/curl/lib/url.h | grep -n "fclosesocket\|FCLOSESOCKET" -i | head -20Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
rg "closesocket|CLOSESOCKET" deps/curl/lib/ --type h -B 2 -A 2 | head -50Repository: nodesource/nsolid
Length of output: 2403
🏁 Script executed:
wc -l deps/curl/lib/url.hRepository: nodesource/nsolid
Length of output: 84
🏁 Script executed:
rg "callback|fopen|fclose" deps/curl/lib/url.h -i -B 1 -A 1 | head -80Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
rg "CURLOPTTYPE_FUNCTIONPOINT.*socket|fopensocket|fclosesocket" deps/curl/include/curl.h -B 1 -A 1Repository: nodesource/nsolid
Length of output: 125
🏁 Script executed:
rg "typedef.*fclosesocket|define.*fclosesocket|fclosesocket.*=" deps/curl/ --type c --type hRepository: nodesource/nsolid
Length of output: 214
🏁 Script executed:
rg "fclosesocket" --type c --type hRepository: nodesource/nsolid
Length of output: 575
🏁 Script executed:
rg "fopensocket" --type c --type h -B 5 -A 5Repository: nodesource/nsolid
Length of output: 6165
🏁 Script executed:
rg "define.*sclose|^sclose\s*\(" deps/curl/ --type c --type h -B 2 -A 2Repository: nodesource/nsolid
Length of output: 1045
🏁 Script executed:
rg "sclose" deps/curl/lib/cf-socket.c -B 5 -A 0Repository: nodesource/nsolid
Length of output: 608
🏁 Script executed:
rg "Curl_socket_close|socket_close|Curl_close" deps/curl/lib/cf-socket.c -B 2 -A 3Repository: nodesource/nsolid
Length of output: 2612
🏁 Script executed:
rg "close.*socket|socket.*close" deps/curl/lib/ --type c | head -30Repository: nodesource/nsolid
Length of output: 2504
🏁 Script executed:
rg "curl_easy_cleanup|easy_cleanup" deps/curl/lib/ --type c -A 20 | head -80Repository: nodesource/nsolid
Length of output: 4840
🏁 Script executed:
rg "socket_close|SocketClose|SOCKET_CLOSE" deps/curl/lib/ -B 3 -A 10 | head -100Repository: nodesource/nsolid
Length of output: 5290
🏁 Script executed:
rg "Curl_socket\s*\(" deps/curl/lib/ --type c -B 3 -A 15 | head -100Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
rg "static.*close|^close_" deps/curl/lib/cf-socket.c -B 2 -A 20 | head -150Repository: nodesource/nsolid
Length of output: 1411
🏁 Script executed:
rg "struct.*conn|Curl_conn|conn_close" deps/curl/lib/cf-socket.c -B 1 -A 3 | head -50Repository: nodesource/nsolid
Length of output: 2023
🏁 Script executed:
grep -n "^[a-zA-Z_].*{" deps/curl/lib/cf-socket.c | head -20Repository: nodesource/nsolid
Length of output: 259
🏁 Script executed:
python3 << 'PYEOF'
with open('deps/curl/lib/cf-socket.c', 'r') as f:
content = f.read()
lines = content.split('\n')
# Find line numbers with USE_SO_NOSIGPIPE
for i, line in enumerate(lines):
if 'USE_SO_NOSIGPIPE' in line:
# Print context around it
start = max(0, i - 40)
end = min(len(lines), i + 25)
for j in range(start, end):
print(f"{j+1:4d}: {lines[j]}")
break
PYEOFRepository: nodesource/nsolid
Length of output: 2460
🏁 Script executed:
rg "socket_conn_close|sock_close|conn->sockfd|socket.*close" deps/curl/lib/cf-socket.c -B 3 -A 3 | head -100Repository: nodesource/nsolid
Length of output: 2613
🏁 Script executed:
grep -n "^[a-zA-Z_].*Curl_" deps/curl/lib/cf-socket.c | head -20Repository: nodesource/nsolid
Length of output: 1357
🏁 Script executed:
python3 << 'PYEOF'
with open('deps/curl/lib/cf-socket.c', 'r') as f:
lines = f.readlines()
# Find lines containing "close"
for i, line in enumerate(lines):
if 'close' in line.lower() and ('static' in line or 'cf_socket' in line):
print(f"{i+1:4d}: {line.rstrip()}")
PYEOFRepository: nodesource/nsolid
Length of output: 440
🏁 Script executed:
rg "socket_conn_close" deps/curl/lib/cf-socket.c -B 5 -A 20Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
grep -n "^[a-zA-Z_].*close" deps/curl/lib/cf-socket.cRepository: nodesource/nsolid
Length of output: 295
🏁 Script executed:
wc -l deps/curl/lib/cf-socket.c
# It's likely small enough to read entirely
cat deps/curl/lib/cf-socket.c | wc -lRepository: nodesource/nsolid
Length of output: 96
🏁 Script executed:
cat deps/curl/lib/cf-socket.cRepository: nodesource/nsolid
Length of output: 50373
Use the callback-aware close path on socket-setup failures.
Lines 361 and 371 use sclose() directly to close the socket on setup failures, even if it was created via the fopensocket callback. This bypasses the fclosesocket callback that may be registered for proper cleanup. Use socket_close(data, data->conn, TRUE, *sockfd) instead to invoke callback-based socket closing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/cf-socket.c` around lines 357 - 374, Replace the direct
sclose(*sockfd) calls in the socket setup failure paths (inside the
Curl_sock_nosigpipe and fcntl error blocks) with the callback-aware close helper
socket_close(data, data->conn, TRUE, *sockfd) so any registered fclosesocket
callback gets invoked; after invoking socket_close set *sockfd = CURL_SOCKET_BAD
and return CURLE_COULDNT_CONNECT as before. Ensure you update both failure
branches where sclose is used (the block around Curl_sock_nosigpipe and the
block around fcntl(F_SETFD, FD_CLOEXEC)) and keep the existing failf messages
and error returns unchanged.
Comment on lines
+39
to
+42
| #ifdef CURLVERBOSE | ||
| int Curl_nghttp2_fr_print(const nghttp2_frame *frame, char *buffer, | ||
| size_t blen); | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
for path in Path('.').rglob('*.[ch]'):
try:
lines = path.read_text(errors='ignore').splitlines()
except Exception:
continue
includes = [line.strip() for line in lines if line.strip().startswith('#include')]
if any('"http2.h"' in inc for inc in includes):
pos = next(i for i, inc in enumerate(includes) if '"http2.h"' in inc)
if not any('nghttp2/nghttp2.h' in inc for inc in includes[:pos]):
print(path)
PYRepository: nodesource/nsolid
Length of output: 161
🏁 Script executed:
#!/bin/bash
# Check the current includes in http2.h around the region in question
echo "=== deps/curl/lib/http2.h (first 50 lines) ==="
head -n 50 deps/curl/lib/http2.h | cat -n
echo -e "\n=== Checking for nghttp2 include in http2.h ==="
grep -n "nghttp2" deps/curl/lib/http2.h || echo "Not found"
echo -e "\n=== deps/curl/lib/http.c - include section ==="
head -n 100 deps/curl/lib/http.c | grep -n "#include" | head -20Repository: nodesource/nsolid
Length of output: 3295
🏁 Script executed:
#!/bin/bash
# Check more includes in http.c to find where http2.h is included
echo "=== deps/curl/lib/http.c - all includes ==="
head -n 150 deps/curl/lib/http.c | grep -n "#include"
echo -e "\n=== Looking for nghttp2 and http2 in http.c ==="
head -n 150 deps/curl/lib/http.c | grep -n "nghttp2\|http2"Repository: nodesource/nsolid
Length of output: 1342
🏁 Script executed:
#!/bin/bash
# Check which files include nghttp2/nghttp2.h
echo "=== Checking nghttp2 includes in the flagged files ==="
for file in deps/curl/lib/cf-https-connect.c deps/curl/lib/curl_trc.c deps/curl/lib/http.c deps/curl/lib/multi.c; do
echo "--- $file ---"
grep -n "nghttp2/nghttp2.h" "$file" || echo "NOT FOUND"
done
echo -e "\n=== Check full include order in http.c ==="
head -n 90 deps/curl/lib/http.c | grep -n "#include"Repository: nodesource/nsolid
Length of output: 1499
🏁 Script executed:
#!/bin/bash
# Check if nghttp2/nghttp2.h is included indirectly through other headers
echo "=== Tracing includes in urldata.h ==="
grep -n "nghttp2" deps/curl/lib/urldata.h | head -20
echo -e "\n=== Check if http2.h forward-declares nghttp2_frame ==="
grep -A5 -B5 "nghttp2_frame" deps/curl/lib/http2.h | head -30
echo -e "\n=== Look for typedef/forward declaration in http2.h ==="
cat deps/curl/lib/http2.h | head -100Repository: nodesource/nsolid
Length of output: 3396
🏁 Script executed:
#!/bin/bash
# Trace where nghttp2/nghttp2.h is actually included from
echo "=== Check curl_setup.h ==="
grep -n "nghttp2" deps/curl/lib/curl_setup.h | head -10
echo -e "\n=== Check which header in the chain includes nghttp2 ==="
# Look in likely candidates
for header in curl_setup.h urldata.h curl_trc.h transfer.h; do
echo "--- $header ---"
grep -l "nghttp2/nghttp2.h" "deps/curl/lib/$header" 2>/dev/null || echo "not here"
done
echo -e "\n=== Simulate compile: what pulls in nghttp2? ==="
python3 <<'PYEOF'
import subprocess
import re
# Find all .h files and check which one includes nghttp2/nghttp2.h
result = subprocess.run(['find', 'deps/curl/lib', '-name', '*.h', '-type', 'f'],
capture_output=True, text=True)
headers = result.stdout.strip().split('\n')
found = []
for hdr in headers:
try:
with open(hdr, 'r', errors='ignore') as f:
content = f.read()
if '<nghttp2/nghttp2.h>' in content:
found.append(hdr)
except:
pass
if found:
print("Headers that include <nghttp2/nghttp2.h>:")
for h in found:
print(f" {h}")
else:
print("No header found with <nghttp2/nghttp2.h>")
PYEOFRepository: nodesource/nsolid
Length of output: 375
🏁 Script executed:
#!/bin/bash
# Verify the include chain: does dynhds.h include nghttp2?
echo "=== dynhds.h content (first 50 lines) ==="
head -n 50 deps/curl/lib/dynhds.h | cat -n
echo -e "\n=== Confirm dynhds.h includes nghttp2/nghttp2.h ==="
grep -n "nghttp2/nghttp2.h" deps/curl/lib/dynhds.h
echo -e "\n=== Verify include order in http.c around dynhds and http2 ==="
sed -n '68,78p' deps/curl/lib/http.c | cat -nRepository: nodesource/nsolid
Length of output: 2635
🏁 Script executed:
#!/bin/bash
# Verify that dynhds.h is indeed included before http2.h in all flagged files
echo "=== Check if dynhds.h comes before http2.h in the flagged files ==="
for file in deps/curl/lib/cf-https-connect.c deps/curl/lib/curl_trc.c deps/curl/lib/http.c deps/curl/lib/multi.c; do
echo "--- $file ---"
dynhds_line=$(grep -n '"dynhds.h"' "$file" 2>/dev/null | cut -d: -f1)
http2_line=$(grep -n '"http2.h"' "$file" 2>/dev/null | cut -d: -f1)
if [ -n "$dynhds_line" ] && [ -n "$http2_line" ]; then
if [ "$dynhds_line" -lt "$http2_line" ]; then
echo " dynhds.h @ line $dynhds_line comes BEFORE http2.h @ line $http2_line ✓"
else
echo " dynhds.h @ line $dynhds_line comes AFTER http2.h @ line $http2_line ✗"
fi
else
echo " dynhds.h found: $([ -n "$dynhds_line" ] && echo "yes" || echo "no"), http2.h found: $([ -n "$http2_line" ] && echo "yes" || echo "no")"
fi
doneRepository: nodesource/nsolid
Length of output: 438
🏁 Script executed:
#!/bin/bash
# Critical: Check if cf-https-connect.c, curl_trc.c, and multi.c actually use http2.h functions
echo "=== Check what these files do with http2.h ==="
for file in deps/curl/lib/cf-https-connect.c deps/curl/lib/curl_trc.c deps/curl/lib/multi.c; do
echo "--- $file ---"
if grep -q "Curl_nghttp2" "$file" 2>/dev/null; then
echo " ✗ USES nghttp2 functions"
grep -n "Curl_nghttp2" "$file" | head -5
else
echo " OK: Does not use nghttp2 functions directly"
fi
done
echo -e "\n=== Check if http2.h is under `#ifdef` CURLVERBOSE in these files ==="
for file in deps/curl/lib/cf-https-connect.c deps/curl/lib/curl_trc.c deps/curl/lib/multi.c; do
echo "--- $file ---"
# Check context around http2.h include
line_num=$(grep -n '"http2.h"' "$file" 2>/dev/null | cut -d: -f1)
if [ -n "$line_num" ]; then
start=$((line_num - 3))
end=$((line_num + 3))
sed -n "${start},${end}p" "$file" | cat -n
fi
doneRepository: nodesource/nsolid
Length of output: 1106
Make http2.h include <nghttp2/nghttp2.h> to declare nghttp2_frame.
The new Curl_nghttp2_fr_print declaration under CURLVERBOSE uses the nghttp2_frame typedef, but http2.h does not include the nghttp2 header. Three files that include http2.h (cf-https-connect.c, curl_trc.c, multi.c) do not pull in nghttp2/nghttp2.h through other headers, creating a compilation failure when CURLVERBOSE is enabled.
♻️ Recommended fix
`#ifdef` USE_NGHTTP2
+#include <nghttp2/nghttp2.h>
/* value for MAX_CONCURRENT_STREAMS we use until we get an updated setting
from the peer */
`#define` DEFAULT_MAX_CONCURRENT_STREAMS 100🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/http2.h` around lines 39 - 42, Add a direct include of
nghttp2's public header so the nghttp2_frame type is declared where
Curl_nghttp2_fr_print is declared: modify deps/curl/lib/http2.h to `#include`
<nghttp2/nghttp2.h> (preferably guarded by the existing macro that enables
nghttp2, e.g. surround the include and the Curl_nghttp2_fr_print prototype with
the same USE_NGHTTP2/CURL_DISABLE_HTTP2 conditional) so callers of http2.h (like
cf-https-connect.c, curl_trc.c, multi.c) get the nghttp2_frame typedef when
CURLVERBOSE is enabled.
Comment on lines
+219
to
+236
| curlx_dyn_init(&line, 2000); | ||
|
|
||
| if(!escape_only && curlx_dyn_addn(&line, "\"", 1)) | ||
| return NULL; | ||
|
|
||
| while(*str) { | ||
| if((*str == '\\' || *str == '"') && | ||
| curlx_dyn_addn(&line, "\\", 1)) | ||
| return NULL; | ||
| if(curlx_dyn_addn(&line, str, 1)) | ||
| return NULL; | ||
| str++; | ||
| } | ||
|
|
||
| if(!escape_only && curlx_dyn_addn(&line, "\"", 1)) | ||
| return NULL; | ||
|
|
||
| return curlx_dyn_ptr(&line); |
There was a problem hiding this comment.
Free the dynbuf on imap_atom() failure paths.
curlx_dyn_init(&line, 2000) hands ownership to this helper, but every later return NULL skips curlx_dyn_free(&line). A failed curlx_dyn_addn() will leak the partially built buffer.
Proposed fix
static char *imap_atom(const char *str, bool escape_only)
{
struct dynbuf line;
size_t nclean;
size_t len;
@@
curlx_dyn_init(&line, 2000);
- if(!escape_only && curlx_dyn_addn(&line, "\"", 1))
- return NULL;
+ if(!escape_only && curlx_dyn_addn(&line, "\"", 1))
+ goto fail;
while(*str) {
if((*str == '\\' || *str == '"') &&
- curlx_dyn_addn(&line, "\\", 1))
- return NULL;
- if(curlx_dyn_addn(&line, str, 1))
- return NULL;
+ curlx_dyn_addn(&line, "\\", 1))
+ goto fail;
+ if(curlx_dyn_addn(&line, str, 1))
+ goto fail;
str++;
}
- if(!escape_only && curlx_dyn_addn(&line, "\"", 1))
- return NULL;
+ if(!escape_only && curlx_dyn_addn(&line, "\"", 1))
+ goto fail;
return curlx_dyn_ptr(&line);
+fail:
+ curlx_dyn_free(&line);
+ return NULL;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| curlx_dyn_init(&line, 2000); | |
| if(!escape_only && curlx_dyn_addn(&line, "\"", 1)) | |
| return NULL; | |
| while(*str) { | |
| if((*str == '\\' || *str == '"') && | |
| curlx_dyn_addn(&line, "\\", 1)) | |
| return NULL; | |
| if(curlx_dyn_addn(&line, str, 1)) | |
| return NULL; | |
| str++; | |
| } | |
| if(!escape_only && curlx_dyn_addn(&line, "\"", 1)) | |
| return NULL; | |
| return curlx_dyn_ptr(&line); | |
| curlx_dyn_init(&line, 2000); | |
| if(!escape_only && curlx_dyn_addn(&line, "\"", 1)) | |
| goto fail; | |
| while(*str) { | |
| if((*str == '\\' || *str == '"') && | |
| curlx_dyn_addn(&line, "\\", 1)) | |
| goto fail; | |
| if(curlx_dyn_addn(&line, str, 1)) | |
| goto fail; | |
| str++; | |
| } | |
| if(!escape_only && curlx_dyn_addn(&line, "\"", 1)) | |
| goto fail; | |
| return curlx_dyn_ptr(&line); | |
| fail: | |
| curlx_dyn_free(&line); | |
| return NULL; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/imap.c` around lines 219 - 236, The dynbuf initialized by
curlx_dyn_init(&line, 2000) must be freed on all error paths in imap_atom():
when any curlx_dyn_addn() fails you should call curlx_dyn_free(&line) before
returning NULL (or refactor to a single cleanup label and jump there on error),
ensuring curlx_dyn_free(&line) is called for every early return and only return
curlx_dyn_ptr(&line) on success.
Comment on lines
+1179
to
+1214
| /* Detect IMAP listings vs. downloading a single email */ | ||
| static bool is_custom_fetch_listing_match(const char *params) | ||
| { | ||
| /* match " 1:* (FLAGS ..." or " 1,2,3 (FLAGS ..." */ | ||
| if(*params++ != ' ') | ||
| return FALSE; | ||
|
|
||
| while(ISDIGIT(*params)) { | ||
| params++; | ||
| if(*params == 0) | ||
| return FALSE; | ||
| } | ||
| if(*params == ':') | ||
| return true; | ||
| if(*params == ',') | ||
| return true; | ||
| return FALSE; | ||
| } | ||
|
|
||
| static bool is_custom_fetch_listing(struct IMAP *imap) | ||
| { | ||
| /* filter out "UID FETCH 1:* (FLAGS ..." queries to list emails */ | ||
| if(!imap->custom) | ||
| return FALSE; | ||
| else if(curl_strequal(imap->custom, "FETCH") && imap->custom_params) { | ||
| const char *p = imap->custom_params; | ||
| return is_custom_fetch_listing_match(p); | ||
| } | ||
| else if(curl_strequal(imap->custom, "UID") && imap->custom_params) { | ||
| if(curl_strnequal(imap->custom_params, " FETCH ", 7)) { | ||
| const char *p = imap->custom_params + 6; | ||
| return is_custom_fetch_listing_match(p); | ||
| } | ||
| } | ||
| return FALSE; | ||
| } |
There was a problem hiding this comment.
This matcher currently black-holes range/comma FETCH responses.
is_custom_fetch_listing() only looks at the message set, so commands like UID FETCH 1:* (FLAGS) and UID FETCH 1:* BODY[] both hit the empty branch in imap_state_listsearch_resp(). The state machine then consumes each untagged FETCH response and returns only the final tagged OK, so callers get neither the listing rows nor the body data.
Also applies to: 1232-1234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/imap.c` around lines 1179 - 1214, The matcher currently only
inspects the message-set and thus treats any FETCH with a range/comma as a
"listing", which causes BODY[] responses to be swallowed; update
is_custom_fetch_listing() (and its helper is_custom_fetch_listing_match()) to
not just recognize the message-set but also verify the fetch data-item token
that follows and only return true for listing-style queries (e.g. when the
data-item is FLAGS/ (FLAGS ...)), otherwise return FALSE so UID/FETCH ... BODY[]
responses are handled normally by imap_state_listsearch_resp().
Comment on lines
+1697
to
+1709
| static void imap_easy_reset(struct IMAP *imap) | ||
| { | ||
| Curl_safefree(imap->mailbox); | ||
| Curl_safefree(imap->uid); | ||
| Curl_safefree(imap->mindex); | ||
| Curl_safefree(imap->section); | ||
| Curl_safefree(imap->partial); | ||
| Curl_safefree(imap->query); | ||
| Curl_safefree(imap->custom); | ||
| Curl_safefree(imap->custom_params); | ||
| /* Clear the transfer mode for the next request */ | ||
| imap->transfer = PPTRANSFER_BODY; | ||
| } |
There was a problem hiding this comment.
Reset uidvalidity state between requests.
imap_done() calls imap_easy_reset() after every transfer. Leaving uidvalidity_set intact makes the next request on the same easy handle inherit the previous URL's UIDVALIDITY constraint, which can spuriously reselect or reject a mailbox.
Proposed fix
static void imap_easy_reset(struct IMAP *imap)
{
Curl_safefree(imap->mailbox);
Curl_safefree(imap->uid);
Curl_safefree(imap->mindex);
Curl_safefree(imap->section);
Curl_safefree(imap->partial);
Curl_safefree(imap->query);
Curl_safefree(imap->custom);
Curl_safefree(imap->custom_params);
+ imap->uidvalidity = 0;
+ imap->uidvalidity_set = FALSE;
/* Clear the transfer mode for the next request */
imap->transfer = PPTRANSFER_BODY;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/imap.c` around lines 1697 - 1709, The imap_easy_reset function
currently frees string fields but doesn't clear the UIDVALIDITY tracking, so
subsequent requests on the same easy handle inherit the previous request's
UIDVALIDITY constraint; update imap_easy_reset to reset the UIDVALIDITY state by
clearing imap->uidvalidity_set (and reset imap->uidvalidity as well) so the next
transfer starts without the previous UIDVALIDITY constraint, referencing the
imap_easy_reset function and the imap->uidvalidity_set/imap->uidvalidity fields.
Comment on lines
+558
to
564
| case MTYPE_LONGLONGU: | ||
| iptr->val.numu = va_arg(arglist, uint64_t); | ||
| break; | ||
|
|
||
| case FORMAT_LONGLONG: | ||
| iptr->val.nums = (mp_intmax_t)va_arg(arglist, mp_intmax_t); | ||
| case MTYPE_LONGLONG: | ||
| iptr->val.nums = va_arg(arglist, int64_t); | ||
| break; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and the lines in question
wc -l deps/curl/lib/mprintf.cRepository: nodesource/nsolid
Length of output: 89
🏁 Script executed:
# Examine lines 555-570 to see the MTYPE_LONGLONGU and MTYPE_LONGLONG cases
sed -n '555,570p' deps/curl/lib/mprintf.cRepository: nodesource/nsolid
Length of output: 411
🏁 Script executed:
# Also check the %lln handling around lines 1043-1047
sed -n '1040,1050p' deps/curl/lib/mprintf.cRepository: nodesource/nsolid
Length of output: 387
🏁 Script executed:
# Find all references to MTYPE_LONGLONG to understand the data structure
grep -n "MTYPE_LONGLONG" deps/curl/lib/mprintf.c | head -20Repository: nodesource/nsolid
Length of output: 412
🏁 Script executed:
# Look for the definition of the iptr->val structure
grep -B5 -A5 "val.nums\|val.numu" deps/curl/lib/mprintf.c | head -40Repository: nodesource/nsolid
Length of output: 948
🏁 Script executed:
# Check the union/struct definition for the format argument value holder
sed -n '1,150p' deps/curl/lib/mprintf.c | grep -A10 "union\|typedef"Repository: nodesource/nsolid
Length of output: 408
🏁 Script executed:
# Search for where MTYPE_LONGLONG is set - look for the format specifiers
sed -n '390,430p' deps/curl/lib/mprintf.c | cat -nRepository: nodesource/nsolid
Length of output: 1468
🏁 Script executed:
# Look more broadly at how the type is determined based on format specifiers
sed -n '370,440p' deps/curl/lib/mprintf.c | cat -nRepository: nodesource/nsolid
Length of output: 2311
🏁 Script executed:
# Check what format characters trigger MTYPE_LONGLONG
rg -n -B2 -A2 "MTYPE_LONGLONG|MTYPE_LONGLONGU" deps/curl/lib/mprintf.c | head -60Repository: nodesource/nsolid
Length of output: 1712
🏁 Script executed:
# Look for the '%lln' format handling to understand the output side
sed -n '1010,1030p' deps/curl/lib/mprintf.c | cat -nRepository: nodesource/nsolid
Length of output: 754
🏁 Script executed:
# Verify the actual format specifiers used - look for '%lld', '%llu', '%lln'
rg -n "lld|llu|lln" deps/curl/lib/mprintf.cRepository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
# Search for where MTYPE_INTPTR is set and how it interacts with FLAGS_LONGLONG
sed -n '380,440p' deps/curl/lib/mprintf.c | grep -B3 -A3 "MTYPE_INTPTR\|case 'n'"Repository: nodesource/nsolid
Length of output: 222
🏁 Script executed:
# Check how FLAGS_LONGLONG is set for the 'n' format
sed -n '280,380p' deps/curl/lib/mprintf.c | tail -50 | cat -nRepository: nodesource/nsolid
Length of output: 1636
🏁 Script executed:
# Look at the complete 'n' case handling and FLAGS parsing
sed -n '330,370p' deps/curl/lib/mprintf.c | cat -nRepository: nodesource/nsolid
Length of output: 1399
🏁 Script executed:
# Check if there's any typedef mapping int64_t to long long
grep -n "typedef.*int64_t\|int64_t.*=" deps/curl/lib/mprintf.c | head -10Repository: nodesource/nsolid
Length of output: 116
🏁 Script executed:
# Look for any platform-specific handling of int64_t
grep -rn "int64_t\|long long" deps/curl/include/ | grep -i "define\|typedef" | head -20Repository: nodesource/nsolid
Length of output: 1325
🏁 Script executed:
# Verify the actual flow: check what format specifiers trigger MTYPE_LONGLONG
# Look at a specific example - where does 'll' modifier get parsed?
sed -n '240,330p' deps/curl/lib/mprintf.c | cat -nRepository: nodesource/nsolid
Length of output: 3234
🏁 Script executed:
# Check the standard C headers that define int64_t
grep -rn "int64_t" deps/curl/include/ | grep -E "define|typedef" | head -20Repository: nodesource/nsolid
Length of output: 43
🏁 Script executed:
# Check if there's any `#include` of standard int types
head -50 deps/curl/lib/mprintf.c | grep -E "#include|int64_t"Repository: nodesource/nsolid
Length of output: 225
🏁 Script executed:
# Check curl_setup.h for int64_t and stdint.h
head -100 deps/curl/lib/curl_setup.h | grep -E "#include|int64_t"Repository: nodesource/nsolid
Length of output: 135
🏁 Script executed:
# Look for stdint.h or similar includes in curl_setup.h
grep -n "stdint\|inttypes\|int64_t" deps/curl/lib/curl_setup.h | head -20Repository: nodesource/nsolid
Length of output: 230
🏁 Script executed:
# Get a broader view of includes in curl_setup.h
sed -n '1,200p' deps/curl/lib/curl_setup.h | grep "#include"Repository: nodesource/nsolid
Length of output: 160
Use long long at the %ll varargs boundary.
Format specifiers %lld, %llu, and %lln require passing and receiving long long/unsigned long long per the C standard. On LP64 systems, int64_t is typically long (a different type), making va_arg(arglist, int64_t) undefined behavior even though both are 64-bit.
Lines 559, 563: Replace va_arg(arglist, int64_t) and va_arg(arglist, uint64_t) with va_arg(arglist, long long) and va_arg(arglist, unsigned long long), then cast the result to the union field's type.
Line 1046: When FLAGS_LONGLONG is set, cast the pointer to long long * instead of int64_t *.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/curl/lib/mprintf.c` around lines 558 - 564, The va_arg usage for %ll
boundaries is incorrect: in the switch cases MTYPE_LONGLONGU and MTYPE_LONGLONG
replace va_arg(arglist, uint64_t)/va_arg(arglist, int64_t) with va_arg(arglist,
unsigned long long)/va_arg(arglist, long long) and then cast the returned long
long/unsigned long long to the union fields iptr->val.numu and iptr->val.nums
respectively; additionally, where FLAGS_LONGLONG causes a pointer to be
stored/retrieved (the code that currently casts to int64_t *), change that cast
to long long * (or unsigned long long * as appropriate) before assigning/copying
so the varargs boundary uses long long types per the %ll specifiers.
|
Hey @santigimeno can you please make sure this PR is attached to a ticket? Currently it is not |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor