deps: update libcurl to 8.15.0 #323
Conversation
|
I ran into an unexpected issue while reviewing this PR. Please try again later. |
WalkthroughThis update reorganizes and refactors libcurl's source tree, especially around DNS resolver backends, string and buffer utilities, and protocol state management. It introduces new async resolver implementations, modularizes utility code into a Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Curl as libcurl
participant Resolver as Async Resolver (c-ares/threaded)
participant DNSCache as DNS Cache
App->>Curl: curl_easy_perform()
Curl->>Resolver: Curl_async_getaddrinfo(host, port, ip_version)
Resolver-->>Curl: async result (via callback or poll)
Curl->>DNSCache: Curl_dnscache_mk_entry(addrinfo, host, port)
DNSCache-->>Curl: DNS entry
Curl->>Curl: Setup connection (metadata for protocol state)
Curl->>App: Data transfer proceeds
sequenceDiagram
participant Curl as libcurl
participant Meta as Meta Data API
Curl->>Meta: Curl_meta_set(handle, key, value, dtor)
Note right of Meta: Stores protocol/file state
Curl->>Meta: Curl_meta_get(handle, key)
Note right of Meta: Retrieves state for operations
Curl->>Meta: Curl_meta_remove(handle, key)
Note right of Meta: Cleans up state with destructor
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 20
🔭 Outside diff range comments (5)
deps/curl/lib/content_encoding.c (1)
794-803:⚠️ Potential issueReturning on duplicate
chunkedsilently skips the remaining encodingsIf a response contains “
chunked, chunked, gzip”, the duplicate-check returnsCURLE_OKimmediately, so any encodings after the secondchunkedare never parsed or validated.
That silently accepts an invalid header instead of rejecting it, and leaves the unencoding stack incomplete.- CURL_TRC_WRITE(data, "ignoring duplicate 'chunked' decoder"); - return CURLE_OK; + CURL_TRC_WRITE(data, "ignoring duplicate 'chunked' decoder"); + /* keep parsing the rest of the list to detect further errors */ + continue;Consider continuing the loop (or, per RFC 9112, failing hard) instead of returning.
deps/curl/lib/cf-socket.c (1)
195-196: 💡 Verification agent🧩 Analysis chain
Verify the function renaming consistency across the codebase.
The time-related functions have been consistently renamed from
Curl_*tocurlx_*throughout this file. However, there are many conversions between different types usingcurlx_sltosi()and string parsing withcurlx_str_number().Run the following script to ensure all time-related functions have been properly renamed:
Also applies to: 223-224, 237-237, 246-247, 256-257, 266-267, 287-287, 296-297, 727-727, 962-963, 968-969, 974-975, 980-981, 1108-1108, 1224-1224, 1343-1343, 1446-1446, 1718-1718, 1719-1719, 1733-1733, 2035-2035, 2044-2044, 2167-2167, 2234-2234, 2271-2271, 2313-2313
🏁 Script executed:
#!/bin/bash # Description: Verify that old Curl_ time functions are not used elsewhere # Check for any remaining uses of old time function names echo "Checking for old Curl_ time function usage..." rg -A 2 'Curl_(now|timediff)\(' --type c # Verify the new curlx_ functions are properly declared echo -e "\nVerifying curlx_ time function declarations..." rg 'curlx_(now|timediff)' deps/curl/lib/curlx/timeval.hLength of output: 973
Function Renaming Verified – Minor Comment Update Needed
All code references to
Curl_now/Curl_timediffhave been replaced withcurlx_now/curlx_timediff. The only occurrence of the old name is in a comment:• deps/curl/lib/pingpong.h: update the comment referencing
Curl_now()tocurlx_now()deps/curl/lib/cf-https-connect.c (1)
149-166: 🛠️ Refactor suggestionReturn-code handling could short-circuit on setup failure.
If
Curl_cf_setup_insert_after()fails,b->resultis set but we still
continue, assignb->cf = cf->next, and leave the caller to discover the
error later. A fast-fail with an earlyreturn(or at least a comment)
would make the intent clearer and avoid dereferencing an unexpected NULL
in later stages.if(!b->result) b->result = Curl_cf_setup_insert_after(cf, data, transport, CURL_CF_SSL_ENABLE); - b->cf = cf->next; + if(b->result) + return; /* abort init – caller will see b->result */ + + b->cf = cf->next;deps/curl/lib/hostip.c (2)
169-180:⚠️ Potential issue
create_dnscache_id()may overflow onmsnprintferror
msnprintf()returns anint, which becomes negative on truncation/error.
Casting that negative value tosize_tand addinglensilently turns it into a huge
positive number, corrupting the length used by the hash table.- return msnprintf(&ptr[len], 7, ":%u", port) + len; + int wrote = msnprintf(&ptr[len], 7, ":%u", port); + if(wrote < 0) + wrote = 0; /* guarantee non-negative */ + return (size_t)wrote + len;Without this guard, any write failure can turn later
entry_len + 1
arguments into gigantic values → out-of-bounds reads/writes in the hash.
195-210:⚠️ Potential issueStale-entry predicate deletes the entire cache when
dns_cache_timeout == -1
-1is documented as “forever”, but hereif(age >= (time_t)prune->max_age_sec)evaluates true for every entry when
max_age_sec == -1because every
ageis ≥ −1.
Result:CURLMOPT_MAX_HOSTCACHE_SIZEis effectively ignored and the cache is
flushed on every prune.- if(age >= (time_t)prune->max_age_sec) + if(prune->max_age_sec >= 0 && /* honour ‘forever’ */ + age >= (time_t)prune->max_age_sec)
🧹 Nitpick comments (27)
deps/curl/include/curl/curl.h (1)
2261-2263: Approve newCURLOPT_SSL_SIGNATURE_ALGORITHMSentry
The enum value 328 follows the existing sequence, uses the correctSTRINGPOINTtype, and the inline comment clearly describes its purpose.
Consider updating the official libcurl API documentation to include this new option.deps/curl/lib/curlx/winapi.c (2)
30-30: Remove redundant#ifdef _WIN32check.There's a duplicate
#ifdef _WIN32check at line 48 that's already covered by the outer check starting at line 30.Apply this diff to remove the redundant check:
-#ifdef _WIN32 /* This is a helper function for Curl_strerror that converts Windows API error * codes (GetLastError) to error messages. * Returns NULL if no error message was found for error code. */ const char *curlx_get_winapi_error(int err, char *buf, size_t buflen) { char *p; wchar_t wbuf[256]; if(!buflen) return NULL; *buf = '\0'; *wbuf = L'\0'; /* We return the local codepage version of the error string because if it is output to the user's terminal it will likely be with functions which expect the local codepage (eg fprintf, failf, infof). FormatMessageW -> wcstombs is used for Windows CE compatibility. */ if(FormatMessageW((FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS), NULL, (DWORD)err, LANG_NEUTRAL, wbuf, CURL_ARRAYSIZE(wbuf), NULL)) { size_t written = wcstombs(buf, wbuf, buflen - 1); if(written != (size_t)-1) buf[written] = '\0'; else *buf = '\0'; } /* Truncate multiple lines */ p = strchr(buf, '\n'); if(p) { if(p > buf && *(p-1) == '\r') *(p-1) = '\0'; else *p = '\0'; } return *buf ? buf : NULL; } -#endif /* _WIN32 */Also applies to: 48-89
104-117: Consider extracting the error formatting logic.The error formatting code is duplicated between the verbose and non-verbose paths. The formatted string could overflow the buffer despite the check.
Consider using a safer approach that guarantees no buffer overflow:
#ifndef CURL_DISABLE_VERBOSE_STRINGS if(!curlx_get_winapi_error((int)err, buf, buflen)) { -#if defined(__GNUC__) && __GNUC__ >= 7 -#pragma GCC diagnostic push -#pragma GCC diagnostic warning "-Wformat-truncation=1" -#endif - /* some GCC compilers cause false positive warnings if we allow this - warning */ - SNPRINTF(buf, buflen, "Unknown error %lu (0x%08lX)", err, err); -#if defined(__GNUC__) && __GNUC__ >= 7 -#pragma GCC diagnostic pop -#endif - + int ret = SNPRINTF(buf, buflen, "Unknown error %lu (0x%08lX)", err, err); + if(ret < 0 || (size_t)ret >= buflen) { + /* Truncation occurred, ensure null termination */ + buf[buflen - 1] = '\0'; + } }deps/curl/lib/cf-https-connect.h (1)
34-34: Consider removing unused forward declaration.The
struct Curl_dns_entryforward declaration appears to be unused after the removal of theremotehostparameter.If this struct is no longer used in this header, consider removing the forward declaration:
struct Curl_easy; struct connectdata; struct Curl_cftype; -struct Curl_dns_entry;deps/curl/lib/curl_rtmp.c (2)
225-233: Destructor good, but missing null-check inside
RTMP_Close()segfaults on a null pointer.
Either guard or rely on the caller never passing NULL; the latter is fragile.- RTMP_Close(r); - RTMP_Free(r); + if(r) { + RTMP_Close(r); + RTMP_Free(r); + }
254-259: Early NULL check avoids UB but loses diagnostic detailIf
ris NULL, the function returnsCURLE_FAILED_INITwithout logging why metadata is missing.
Consider adding afailf()to aid debugging.deps/curl/lib/curl_config.h.cmake (1)
706-720: Keep backward-compat macro names until all call-sites are migratedOld wolfSSL feature macros (
HAVE_WOLFSSL_BIO,HAVE_WOLFSSL_FULL_BIO) were renamed.
Ensure every source file was updated, otherwise compilation breaks on mixed trees.If complete migration cannot be guaranteed, temporarily alias the old names:
#ifndef HAVE_WOLFSSL_BIO #define HAVE_WOLFSSL_BIO HAVE_WOLFSSL_BIO_NEW #endif #ifndef HAVE_WOLFSSL_FULL_BIO #define HAVE_WOLFSSL_FULL_BIO HAVE_WOLFSSL_BIO_SET_SHUTDOWN #endifdeps/curl/lib/cshutdn.c (1)
181-217: Well-designed selective connection destruction feature.The enhanced
cshutdn_destroy_oldest()function now supports selective destruction based on destination, and the new publicCurl_cshutdn_close_oldest()function provides a clean API for this functionality. The implementation correctly handles the case when no matching connection is found by returningFALSE.Consider adding a trace log when a connection matching the destination is found and destroyed:
if(e) { SIGPIPE_VARIABLE(pipe_st); conn = Curl_node_elem(e); + CURL_TRC_M(data, "[SHUTDOWN] destroying connection #%" FMT_OFF_T + " to destination '%s'", conn->connection_id, + destination ? destination : "any"); Curl_node_remove(e); sigpipe_init(&pipe_st); sigpipe_apply(data, &pipe_st); Curl_cshutdn_terminate(data, conn, FALSE); sigpipe_restore(&pipe_st); return TRUE; }deps/curl/lib/ftplistparser.c (1)
971-977: Dynamic-buffer pointer may be stale aftercurlx_dyn_setlen()/curlx_dyn_reset()Within
parse_unix()the localmempointer is obtained once at entry,
but later branches (TOTALSIZE_READING,SYMLINK_PRETARGET3…) call
curlx_dyn_setlen()orcurlx_dyn_reset(), either of which can reallocate
the internal buffer. Accessingmemafter such a call would then be
undefined.Although the current control-flow does not reuse
memafter those
mutating calls, a future maintenance edit could easily do so.
Recommend re-fetching the pointer (mem = curlx_dyn_ptr(&infop->buf);)
immediately after every operation that can reallocate to eliminate the
foot-gun.deps/curl/lib/http.c (1)
3641-3641: Grammar correction.Fixed the article usage: "a HTTP/2" → "an HTTP/2".
deps/curl/lib/curlx/timeval.c (1)
36-37: Address the TODO about initialization function placement.The comment indicates that providing
curlx_now_init()here is architecturally wrong for tools and tests. This suggests a design issue that should be addressed rather than left as a comment.Would you like me to suggest a better initialization pattern that separates library initialization from tool/test initialization?
deps/curl/lib/conncache.c (1)
324-326: Tiny style typo –=- 1
timediff_t highscore =- 1;works but looks like a typo.
Use the conventional= -1(with a blank) for readability.deps/curl/lib/curlx/base64.c (2)
179-229:base64_encode()may generate padding-less output thatcurlx_base64_decode()rejectsWhen
padbyte == 0(URL-safe variant) the encoder emits 2 or 3 characters for the last quantum,
yet the decoder enforcessrclen % 4 == 0and expects=padding.Either:
- Document that
curlx_base64_decode()only handles RFC 4648 section 4 (“classic”) input, or- Extend the decoder to tolerate un-padded Base64URL strings so the pair round-trips.
Failure manifests as
CURLE_BAD_CONTENT_ENCODING.
47-49: Naming inconsistencyEverything in the new namespace is prefixed
curlx_, but the exported table is still
Curl_base64encdec. Considercurlx_base64_tablefor uniformity.deps/curl/lib/easy.c (1)
1083-1091: Double-cleaningmeta_hash
curl_easy_reset()callsCurl_hash_clean(&data->meta_hash);and immediately afterwards
Curl_meta_reset(data)which cleans the same hash again.
Harmless but redundant; drop one of them to avoid extra hash traversal.deps/curl/lib/cf-https-connect.c (1)
653-667: Stale(void)sockindex;annotation – variable is actually used below.The cast-to-void was meant to silence “unused parameter” warnings, but
sockindexis now referenced when accessingdata->state.dns[sockindex].
Leaving the cast is misleading.- (void)sockindex;deps/curl/lib/http2.c (1)
286-314: Conditional-return logic harder to followInside
cf_h2_get_desired_local_win()the#ifdef DEBUGBUILDbranch nests anelsethat returns, while the non-debug build returns in a detached#else.
The current layout is correct but brittle; a future refactor might accidentally drop the final return in the DEBUG path and make the function fall through.
Recommend a single exit point or explicitreturnimmediately after the#ifdefblock for clarity.deps/curl/lib/file.c (1)
140-148: Double-free / dangling pointer risk infile_cleanupAfter freeing
file->freepath, the pointer is not reset.
Iffile_cleanup()is called twice (possible via multiple error paths), the second call will free the same memory again.Curl_safefree(file->freepath); + file->freepath = NULL;Same applies to
pathfor completeness.deps/curl/lib/doh.c (2)
306-313: Incorrect error mapping fordoh_req_encodefailuresAll failures from
doh_req_encode()(e.g.DOH_DNS_NAME_TOO_LONG,
DOH_DNS_BAD_LABEL) are converted toCURLE_OUT_OF_MEMORY, which hides
the real cause from callers and makes debugging hard.Consider translating the individual
DOHcodevalues to the most
appropriateCURLcode(CURLE_URL_MALFORMAT,CURLE_BAD_FUNCTION_ARGUMENT
…) or extending libcurl with a dedicated error constant for DoH encoding
problems.
265-275: Destructor must tolerate NULL gracefully
doh_probe_dtor()unconditionally freesdoh_req->req_hds.
curl_slist_free_all()already handlesNULL, so the explicitif(e)
guard is redundant and slightly obscures intent.
Nit but easy to simplify:-static void doh_probe_dtor(void *key, size_t klen, void *e) -{ - (void)key; - (void)klen; - if(e) { - struct doh_request *doh_req = e; - ... - } -} +static void doh_probe_dtor(void *key, size_t klen, void *e) +{ + (void)key; (void)klen; + if(!e) return; + struct doh_request *doh_req = e; + ... +}deps/curl/include/curl/typecheck-gcc.h (1)
319-323:curlcheck_curl_option()currently matches no optionsThe macro ends with
|| 0and only lists the two stream-dependency
options. If newCURL *options are added (e.g. future meta-handle
setters) they will silently miss compile-time checking.Recommend adding a comment and a
TODOto keep this list in sync with
curl_easy_setopt()additions, or deriving it fromCURLOPTTYPE_OBJECTPOINT
range like the other helper macros.deps/curl/lib/curlx/strparse.c (1)
142-153: Verify the comment about bitwise AND operation.The comment on line 144-146 mentions "the necessary bitwise AND in the loop" but the
valid_digitmacro doesn't show any bitwise AND operation in its definition. The AND operation appears to be in theCurl_hexvalmacro (likely defined in the header), not in thevalid_digitmacro itself.Consider updating the comment to clarify that the bitwise AND operation is performed in
Curl_hexval, not invalid_digit:-/* We use 16 for the zero index (and the necessary bitwise AND in the loop) - to be able to have a non-zero value there to make valid_digit() able to - use the info */ +/* We use 16 for the zero index to have a non-zero value there, making + valid_digit() able to use the info. The bitwise AND is performed in + Curl_hexval() to extract the lower 4 bits */deps/curl/lib/asyn.h (1)
73-82: Comment talks aboutnumsock, prototype no longer receives it.Documentation still refers to a
numsockparameter that was removed from the
API. This is misleading for downstream contributors and for anyone generating
bindings from the public headers.Either restore the parameter or update the comment block to reflect the new
signature.deps/curl/lib/hostip.h (1)
126-132: DNS cache struct is global but not thread-safe
Curl_dnscache_init()exposes a rawCurl_hashwithout any locking strategy.
With multi-handle / multi-threaded users this will race.Consider embedding a mutex inside
struct Curl_dnscacheor mandate external
serialisation in the public API documentation.deps/curl/lib/hostip.c (2)
243-256:dnscache_lock/unlock()ignore the multi-handle cache – race windowOnly the shared DNS cache is protected; the default multi-handle cache
(data->multi->dnscache) is left unlocked. libcurl guarantees that a
single multi handle is used from one thread, but applications frequently
drive several multi handles from different threads. Two multi handles
sharing the same easy via connection coalescing will now race on the
hash table.Consider at least:
- Adding an internal lock in
struct Curl_dnscache.- Leveraging
Curl_simple_lockaround the hash for non-shared caches.
995-1003: Timeout path bypassesallowDOH == TRUEearly exit
Curl_resolv_timeout()shortcuts toCurl_resolv()only when
timeout==0OR DoH is enabled. When a finite timeout is set and DoH is
requested, the code drops into the slow SIGALRM path although DoH never
uses blocking resolver calls.Consider jumping directly to
Curl_resolv()whendata->set.dohis
true, regardless oftimeout.Also applies to: 1089-1097
deps/curl/lib/ftp.c (1)
308-313:(void)ftpchides a real-use variable – drop the cast instead of pretending it is unusedThe compile-time cast marks
ftpcas intentionally unused, yet the very next line uses it in the trace macro.
That is misleading for readers and static-analysis tools.- (void)ftpc; - CURL_TRC_FTP(data, "[%s] closing DATA connection", FTP_CSTATE(ftpc)); + /* `ftpc` is required for the log line below. */ + CURL_TRC_FTP(data, "[%s] closing DATA connection", FTP_CSTATE(ftpc));This is a trivial change that eliminates the dead-code annotation and keeps the intent crystal-clear.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (123)
deps/curl/curl.gyp(3 hunks)deps/curl/include/curl/Makefile.am(1 hunks)deps/curl/include/curl/Makefile.in(2 hunks)deps/curl/include/curl/curl.h(2 hunks)deps/curl/include/curl/curlver.h(3 hunks)deps/curl/include/curl/system.h(2 hunks)deps/curl/include/curl/typecheck-gcc.h(6 hunks)deps/curl/include/curl/websockets.h(1 hunks)deps/curl/lib/CMakeLists.txt(9 hunks)deps/curl/lib/Makefile.am(4 hunks)deps/curl/lib/Makefile.inc(7 hunks)deps/curl/lib/altsvc.c(11 hunks)deps/curl/lib/altsvc.h(1 hunks)deps/curl/lib/asyn-ares.c(22 hunks)deps/curl/lib/asyn-base.c(1 hunks)deps/curl/lib/asyn-thrdd.c(1 hunks)deps/curl/lib/asyn-thread.c(0 hunks)deps/curl/lib/asyn.h(3 hunks)deps/curl/lib/cf-h1-proxy.c(13 hunks)deps/curl/lib/cf-h2-proxy.c(1 hunks)deps/curl/lib/cf-haproxy.c(5 hunks)deps/curl/lib/cf-https-connect.c(9 hunks)deps/curl/lib/cf-https-connect.h(1 hunks)deps/curl/lib/cf-socket.c(17 hunks)deps/curl/lib/cf-socket.h(1 hunks)deps/curl/lib/cfilters.c(5 hunks)deps/curl/lib/cfilters.h(2 hunks)deps/curl/lib/config-win32.h(1 hunks)deps/curl/lib/conncache.c(12 hunks)deps/curl/lib/conncache.h(2 hunks)deps/curl/lib/connect.c(27 hunks)deps/curl/lib/connect.h(2 hunks)deps/curl/lib/content_encoding.c(3 hunks)deps/curl/lib/cookie.c(12 hunks)deps/curl/lib/cshutdn.c(9 hunks)deps/curl/lib/cshutdn.h(2 hunks)deps/curl/lib/curl_addrinfo.c(5 hunks)deps/curl/lib/curl_config.h.cmake(5 hunks)deps/curl/lib/curl_config.h.in(7 hunks)deps/curl/lib/curl_get_line.c(3 hunks)deps/curl/lib/curl_get_line.h(1 hunks)deps/curl/lib/curl_krb5.h(1 hunks)deps/curl/lib/curl_ntlm_core.c(1 hunks)deps/curl/lib/curl_printf.h(2 hunks)deps/curl/lib/curl_range.c(2 hunks)deps/curl/lib/curl_rtmp.c(7 hunks)deps/curl/lib/curl_sasl.c(4 hunks)deps/curl/lib/curl_sasl.h(1 hunks)deps/curl/lib/curl_setup.h(1 hunks)deps/curl/lib/curl_sha512_256.c(1 hunks)deps/curl/lib/curl_sspi.c(3 hunks)deps/curl/lib/curl_sspi.h(1 hunks)deps/curl/lib/curl_threads.c(3 hunks)deps/curl/lib/curl_threads.h(1 hunks)deps/curl/lib/curl_trc.c(3 hunks)deps/curl/lib/curlx/base64.c(7 hunks)deps/curl/lib/curlx/base64.h(1 hunks)deps/curl/lib/curlx/curlx.h(2 hunks)deps/curl/lib/curlx/dynbuf.c(15 hunks)deps/curl/lib/curlx/dynbuf.h(2 hunks)deps/curl/lib/curlx/inet_pton.c(3 hunks)deps/curl/lib/curlx/inet_pton.h(1 hunks)deps/curl/lib/curlx/multibyte.c(12 hunks)deps/curl/lib/curlx/multibyte.h(2 hunks)deps/curl/lib/curlx/nonblock.c(1 hunks)deps/curl/lib/curlx/strparse.c(12 hunks)deps/curl/lib/curlx/strparse.h(2 hunks)deps/curl/lib/curlx/timediff.h(1 hunks)deps/curl/lib/curlx/timeval.c(8 hunks)deps/curl/lib/curlx/timeval.h(1 hunks)deps/curl/lib/curlx/version_win32.c(2 hunks)deps/curl/lib/curlx/version_win32.h(1 hunks)deps/curl/lib/curlx/warnless.c(1 hunks)deps/curl/lib/curlx/warnless.h(1 hunks)deps/curl/lib/curlx/winapi.c(1 hunks)deps/curl/lib/curlx/winapi.h(1 hunks)deps/curl/lib/cw-out.c(6 hunks)deps/curl/lib/cw-out.h(1 hunks)deps/curl/lib/cw-pause.c(1 hunks)deps/curl/lib/cw-pause.h(1 hunks)deps/curl/lib/dict.c(2 hunks)deps/curl/lib/dllmain.c(1 hunks)deps/curl/lib/doh.c(22 hunks)deps/curl/lib/doh.h(3 hunks)deps/curl/lib/dynhds.c(1 hunks)deps/curl/lib/dynhds.h(3 hunks)deps/curl/lib/easy.c(17 hunks)deps/curl/lib/easygetopt.c(1 hunks)deps/curl/lib/easyoptions.c(3 hunks)deps/curl/lib/easyoptions.h(1 hunks)deps/curl/lib/escape.c(4 hunks)deps/curl/lib/escape.h(1 hunks)deps/curl/lib/fake_addrinfo.c(1 hunks)deps/curl/lib/fake_addrinfo.h(1 hunks)deps/curl/lib/file.c(11 hunks)deps/curl/lib/file.h(0 hunks)deps/curl/lib/fileinfo.c(1 hunks)deps/curl/lib/fileinfo.h(1 hunks)deps/curl/lib/fopen.c(2 hunks)deps/curl/lib/formdata.c(7 hunks)deps/curl/lib/formdata.h(1 hunks)deps/curl/lib/ftp.c(145 hunks)deps/curl/lib/ftp.h(2 hunks)deps/curl/lib/ftplistparser.c(4 hunks)deps/curl/lib/getinfo.c(7 hunks)deps/curl/lib/gopher.c(1 hunks)deps/curl/lib/hash.c(1 hunks)deps/curl/lib/hash.h(1 hunks)deps/curl/lib/headers.c(4 hunks)deps/curl/lib/hmac.c(1 hunks)deps/curl/lib/hostasyn.c(0 hunks)deps/curl/lib/hostip.c(38 hunks)deps/curl/lib/hostip.h(5 hunks)deps/curl/lib/hostip4.c(2 hunks)deps/curl/lib/hostip6.c(3 hunks)deps/curl/lib/hostsyn.c(0 hunks)deps/curl/lib/hsts.c(9 hunks)deps/curl/lib/hsts.h(1 hunks)deps/curl/lib/http.c(60 hunks)deps/curl/lib/http.h(1 hunks)deps/curl/lib/http1.c(5 hunks)deps/curl/lib/http1.h(1 hunks)deps/curl/lib/http2.c(20 hunks)
💤 Files with no reviewable changes (4)
- deps/curl/lib/file.h
- deps/curl/lib/hostsyn.c
- deps/curl/lib/hostasyn.c
- deps/curl/lib/asyn-thread.c
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test-macOS
- GitHub Check: build-docs
- GitHub Check: lint-js-and-md
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: build-tarball
🔇 Additional comments (219)
deps/curl/lib/curl_sha512_256.c (1)
30-30: Consistent include path update for warnless.h
The include has been updated to thecurlx/namespace, reflecting the relocation ofwarnless.hintocurlx/. Confirm that no residual references to the old header remain to avoid duplicate definitions.deps/curl/lib/fileinfo.h (1)
29-29: Align dynamic buffer include with new curlx namespace
Updating"dynbuf.h"to"curlx/dynbuf.h"matches the reorganization. Verify thatstruct dynbufis still exported from the new header and no build breakage occurs.deps/curl/lib/gopher.c (1)
42-42: Update warnless include to curlx namespace
Replacing the internal helper include with"curlx/warnless.h"aligns with other subsystems. All references towarnless.hshould now be consolidated undercurlx/.deps/curl/lib/curlx/nonblock.c (1)
25-25: Adjust include for relocated curl_setup.h
Using#include "../curl_setup.h"is correct now that this file lives undercurlx/. Ensure all build configurations include this path.deps/curl/lib/curlx/warnless.h (1)
27-27: Relative include for curl_setup.h within curlx module
The change to../curl_setup.his consistent with the new directory layout. Double-check include guards and path resolution during builds.deps/curl/lib/hmac.c (1)
37-37: Maintain consistency with reorganized utility headers
The include directive has been updated to"curlx/warnless.h"to align with the newcurlxnamespace structure.deps/curl/lib/curlx/inet_pton.h (1)
27-27: Use correct relative include for curl_setup.h
The header include is now"../curl_setup.h", matching the relocatedcurlxdirectory.deps/curl/lib/curl_ntlm_core.c (1)
129-129: Update include tocurlx/warnless.h
Thewarnless.hinclude is moved undercurlx, reflecting the reorganized layout.deps/curl/lib/curlx/timediff.h (1)
27-27: Adjust include path for curl_setup.h
Changed to"../curl_setup.h"to reflect the file’s new location within thecurlxfolder.deps/curl/lib/curlx/version_win32.h (1)
27-27: Correct relative include for curl_setup.h
The include directive has been updated to"../curl_setup.h"for consistency acrosscurlxheaders.deps/curl/lib/cf-h2-proxy.c (1)
35-35: Update dynamic buffer include path to curlx namespace
The include of"curlx/dynbuf.h"correctly reflects the relocation of the dynamic buffer API into thecurlxsubdirectory.deps/curl/lib/curl_sspi.h (1)
48-48: Skip whitespace-only comment formatting change
This modification only adjusts whitespace around the SSPI realm comment and does not affect any logic or declarations.deps/curl/lib/fileinfo.c (1)
43-43: Switch to newcurlx_dyn_freeAPI for buffer cleanup
ReplacingCurl_dyn_freewithcurlx_dyn_freealigns with the standardizedcurlx_prefix for dynamic buffer utilities.deps/curl/lib/curlx/warnless.c (1)
25-25: Consolidatewarnless.hinclude at file top
Moving the header include to the top removes redundancy and ensures all macro overrides apply consistently.deps/curl/lib/cf-socket.h (1)
28-28: Update non-blocking socket include tocurlx/nonblock.h
This aligns the include path with the relocatednonblockutility under thecurlxnamespace.deps/curl/lib/config-win32.h (1)
228-231: LGTM! Good grammatical correction.The change from "a IPv6" to "an IPv6" is grammatically correct since IPv6 starts with a vowel sound. This improves code documentation quality.
deps/curl/lib/http1.h (1)
45-45: LGTM! Bitfield conversion for memory optimization.The conversion from
bool donetoBIT(done)aligns with the systematic optimization to use bitfields for compact storage. This should maintain the same logical behavior while improving memory efficiency.deps/curl/lib/dynhds.h (3)
29-29: LGTM! Include path updated for reorganized structure.The change to
"curlx/dynbuf.h"aligns with the broader codebase reorganization moving dynamic buffer utilities to the curlx namespace.
35-35: LGTM! Terminology standardization.The change from "NUL terminated" to "null-terminated" improves consistency and follows standard terminology conventions.
Also applies to: 116-116
159-159: LGTM! Protocol terminology standardization.Using "CRLF" instead of "\r\n" in comments improves readability and follows standard HTTP protocol terminology.
Also applies to: 166-166
deps/curl/lib/curl_sasl.h (1)
165-165: LGTM! New diagnostic function for SASL authentication.The addition of
Curl_sasl_is_blocked()provides useful diagnostic capability to check and report why SASL authentication mechanisms may be blocked. The function signature is consistent with existing SASL API conventions.deps/curl/lib/http.h (1)
177-180: Define MAX_HTTP_RESP_HEADER_COUNT to cap header entriesAdding this macro helps guard against excessive header accumulation during CONNECT and redirect scenarios, preventing potential memory bloat.
deps/curl/lib/curl_get_line.h (1)
27-27: Update include path for dynbuf headerSwitching to the new
curlx/dynbuf.hpath aligns this header with the refactored utility namespace.deps/curl/lib/hash.c (1)
355-363: Rename key compare function tocurlx_str_key_compareAligns with the standardized
curlx_prefix for internal utility functions; signature remains unchanged.deps/curl/lib/cw-pause.c (1)
90-97: MarkCurl_cwt_pauseas constEnforcing
conston the globalCurl_cwt_pauseinstance improves immutability, safety, and readability.deps/curl/lib/hsts.h (1)
39-41: ConvertincludeSubDomainsto a bitfieldUsing
BIT(includeSubDomains)reduces memory footprint and matches other HSTS flag conventions.deps/curl/lib/dllmain.c (1)
36-37: LGTM: Clarified DLL build conditionsThe change correctly restricts
DllMain()to native Windows DLL builds only, excluding Cygwin. This aligns with the libcurl 8.14.1 build system updates and simplifies the build configuration.deps/curl/lib/cw-out.h (1)
36-36: LGTM: Improved const-correctnessMaking
Curl_cwt_outconst prevents accidental modification and improves type safety. This aligns with the broader const-correctness improvements in libcurl 8.14.1.deps/curl/lib/easyoptions.h (1)
32-32: LGTM: Enhanced type safety for options arrayMaking
Curl_easyoptsconst is appropriate for what appears to be a lookup table. This prevents accidental modification and improves type safety, consistent with the const-correctness improvements throughout libcurl 8.14.1.deps/curl/lib/dynhds.c (1)
362-365: LGTM: Namespace update for dynamic buffer APIThe function call update from
Curl_dyn_addftocurlx_dyn_addfis part of libcurl's namespace reorganization, moving utility functions to thecurlx_prefix. The functionality remains unchanged.deps/curl/lib/cw-pause.h (1)
35-35: Good practice: Making global variable immutable.Adding the
constqualifier to theCurl_cwt_pausedeclaration improves code safety by preventing accidental modifications to this global control word type instance.deps/curl/lib/curlx/version_win32.c (1)
25-25: LGTM: Include path adjustments for file reorganization.The relative path changes correctly reflect the file's new location in the
curlx/subdirectory. These adjustments are necessary for proper compilation.Also applies to: 34-35
deps/curl/include/curl/websockets.h (1)
77-77: LGTM: Well-designed WebSocket option flag addition.The new
CURLWS_NOAUTOPONGflag properly uses the next available bit position and follows established naming conventions. This extends the WebSocket API functionality appropriately.deps/curl/lib/cf-haproxy.c (1)
57-57:Details
✅ Verification successful
LGTM: Consistent namespace refactoring for dynamic buffer functions.
All dynamic buffer function calls have been systematically updated from
Curl_dyn_*tocurlx_dyn_*. This refactoring improves code organization by moving utility functions to the dedicatedcurlxnamespace while maintaining identical functionality.Verify that all dynamic buffer function references have been updated consistently across the codebase:
Also applies to: 63-63, 82-82, 95-98, 132-132, 136-136, 144-145, 153-153, 225-225
🏁 Script executed:
#!/bin/bash # Description: Verify no remaining Curl_dyn_* function calls that should be curlx_dyn_* # Expected: No remaining Curl_dyn_ calls in updated files # Check for any remaining Curl_dyn_ function calls echo "Checking for remaining Curl_dyn_ function calls:" rg "Curl_dyn_" --type c # Check for proper curlx_dyn_ usage echo -e "\nVerifying curlx_dyn_ function usage:" rg "curlx_dyn_" --type c -A 2Length of output: 70774
Verification passed: No remaining
Curl_dyn_*usagesAll occurrences of the old
Curl_dyn_*functions have been replaced withcurlx_dyn_*across thedeps/curl/libtree. No further changes are needed.deps/curl/lib/cfilters.h (2)
27-27: LGTM: Include path updated for reorganization.The include path change from
"timediff.h"to"curlx/timediff.h"aligns with the broader reorganization effort to centralize utility headers under thecurlx/subdirectory.
373-376: LGTM: Useful utility function added.The new
Curl_conn_is_setupfunction provides a clean way to check if a filter chain exists at a given socket index, which complements the existing connection management functions.deps/curl/lib/hash.h (1)
101-102: LGTM: Function renamed for consistency.The renaming from
Curl_str_key_comparetocurlx_str_key_comparealigns with the standardization effort to use thecurlx_prefix for utility functions throughout libcurl.deps/curl/lib/curl_range.c (2)
29-29: LGTM: Include path updated for reorganization.The include path change from
"strparse.h"to"curlx/strparse.h"aligns with the utility reorganization effort.
44-51: LGTM: Function calls updated to use curlx namespace.All string parsing function calls have been consistently updated to use the new
curlx_prefixed versions (curlx_str_numberandcurlx_str_single), maintaining the same functionality while aligning with the naming standardization effort.deps/curl/lib/curl_krb5.h (1)
42-42: LGTM: Conditional compilation refined appropriately.The refined conditional from
#ifdef HAVE_GSSAPIto#if defined(HAVE_GSSAPI) && !defined(CURL_DISABLE_FTP)properly restricts Kerberos security functions to builds with both GSSAPI enabled and FTP support not disabled, which aligns with their usage context.deps/curl/lib/curl_threads.h (1)
64-64: LGTM: Function signature improved for better resource management.Changing
Curl_thread_destroyto acceptcurl_thread_t *hndinstead ofcurl_thread_t hndallows the function to properly nullify the caller's handle after destruction, preventing dangling thread handles and improving resource management consistency.deps/curl/lib/altsvc.h (1)
43-44: LGTM! Memory optimization through bitfield conversion.The conversion of
persistfromboolto a bitfield using theBIT()macro is a good memory optimization. The reordering of struct members (priomoved abovepersist) likely improves memory alignment. This change maintains the same functionality while reducing memory footprint.deps/curl/lib/easygetopt.c (2)
32-32: LGTM! Improved const-correctness for function signature.Changing the return type to
const struct curl_easyoption *correctly enforces immutability of the option data returned by the lookup function. This prevents accidental modification and clearly communicates the intent that callers should treat the returned data as read-only.
37-37: LGTM! Consistent const usage for local variable.Making the local variable
oconst is consistent with the function's return type and prevents accidental modification within the function. This enhances code safety and maintains const-correctness throughout the lookup operation.deps/curl/lib/escape.h (1)
44-46: LGTM! Well-designed utility function for hex encoding.The new
Curl_hexbytefunction provides a clean, reusable interface for encoding a single byte to its hexadecimal representation. The design is good:
- Clear parameter names and types
- Helpful comment about buffer size requirement
lowercaseparameter provides formatting flexibility- Follows libcurl naming conventions
This centralizes hex encoding logic and improves code maintainability.
deps/curl/lib/curl_trc.c (3)
46-46: LGTM! Include path updated for curlx reorganization.The include path correctly reflects the new location of string parsing utilities in the
curlxsubdirectory.
500-500: LGTM! Function calls updated to curlx namespace.The function calls have been correctly updated from
Curl_str_casecomparetocurlx_str_casecompareas part of the namespace reorganization.Also applies to: 506-506
530-530: LGTM! String utility functions updated to curlx namespace.All string utility function calls have been consistently updated to use the new
curlx_str_*namespace, aligning with the modularization effort described in the AI summary.Also applies to: 532-532, 536-536, 539-539, 541-541, 543-543, 545-545, 547-547, 549-549, 556-556
deps/curl/include/curl/Makefile.am (1)
38-38: LGTM! Conditional compilation improves CI build performance.The additional
NOT_CURL_CIcondition appropriately restricts source checking to local debug builds, avoiding redundant checks during CI while preserving the quality control for development builds.Also applies to: 43-43
deps/curl/lib/fopen.c (1)
69-69: LGTM! Dynamic buffer functions updated to curlx namespace.The dynamic buffer function calls have been correctly updated from
Curl_dyn_*tocurlx_dyn_*as part of the namespace reorganization. The changes maintain the same functionality while aligning with the new modular structure.Also applies to: 79-79, 82-82, 84-84
deps/curl/lib/curlx/multibyte.h (2)
26-26: LGTM! Include path correctly adjusted for directory structure.The relative path
"../curl_setup.h"is appropriate since this file is now located in thecurlxsubdirectory.
85-85:Details
✅ Verification successful
Verify that the simplified macro doesn't introduce memory safety issues.
The
curlx_unicodefreemacro has been simplified from a likely multi-line implementation to a directfree()call. This could introduce issues if:
- The original implementation included null pointer checks
- The calling code expects the pointer to be nullified after freeing
- Double-free scenarios become possible
🏁 Script executed:
#!/bin/bash # Description: Search for the original implementation of curlx_unicodefree to understand what safety measures were removed # Search for any previous implementation patterns of unicodefree or similar macros rg -A 10 -B 5 "unicodefree.*do.*while|unicodefree.*\{" --type c --type h # Search for usage patterns to understand if null checks or pointer nullification are expected rg -A 3 -B 3 "curlx_unicodefree" --type c --type hLength of output: 16252
No memory-safety issues introduced by the simplified
curlx_unicodefreemacroThe macro now expands to an untracked
free()call, which:
- Safely handles
NULLpointers (per the C standard)- Does not pretend to null out the caller’s pointer (and none of the existing call sites use the pointer after freeing)
- Would only lead to double-free errors if the calling code were already doing so
All current usages guard against freeing invalid pointers or never reference them post-free. You can consider this resolved.
deps/curl/include/curl/Makefile.in (1)
534-535: LGTM! Improved build conditional logic.The changes correctly combine
DEBUGBUILDandNOT_CURL_CIconditions to control when source code checking runs. This ensureschecksrconly executes during local debug builds, avoiding expensive checks in CI environments while preserving development-time validation.Also applies to: 660-660
deps/curl/lib/curl_sspi.c (2)
31-31: LGTM! Correct include path updates for curlx reorganization.The include directives properly reference the relocated utility headers in the
curlx/subdirectory, maintaining build consistency with the broader libcurl modularization effort.Also applies to: 33-34
132-132: LGTM! Improved comment clarity.The grammar fixes enhance the documentation by clarifying the distinction between "identity structure" and "identifier structure" in the function descriptions.
Also applies to: 224-224
deps/curl/include/curl/system.h (1)
339-340: LGTM! Well-designed compiler-optimized bit manipulation macros.The new
CURL_POPCOUNT64andCURL_CTZ64macros correctly map to GCC built-in functions with appropriate type handling:
- Uses
llsuffix built-ins for 32-bit targets where 64-bit integers arelong long- Uses
lsuffix built-ins for 64-bit targets where 64-bit integers arelongThis provides efficient bit counting and trailing zero operations that can be utilized by other libcurl components.
Also applies to: 351-352
deps/curl/lib/http1.c (2)
47-47: LGTM! Consistent API namespace migration to curlx.The systematic replacement of
Curl_dyn_*functions withcurlx_dyn_*equivalents correctly aligns with the broader libcurl modularization effort. All dynamic buffer operations maintain the same functionality while using the new namespace:
curlx_dyn_init,curlx_dyn_free,curlx_dyn_resetcurlx_dyn_len,curlx_dyn_ptr,curlx_dyn_addn,curlx_dyn_addfThe changes preserve existing logic and maintain API consistency.
Also applies to: 54-54, 111-111, 116-116, 118-118, 121-122, 130-131, 303-303, 325-331, 339-339
212-212: LGTM! Improved terminology in comment.The clarification from "0-termination" to "null-termination" uses more standard and precise terminology for describing string handling requirements.
deps/curl/lib/cshutdn.h (2)
29-29: LGTM: Include path updated for reorganized utility headers.The change from
"timeval.h"to"curlx/timeval.h"aligns with the broader reorganization of utility functions into thecurlxnamespace and subdirectory structure.
78-82: LGTM: New function declaration for connection management.The new
Curl_cshutdn_close_oldestfunction provides selective connection shutdown capability, allowing filtering by destination. The function signature is clear and the documentation comment explains the purpose well.deps/curl/lib/cw-out.c (2)
90-90: LGTM: Dynamic buffer API updated to curlx namespace.All
Curl_dyn_*function calls have been consistently updated tocurlx_dyn_*as part of the utility API modernization. The changes maintain the same functionality while adopting the new naming convention.Also applies to: 98-98, 149-149, 266-266, 270-272, 277-283, 323-323, 353-353
117-117: LGTM: Improved immutability with const qualifier.Making
Curl_cwt_outconst improves code safety by preventing accidental modifications to this global structure.deps/curl/lib/headers.c (4)
32-32: LGTM: Include path updated for reorganized utility headers.The change from
"strparse.h"to"curlx/strparse.h"aligns with the utility function reorganization into thecurlxnamespace and subdirectory structure.
220-220: LGTM: Improved comment terminology.The terminology updates to "null-terminate" provide clearer and more consistent documentation.
Also applies to: 326-326
316-320: Excellent security improvement: Header count limiting.Adding the
MAX_HTTP_RESP_HEADER_COUNTlimit (5000 headers) prevents potential DoS attacks through excessive header counts while providing clear error messaging. This is a critical security enhancement.
339-342: LGTM: Enhanced error handling with proper logging.The addition of error logging with
failf(data, "Invalid response header")before freeing resources improves debugging capabilities and follows good error handling practices by adding braces around the error block.deps/curl/lib/curlx/multibyte.c (3)
26-31: LGTM: Clear documentation of memory allocation approach.The updated comments clearly explain why memory allocation functions are wrapped in parentheses to bypass curl's memory debugging macros, which is necessary for this utility module.
33-33: LGTM: Include paths updated for new curlx location.The relative include paths have been properly updated to reflect the file's new location in the
curlx/subdirectory.Also applies to: 37-37
51-51: LGTM: Consistent memory allocation bypass implementation.All memory allocation and deallocation calls are properly wrapped in parentheses to bypass curl's memory debugging macros (
(malloc),(free)). This ensures the utility functions use the standard C library functions directly, which is appropriate for this mem-include-scan clean module.Also applies to: 55-55, 73-73, 77-77, 139-139, 159-159, 192-192, 205-205, 213-213, 223-223, 237-241, 279-279, 282-282, 315-315, 354-354
deps/curl/lib/getinfo.c (6)
34-34: Align include path withcurlxnamespace.
Replaces the legacy"strparse.h"with"curlx/strparse.h"to reflect the new utility directory structure.
210-210: Usecurlx_str_numberingetinfo_longdebug branches.
Both debug-time numeric parses have been renamed fromCurl_str_numbertocurlx_str_numberfor consistency with the reorganized string-parsing API.Also applies to: 223-223
387-387: Rename parsing call ingetinfo_offtdebug branch.
Replaces the legacyCurl_str_numberwithcurlx_str_numberwhen a debug-override time is set.
425-427: Use boolean flags for content-length ingetinfo_offt.
Switches to checkingdata->progress.dl_size_knownanddata->progress.ul_size_knowndirectly, simplifying the old bitwise-flag logic.Also applies to: 429-431
486-486: Rename parsing call ingetinfo_doubledebug branch.
Updates the debug double-parsing path to callcurlx_str_numberinstead ofCurl_str_number.
537-539: Update content-length handling ingetinfo_double.
Adopts thedl_size_known/ul_size_knownboolean flags and caststotal_sizetodouble, matching the new data model.Also applies to: 541-543
deps/curl/lib/cf-h1-proxy.c (13)
31-31: Include dynamic buffer API header.
Imports"curlx/dynbuf.h"to use the newcurlx_dyn_*buffer functions.
43-43: Addstrcase.hfor case-insensitive compares.
Replaces the previous catch-all header with a dedicated"strcase.h".
47-47: Importcurlx/strparse.hfor parsing helpers.
Aligns this file with the reorganized string-parsing utilities undercurlx.
100-101: Reset dynamic buffers intunnel_reinit.
ReplacesCurl_dyn_resetcalls withcurlx_dyn_resetto clear both receive and request buffers.
126-127: Initialize dynamic buffers intunnel_init.
Switches tocurlx_dyn_initwithDYN_PROXY_CONNECT_HEADERSandDYN_HTTP_REQUESTfor buffer setup.
153-153: Reset buffers on state transitions.
Ensures that entering CONNECT and FAILED states resets bothrcvbufandrequest_dataviacurlx_dyn_reset.Also applies to: 176-177
196-197: Free dynamic buffers intunnel_free.
ReplacesCurl_dyn_freewithcurlx_dyn_freeto properly release allocated buffers.
229-229: Clear request buffer before writing CONNECT.
Usescurlx_dyn_resetto ensure the request buffer is empty prior to appending the new CONNECT headers.
251-252: Retrieve buffer pointer and length viacurlxAPI.
Replaces direct struct access withcurlx_dyn_ptrandcurlx_dyn_lenforrequest_data.
319-319: Usecurlx_str_numblanksin header parsing.
Replaces manual blank-skipping logic with the newcurlxstring parser for Content-Length.
447-447: Append response byte usingcurlx_dyn_addn.
Switches from the old dynamic-buffer append tocurlx_dyn_addnfor consistency.
457-459: Parse header line withcurlx_dyn_ptrandcurlx_dyn_len.
Retrieves the full header line via the new buffer API before dispatching to callbacks.
515-515: Reset receive buffer after header processing.
Clears the dynamic buffer withcurlx_dyn_resetto prepare for the next header.deps/curl/lib/curlx/curlx.h (7)
45-46: Includemultibyte.hfor wide-char conversions.
Adds the new UTF-8 ↔ wide-char helper functions undercurlx.
61-63: Importdynbuf.hfor buffer management.
Exposes thecurlx_dyn_*APIs needed across libcurl code.
64-64: Addbase64.hfor encoding/decoding routines.
Integrates Base64 support into the unifiedcurlxutility set.
65-65: Includetimeval.hfor timestamp helpers.
Consolidates time-of-day functions under thecurlxnamespace.
66-66: Importtimediff.hfor interval computations.
Providescurlx_timediff()and related utilities.
68-69: Addwinapi.hfor Windows API error strings.
Declarescurlx_winapi_strerrorto support consistent error reporting on Windows.
71-73: Includeinet_pton.hfor IP address parsing.
Exposes portablecurlx_inet_ptonfor IPv4/IPv6 conversions.deps/curl/lib/hsts.c (5)
43-43: Update string parsing include path.
Replaces"strparse.h"with"curlx/strparse.h"to use the new parsing helpers.
65-65: Replace numeric parsing withcurlx_str_number.
Uses the newcurlx_str_numberin the debug-time helper, maintaining identical behavior.
157-174: Leveragecurlxparsing helpers inCurl_hsts_parse.
Substitutes manual blank skipping and single-character checks withcurlx_str_passblanks,curlx_str_single, andcurlx_str_numberfor robust directive parsing.
419-423: Usecurlxword-parsing functions inhsts_add.
Switches tocurlx_str_word,curlx_str_singlespace,curlx_str_quotedword, andcurlx_str_newlinefor line tokenization.
431-436: Adoptcurlx_str*and related helpers inhsts_add.
Replaces raw pointer/length operations withcurlx_str,curlx_strlen, andcurlx_str_nudgefor safer host and date extraction.Also applies to: 442-448
deps/curl/lib/conncache.h (2)
29-29: Usecurlx/timeval.hfor time structures.
Updates the include to"curlx/timeval.h", centralizing time utilities under thecurlxnamespace.
67-70:Details
✅ Verification successful
Change
Curl_cpool_initreturn type tovoid.
The pool initialization no longer fails; its signature and documentation have been updated. Ensure all callers have been updated to ignore any return value.Please verify there are no remaining callers expecting an integer result:
🏁 Script executed:
rg -n "Curl_cpool_init" .Length of output: 359
🏁 Script executed:
#!/bin/bash # Inspect caller in share.c around the invocation of Curl_cpool_init sed -n '135,155p' deps/curl/lib/share.c echo "----" # Inspect caller in multi.c around the invocation of Curl_cpool_init sed -n '260,285p' deps/curl/lib/multi.cLength of output: 1240
All callers ignore
Curl_cpool_initreturn value – safe to switch tovoid
No sites assign or check its return value (verified in deps/curl/lib/share.c and deps/curl/lib/multi.c), so updating the signature and docs is safe.deps/curl/lib/curl_setup.h (1)
455-458: LGTM: Well-structured feature detection macro.The
USE_EVENTFDmacro properly checks for both the eventfd function availability and the required header, following standard feature detection patterns in libcurl.deps/curl/include/curl/curlver.h (1)
35-35: Version update appears consistent and correct.All version fields are properly synchronized:
- String version: "8.14.1"
- Numeric components: 8.14.1
- Hex version: 0x080e01 (correctly represents 8.14.1)
The timestamp "2025-06-04" should be verified as it appears to be in the future relative to typical development timelines.
Also applies to: 40-41, 62-62, 73-73
deps/curl/lib/curl_get_line.c (2)
43-43: Function renames align with refactoring effort.The systematic renaming from
Curl_dyn_*tocurlx_dyn_*is consistent with the broader modularization mentioned in the AI summary.Also applies to: 53-53, 64-64
71-72: Enhanced EOF handling improves robustness.The new logic handles the edge case where
fgetsreturns NULL but the buffer contains partial data, treating it as a valid line. This is a reasonable improvement for handling incomplete lines at EOF.deps/curl/lib/dict.c (2)
69-74: Well-organized DICT protocol command definitions.The new macros clearly define the various DICT protocol command prefixes (MATCH, DEFINE, etc.) with their alternative forms, improving code readability and maintainability.
118-118: Function renames consistent with refactoring effort.The systematic renaming from
Curl_dyn_*tocurlx_dyn_*maintains consistency with the broader libcurl modularization effort described in the AI summary.Also applies to: 126-126, 128-128, 132-132
deps/curl/lib/ftp.h (2)
41-41: LGTM! Function declaration follows libcurl conventions.The new
ftp_conns_matchfunction declaration is properly formatted and aligns with the metadata-based refactoring described in the summary.
169-172: LGTM! Metadata keys follow consistent naming patterns.The new metadata keys
CURL_META_FTP_EASYandCURL_META_FTP_CONNfollow libcurl's established naming conventions and support the transition to metadata-based state management for FTP connections.deps/curl/lib/altsvc.c (2)
38-38: LGTM! Header includes updated for curlx reorganization.The header includes have been properly updated to reflect the new
curlx/module structure for utility functions.Also applies to: 42-44
142-144: LGTM! Systematic namespace migration to curlx.All string parsing and utility function calls have been consistently updated from
Curl_*tocurlx_*prefixes. This change maintains the same functionality while aligning with the new utility code organization.Also applies to: 147-148, 169-187, 193-196, 234-242, 264-269, 450-451, 490-494, 503-548, 549-582, 592-594, 607-608, 617-624
deps/curl/lib/formdata.h (1)
45-50: LGTM! Memory-efficient struct optimization.The conversion from
long flagstounsigned char flagsand the consolidation of boolean allocation flags into bitfields using theBIT()macro improves memory efficiency while maintaining the same functionality. This aligns with similar optimizations across the libcurl codebase.deps/curl/lib/curlx/inet_pton.c (3)
21-23: LGTM! Include paths updated for new directory structure.The include paths have been properly updated to reflect the file's new location in the
curlx/directory and to use the new utility headers.
112-113: LGTM! Improved digit detection using utility macros.The replacement of manual digit checking with the
ISDIGIT()macro improves code readability and maintainability while providing the same functionality.
173-175: LGTM! Enhanced hex digit handling with utility functions.The use of
ISXDIGIT()andCurl_hexval()instead of manual hex character detection and conversion improves code clarity and leverages existing utility functions.deps/curl/lib/curl_printf.h (2)
33-37: LGTM: Well-documented external digit arrays.The addition of
Curl_ldigitsandCurl_udigitsexternal arrays with clear comments follows good naming conventions and provides necessary utility for hex encoding/decoding functions mentioned in the AI summary.
39-63: LGTM: Improved macro scoping prevents external conflicts.The conditional scoping of printf macro redefinitions to
#ifdef BUILDING_LIBCURLis a good practice that prevents macro pollution in external code while maintaining internal consistency. The unconditional inclusion ofcurl/mprintf.hensures availability across all build configurations.deps/curl/lib/hostip6.c (3)
52-52: LGTM: Include path updated for curlx namespace.The update from
"inet_pton.h"to"curlx/inet_pton.h"correctly reflects the utility code reorganization into the curlx namespace mentioned in the AI summary.
86-89: LGTM: Function signature modernized for sync resolver.The rename to
Curl_sync_getaddrinfoand parameter change fromint *waitptoint ip_versioncorrectly aligns with the DNS resolver refactoring. Synchronous functions don't need wait flags, and the IP version parameter provides better control over resolution behavior.
101-103: LGTM: Proper IP version handling.The logic correctly uses the
ip_versionparameter to determine protocol family, maintaining IPv6 capability when appropriate while defaulting to IPv4 compatibility.deps/curl/lib/easyoptions.c (3)
31-31: LGTM: Improved const correctness.Declaring
Curl_easyoptsasconstis correct since this array should be immutable, improving code safety and aligning with the const declaration in the corresponding header file.
320-321: LGTM: New SSL option properly integrated.The
SSL_SIGNATURE_ALGORITHMSoption is correctly added with appropriate typeCURLOT_STRINGand follows the existing pattern. This expands TLS configuration capabilities as mentioned in the AI summary.
383-383: LGTM: Validation count correctly updated.The validation logic correctly updated from
(327 + 1)to(328 + 1)to account for the new SSL_SIGNATURE_ALGORITHMS option.deps/curl/lib/hostip4.c (3)
61-61: LGTM: Function comment updated correctly.The comment correctly reflects the new function name
Curl_sync_getaddrinfoand maintains the IPv4 synchronous version designation.
76-79: LGTM: Function signature aligned with sync resolver pattern.The rename to
Curl_sync_getaddrinfoand parameter change fromwaitptoip_versioncorrectly aligns with the DNS resolver modernization, matching the pattern used inhostip6.c.
83-83: LGTM: Proper unused parameter handling.The
(void)ip_version;statement correctly suppresses compiler warnings for the unused parameter, which is appropriate since IPv4-only builds don't need to differentiate IP versions.deps/curl/lib/cookie.c (3)
90-90: LGTM! Include path updated correctly.The include path change from
"strparse.h"to"curlx/strparse.h"aligns with the library's reorganization of utility headers into thecurlxsubdirectory.
500-532: LGTM! Consistent namespace updates for string parsing functions.All string parsing functions have been correctly updated from
Curl_str_*tocurlx_str_*prefix, maintaining the same functionality while following the new namespace convention.
1213-1226: LGTM! Dynamic buffer functions updated to new namespace.The dynamic buffer functions have been correctly updated from
Curl_dyn_*tocurlx_dyn_*prefix, maintaining consistency with the library-wide namespace reorganization.deps/curl/lib/curl_sasl.c (3)
45-52: LGTM! Include paths updated for curlx namespace.The header includes have been correctly updated to reflect the new
curlxsubdirectory structure for base64 and warnless utilities.
286-310: LGTM! Base64 function calls updated to new namespace.The base64 encode/decode function calls have been correctly updated from
Curl_base64_*tocurlx_base64_*, maintaining consistency with the library-wide namespace reorganization.
760-854: Verify error handling in the new diagnostic function.The new
Curl_sasl_is_blockedfunction provides helpful diagnostic information about failed SASL authentication. However, it always returnsCURLE_LOGIN_DENIEDregardless of the actual failure reason.Consider whether different error codes should be returned based on the specific failure reason (e.g.,
CURLE_NOT_BUILT_INfor unsupported mechanisms) to provide more granular error handling to callers.deps/curl/lib/fake_addrinfo.h (1)
1-55: LGTM! Well-structured debug interface for DNS resolution.The new header file is properly structured with:
- Correct header guards
- Proper conditional compilation based on debug mode, c-ares usage, and version requirements
- Clear function declarations for the fake getaddrinfo interface
- Appropriate includes for networking headers
This debug interface will be useful for testing DNS resolution behavior.
deps/curl/lib/curlx/winapi.h (1)
1-33: LGTM! Well-structured Windows API utility header.The implementation follows curl's coding standards with proper include guards, copyright notice, and platform-specific conditional compilation. The API design is clean with const correctness and a reasonable error buffer size.
deps/curl/lib/cfilters.c (5)
37-38: LGTM! Header includes updated for curlx reorganization.The header includes are correctly updated to use the new
curlx/prefix, aligning with the broader reorganization of utility functions.
201-201: LGTM! Function call updated to curlx namespace.The timing function call is correctly updated from
Curl_now()tocurlx_now(), consistent with the utility function reorganization.
445-445: LGTM! Consistent timing function update.Another correct update from
Curl_now()tocurlx_now()maintaining consistency across the codebase.
500-503: LGTM! Clean utility function implementation.The new
Curl_conn_is_setup()function has a clear, simple implementation that checks if a connection filter chain exists. The function name and logic are self-explanatory.
949-949: LGTM! String parsing function updated correctly.The function call is properly updated from
Curl_str_number()tocurlx_str_number(), following the utility function reorganization pattern.deps/curl/lib/curl_threads.c (3)
83-89: LGTM! Improved thread handle management for POSIX.The function signature change to accept a pointer allows proper nullification of the caller's handle after cleanup, preventing use-after-free issues. The implementation correctly dereferences the pointer, detaches the thread, frees memory, and sets the handle to null.
140-145: LGTM! Consistent Windows implementation.The Windows version follows the same pattern as the POSIX implementation, ensuring consistent behavior across platforms. The handle is properly closed and nullified.
157-157: LGTM! Updated call aligns with new function signature.The call to
Curl_thread_destroyis correctly updated to pass the pointer directly, since the function now handles nullification internally.deps/curl/lib/escape.c (6)
35-35: LGTM! Header includes updated for curlx reorganization.The includes are correctly updated to use the
curlx/prefix for utility headers, consistent with the broader reorganization.Also applies to: 38-38
74-74: LGTM! Dynamic buffer functions updated consistently.All dynamic buffer function calls are properly updated from
Curl_dyn_*tocurlx_dyn_*, maintaining consistency with the utility function reorganization.Also applies to: 82-82, 89-89, 94-94
87-88: LGTM! Cleaner hex encoding implementation.The manual hex encoding is replaced with a call to the new
Curl_hexbyte()function, which provides a cleaner and more maintainable approach. TheFALSEparameter correctly specifies uppercase hex output.
137-138: LGTM! Hex decoding updated to use utility function.The hex digit conversion is updated to use
Curl_hexval()from the utility library, removing code duplication and improving maintainability.
215-215: LGTM! Consistent use of new hex function.The
Curl_hexencode()function is updated to useCurl_hexbyte()with theTRUEparameter for lowercase output, maintaining the function's existing behavior while improving code reuse.
226-238: LGTM! Well-implemented hex byte function.The new
Curl_hexbyte()function is cleanly implemented with:
- Clear parameter documentation
- Proper const correctness
- Support for both uppercase and lowercase output
- Efficient bit manipulation for hex conversion
This centralizes hex encoding logic and improves code maintainability.
deps/curl/lib/curl_addrinfo.c (3)
53-55: Header reorganization looks good.The updates align with the broader reorganization of utility headers into the
curlxsubdirectory. The newfake_addrinfo.hinclude supports the new async resolver implementation mentioned in the PR summary.
471-471: Comment terminology standardization approved.The change from "NUL-terminated" to "null-terminated" improves consistency with standard terminology.
545-551: Conditional compilation logic is consistent.The same environment variable check pattern is correctly applied in both debug wrapper functions, maintaining consistency in the fake getaddrinfo integration.
deps/curl/lib/Makefile.am (4)
82-82: Path updates for curlx reorganization approved.The updated exclusion paths correctly reflect the movement of utility files into the
curlx/subdirectory.
84-87: Simplified debug exclusion logic looks good.The cleanup removes unnecessary exclusions while preserving the essential
memdebug.cexclusion for debug builds to avoid header redefinition issues.
157-162: CI conditional makes sense.Adding the
NOT_CURL_CIconditional prevents source checks from running in CI environments, which can improve build performance and avoid unnecessary failures.
54-55:Details
✅ Verification successful
Verify the removal of private source directory inclusion.
Removing
$(top_srcdir)/libfromAM_CPPFLAGSmeans internal headers will no longer be automatically found. This could break builds if any source files rely on this include path.
🏁 Script executed:
#!/bin/bash # Description: Check for potential build issues from removing $(top_srcdir)/lib include path # Look for includes that might rely on the removed path rg -n '#include\s+"[^/]*\.h"' deps/curl/lib/ | head -20 # Check if any internal headers are included without relative paths ast-grep --pattern $'#include "$_"' deps/curl/lib/ | grep -v "curlx/" | head -10Length of output: 1225
AM_CPPFLAGS Update Is Safe
All internal headers in deps/curl/lib (e.g. curl_setup.h, bufq.h, http.h, etc.) are now included via -I$(top_builddir)/lib, which mirrors the source lib directory during the build. The removal of -I$(top_srcdir)/lib does not break any header lookups.
No further action required.
deps/curl/lib/curlx/base64.h (2)
27-33: API simplification approved.Removing the macro indirection and declaring the
curlx_base64_*functions directly improves code clarity and aligns with the curlx namespace unification mentioned in the PR summary.
34-34: New constant declaration looks appropriate.The
Curl_base64encdecexternal constant declaration appears to be part of the base64 implementation reorganization.deps/curl/lib/connect.h (2)
28-30: Include path updates approved.The updates to use
curlx/nonblock.handcurlx/timeval.hcorrectly align with the utility header reorganization into the curlx subdirectory.
140-140:Details
✅ Verification successful
Verify function signature change consistency.
The change from
const struct Curl_dns_entry *remotehosttostruct Curl_dns_entry *dnsrepresents a significant API modification. This should be verified to ensure all callers have been updated accordingly.
🏁 Script executed:
#!/bin/bash # Description: Verify all calls to Curl_conn_setup have been updated to match the new signature # Search for function calls to Curl_conn_setup rg -A 5 -B 2 "Curl_conn_setup\s*\(" deps/curl/lib/ # Look for any remaining references to remotehost parameter rg "remotehost" deps/curl/lib/ | grep -v "\.orig" | head -10Length of output: 1866
Signature change consistency verified
All calls to the
Curl_conn_setupfunction indeps/curl/libnow use the updatedstruct Curl_dns_entry *dnsparameter, and there are no remaining references toremotehost. No further changes are required.deps/curl/lib/curlx/winapi.c (1)
71-71: Verify buffer size for wcstombs conversion.The code passes
buflen - 1towcstombsbut the wbuf is fixed at 256 wide characters. When converted to multibyte, this could potentially require more bytes than the original wide character count.Consider adding a bounds check to ensure the converted string fits in the buffer, especially for multi-byte character encodings where one wide character can expand to multiple bytes.
deps/curl/lib/fake_addrinfo.c (1)
107-107:Details
❌ Incorrect review comment
Potential integer truncation in ai_addrlen assignment.
The code assigns
ss_size(of typesize_t) toai_addrlen(of typecurl_socklen_t) which could lead to truncation on platforms wheresize_tis larger thancurl_socklen_t.
🏁 Script executed:
#!/bin/bash # Check the definition of curl_socklen_t and potential size differences ast-grep --pattern 'typedef $_ curl_socklen_t' rg -A 3 "curl_socklen_t" --type c | head -20Length of output: 48
🏁 Script executed:
#!/bin/bash echo "Searching for curl_socklen_t in deps/curl directory" rg -n "curl_socklen_t" deps/curl echo "Searching for typedef for curl_socklen_t in header files under deps/curl" rg -n "typedef.*curl_socklen_t" deps/curl echo "Searching for socklen_t usage in deps/curl" rg -n "socklen_t" deps/curlLength of output: 9114
Ignore truncation warning; this cast is intentional and safe
curl_socklen_tis defined assocklen_t, and all socket address lengths (ss_size) are well within its range. The explicit cast is used consistently throughout the code and poses no practical risk of truncation.Likely an incorrect or invalid review comment.
deps/curl/lib/cf-https-connect.h (1)
38-51:Details
❓ Verification inconclusive
Verify all callers have been updated for the API change.
The removal of the
remotehostparameter from these functions is a breaking API change that requires all callers to be updated.Run the following script to verify all function calls have been updated:
🏁 Script executed:
#!/bin/bash # Search for any remaining calls to these functions with the old signature echo "Checking for calls to Curl_cf_http_connect_add with 5 parameters:" ast-grep --pattern 'Curl_cf_http_connect_add($_, $_, $_, $_, $_)' echo -e "\nChecking for calls to Curl_cf_http_connect_insert_after with 5 parameters:" ast-grep --pattern 'Curl_cf_http_connect_insert_after($_, $_, $_, $_, $_)' echo -e "\nChecking for calls to Curl_cf_https_setup with 4 parameters:" ast-grep --pattern 'Curl_cf_https_setup($_, $_, $_, $_)' # Also check with ripgrep for any pattern matches echo -e "\nDouble-checking with ripgrep:" rg "Curl_cf_http_connect_add\s*\([^)]*,[^)]*,[^)]*,[^)]*,[^)]*\)" --type c rg "Curl_cf_https_setup\s*\([^)]*,[^)]*,[^)]*,[^)]*\)" --type cLength of output: 208
To ensure the script doesn’t abort on the first “no‐match” and that all functions (including
Curl_cf_http_connect_insert_after) are checked, please rerun the following:
🏁 Script executed:
#!/bin/bash set +e echo "1) Verifying old‐signature calls to Curl_cf_http_connect_add (5 params):" ast-grep --pattern 'Curl_cf_http_connect_add($_, $_, $_, $_, $_)' || echo " → no deprecated calls found" echo -e "\nListing all occurrences of Curl_cf_http_connect_add:" rg -n --type c "Curl_cf_http_connect_add\s*\(" || echo " → none" echo -e "\n2) Verifying old‐signature calls to Curl_cf_http_connect_insert_after (5 params):" ast-grep --pattern 'Curl_cf_http_connect_insert_after($_, $_, $_, $_, $_)' || echo " → no deprecated calls found" echo -e "\nListing all occurrences of Curl_cf_http_connect_insert_after:" rg -n --type c "Curl_cf_http_connect_insert_after\s*\(" || echo " → none" echo -e "\n3) Verifying old‐signature calls to Curl_cf_https_setup (4 params):" ast-grep --pattern 'Curl_cf_https_setup($_, $_, $_, $_)' || echo " → no deprecated calls found" echo -e "\nListing all occurrences of Curl_cf_https_setup:" rg -n --type c "Curl_cf_https_setup\s*\(" || echo " → none"Length of output: 751
Confirm removal of deprecated API calls
Automated checks didn’t find any uses of the old‐signature functions other than their declaration in the header. Please manually verify that no call sites still pass the removedremotehostparameter—for example, by grepping your.c/.hfiles (excluding the header itself):rg -R --type-add 'src:c,h' --type src \ "Curl_cf_http_connect_add\(" --glob '!deps/curl/lib/cf-https-connect.h' rg -R --type-add 'src:c,h' --type src \ "Curl_cf_http_connect_insert_after\(" --glob '!deps/curl/lib/cf-https-connect.h' rg -R --type-add 'src:c,h' --type src \ "Curl_cf_https_setup\(" --glob '!deps/curl/lib/cf-https-connect.h'Once you confirm that no call sites remain with the old parameter list, this API change is fully adopted.
deps/curl/lib/content_encoding.c (1)
769-783: Order/validation logic still allows TE after ‘chunked’ whenidentityis presentThe special-case for
identity(lines 769-781) allows “chunked, identity”.
RFC 9112 §6.1 disallows any transfer-coding afterchunked, includingidentity.
Either:
- Reject any list member that follows
chunked, or- Confirm the spec intent and document the deviation.
Otherwise libcurl may accept a response that intermediates will drop.
deps/curl/lib/curl_rtmp.c (1)
321-326:Curl_conn_meta_remove()already frees – avoid double-free paranoiaAfter
Curl_conn_meta_remove()the destructor runs; do not accessrafterward to avoid use-after-free if future changes add logic below.deps/curl/lib/curl_config.h.cmake (1)
200-203: Feature-test macros added – remember to gate syscall usage
accept4andpipe2are now detected.
Verify that any new code using them is correctly wrapped in#ifdef HAVE_ACCEPT4/HAVE_PIPE2to avoid build failures on older kernels.Also applies to: 433-435
deps/curl/lib/cf-socket.c (3)
77-87: LGTM! Good namespace reorganization.The header includes have been properly updated to use the
curlx/subdirectory structure, which aligns with the broader refactoring effort to modularize utility functions.
677-684: Good refactoring: Simplified IP version handling.The code now uses a local
ip_versionvariable instead of temporarily modifyingconn->ip_version, which makes the logic cleaner and avoids potential side effects.
2141-2156: Excellent use of accept4() for atomic socket flag setting.The implementation properly uses
accept4()withSOCK_NONBLOCK | SOCK_CLOEXECflags when available, which atomically sets these flags during socket creation. This avoids race conditions and is more efficient than setting flags after creation. The fallback to regularaccept()with subsequentcurlx_nonblock()call is correctly implemented.deps/curl/lib/cshutdn.c (2)
44-44: Time function updates are consistent.The time-related functions have been properly updated to use the
curlx_prefix throughout the file.Also applies to: 272-272, 290-290, 313-313, 363-363
164-166: Enhanced logging provides better debugging information.The improved logging now shows whether shutdown is forced or filtered, along with the connection ID, which will help with debugging connection lifecycle issues.
deps/curl/lib/curlx/timeval.h (3)
27-27: Correct header path adjustment for new directory structure.The include path has been properly updated to reflect the new
curlx/subdirectory structure.
36-39: Platform-specific initialization properly scoped.The
curlx_now_init()function is now correctly declared only for Windows platforms where it's needed.
41-41: Function declarations properly renamed to curlx_ prefix.All timing function declarations have been consistently renamed from
Curl_*tocurlx_*, which aligns with the broader namespace reorganization effort. The removal of macro aliasing makes the API cleaner and more direct.Also applies to: 49-49, 57-57, 65-65
deps/curl/lib/ftplistparser.c (1)
321-333:curlx_dyn_ptr()integration looks correctGood catch switching from the deprecated
Curl_dyn_ptrhelper to its new
curlxnamespaced equivalent; the surrounding offset bookkeeping remains
unchanged, so no functional regression is introduced here.deps/curl/lib/curl_config.h.in (1)
167-169: New feature-detection macros added cleanlyThe additions for
HAVE_ACCEPT4,HAVE_PIPE2, QUIC helpers, and new
wolfSSL APIs follow the existing pattern and won’t disrupt autoheader
generation. 👍Also applies to: 504-506, 594-596, 724-742, 770-772
deps/curl/curl.gyp (4)
10-11: New async resolver implementation files added.The addition of
asyn-base.candasyn-thrdd.caligns with the libcurl refactoring to unify async resolver backends under a new interface. These files replace older async resolver implementations.
47-47: New fake_addrinfo utility added.Addition of
fake_addrinfo.cprovides new utility functionality as part of the libcurl modularization effort.
122-125: New uint utility files added.The uint-*.c files (
uint-bset.c,uint-hash.c,uint-spbset.c,uint-table.c) expand utility functionality for handling unsigned integer operations and data structures.
129-139: Curlx utility modules successfully reorganized.The curlx/*.c files represent the modularization of libcurl's internal utility code into a dedicated
curlxnamespace and directory. This includes base64, dynamic buffers, string parsing, timing functions, and other utilities that were previously scattered in the main lib directory.deps/curl/lib/Makefile.inc (5)
25-36: Curlx source files properly modularized.The new
LIB_CURLX_CFILESgroup correctly organizes the utility source files that were moved to thecurlx/subdirectory. This modularization improves code organization and maintainability.
38-50: Curlx header files properly modularized.The new
LIB_CURLX_HFILESgroup correctly organizes the utility header files corresponding to the curlx source files, maintaining proper separation between interface and implementation.
141-142: Async resolver files consistent with build configuration.The addition of
asyn-base.candasyn-thrdd.cin the main source list matches the changes incurl.gyp, ensuring build consistency across different build systems.
258-261: Uint utility files properly included.The new uint-*.c files are correctly added to the main source list, consistent with the corresponding changes in
curl.gyp.
403-405: Curlx modules properly integrated into build system.The addition of
$(LIB_CURLX_CFILES)and$(LIB_CURLX_HFILES)to theCSOURCESandHHEADERSlists ensures that the new curlx utility modules are properly included in the overall build process.deps/curl/lib/http.c (6)
57-57: LGTM!The migration to
curlxnamespace for include headers is consistent with the broader project-wide refactoring effort.Also applies to: 79-79, 88-88
288-293: Consistent migration to curlx string utilities.The systematic replacement of
Curl_str_*andCurl_dyn_*functions with theircurlx_*counterparts is complete and correct. This aligns with the broader project-wide migration of utility functions to thecurlxnamespace.Also applies to: 1076-1076, 1724-1730, 1807-1807, 2831-2859, 2783-2783, 2787-2787, 2795-2795, 2803-2803, 2810-2904, 3875-3889, 4025-4025, 4051-4051, 4060-4061, 4119-4121, 4125-4125, 4140-4141, 4178-4186, 4349-4349, 4295-4298, 4319-4320, 4326-4327, 4331-4336, 4339-4339, 4361-4362, 4379-4385, 4388-4388, 4397-4397
1076-1078: Fixed missing return statement.Good catch! The missing return statement after processing all authentication tokens has been properly added.
1179-1180: Improved HTTP redirect error handling.The enhanced redirect logic now properly:
- Captures the result of
Curl_req_soft_reset- Tracks method changes to GET on 301/302/303 redirects
- Returns rewind errors when upload data cannot be rewound and the method wasn't switched to GET
This prevents potential data corruption or request failures when redirects occur during uploads.
Also applies to: 1389-1390, 1414-1415, 1430-1431, 1448-1453
4461-4510: RFC 9113 compliant TE header handling.The changes correctly implement RFC 9113 section 8.2.2 requirements:
- Removed "TE" from the blanket list of prohibited HTTP/2 headers
- Added proper parsing to allow "TE: trailers" specifically
- The
http_TE_has_tokenfunction properly handles quoted strings and follows HTTP syntaxThis allows HTTP/2 connections to properly use trailer fields when needed.
4666-4666:Details
✅ Verification successful
Verify time function naming consistency.
The code uses
curlx_now()for getting current time. Please verify this is the intended new name for the time function in the curlx namespace refactoring, as it differs from the typicalcurlx_prefix pattern seen with other utilities.Also applies to: 4678-4678
🏁 Script executed:
#!/bin/bash # Search for other occurrences of time-related functions in the curlx namespace rg -A 2 'curlx_(now|time)' --type cLength of output: 32525
Time function naming is consistent – no changes required.
All occurrences of the time utilities use
curlx_now()as the designated “current time” function, with its prototype declared indeps/curl/lib/curlx/timeval.h. There is no othercurlx_timefunction, and related helpers (curlx_timediff, etc.) follow the same naming.deps/curl/lib/doh.h (3)
83-96: LGTM! Well-structured DoH request/response data types.The new
doh_requeststructure clearly encapsulates DoH probe request data with appropriate fields for request body, headers, response buffer, and DNS type. The fixed-size request body array is appropriately sized based on RFC specifications.
98-113: LGTM! Clean separation of response and probe state.The refactoring separates response data (
doh_response) from the overall probe state (doh_probes), providing better encapsulation and clearer ownership of data. The array of responses indoh_probesaligns well with theDOH_SLOT_COUNTenum.
122-122: LGTM! Consistent API update for IP version parameter.The addition of the
ip_versionparameter toCurl_dohand the corresponding macro update are properly aligned. This allows callers to specify IPv4/IPv6 preference for DNS resolution.Also applies to: 186-186
deps/curl/lib/curlx/timeval.c (1)
50-50: LGTM! Consistent function renaming to curlx_ prefix.All timing functions have been consistently renamed from
Curl_*tocurlx_*, aligning with the module reorganization. The internal logic remains unchanged, preserving functionality while improving namespace organization.Also applies to: 85-85, 159-159, 187-187, 204-204, 223-223, 237-237, 251-251
deps/curl/lib/curlx/dynbuf.h (2)
39-62: LGTM! Consistent API renaming for dynamic buffer functions.All dynamic buffer functions have been systematically renamed from
Curl_dyn_*tocurlx_dyn_*, maintaining the same signatures and semantics. This aligns with the broader modularization effort.
83-84: LGTM! Appropriate size limits for certificate and key files.The new constants
DYN_CERTFILE_SIZEandDYN_KEYFILE_SIZEset at 100 KiB provide reasonable limits for certificate and key file buffers, preventing excessive memory usage while accommodating typical certificate chains.deps/curl/lib/easy.c (1)
69-70: Include path moved tocurlx/– verify remaining referencesThe file now pulls
"curlx/warnless.h"and"curlx/dynbuf.h".
Ensure all other translation units were updated; a stale#include "warnless.h"
elsewhere will break the build on some platforms.Also applies to: 76-77
deps/curl/lib/asyn-base.c (2)
60-93: Async timeout wiring looks correct – nothing to flag.The
Curl_ares_getsockimplementation mirrors upstream logic, caps timeouts viaCurl_expire, and returns the ares-calculated bitmap. No correctness, style, or performance issues spotted here.
104-158: Good defensive polling loop.
Curl_ares_performcleanly builds thepollfdarray, limits the poll to the number of active fds, and guarantees a finalares_process_fdcall even on pure time-out – avoiding resolver hangs. No red flags.deps/curl/lib/cf-https-connect.c (1)
660-706: Potential null-dereference on DNS HTTPS-RR path.
data->state.dns[sockindex]is accessed without validating that
sockindexis within bounds (or non-negative) for thedns[]array.
While current call-sites likely pass 0/1, adding a defensive range check
here would harden the code against future misuse.- struct Curl_dns_entry *dns = data->state.dns[sockindex]; + struct Curl_dns_entry *dns = NULL; + if(sockindex >= 0 && + (unsigned)sockindex < CURL_ARRAYSIZE(data->state.dns)) + dns = data->state.dns[sockindex];deps/curl/lib/curlx/dynbuf.c (1)
60-66: Free-on-error path is sound – nice catch.
curlx_dyn_free()zeroes bothlengandallcafter releasing memory,
preventing double-free and stale-length mishaps. Good defensive coding.deps/curl/lib/formdata.c (1)
81-89:CURL_HTTPPOST_LARGEis now set unconditionally – this changes public behaviour
post->flagsused to mirror the caller-supplied flags verbatim.
By OR-ingCURL_HTTPPOST_LARGEhere every part is flagged as “large”
even when the user did not request it, which:
- makes libcurl treat all
contentlenfields ascurl_off_t,- changes the HTTP headers emitted for every form part,
- can break existing applications that rely on the previous semantics.
- post->flags = src->flags | CURL_HTTPPOST_LARGE; + post->flags = src->flags; /* keep caller intent */Please double-check whether this flag promotion is truly intended; if so,
update the documentation to warn users about the new implicit behaviour.
Otherwise, revert as above.deps/curl/lib/curlx/strparse.c (4)
25-26: LGTM! Include path properly updated for reorganized directory structure.The relative path change from
"strparse.h"to"../strcase.h"correctly reflects the file reorganization into thecurlxsubdirectory.
28-32: Function renaming looks good.The rename from
Curl_str_inittocurlx_str_initis consistent with the namespace reorganization.
173-174: Good use of the new Curl_hexval macro.The code correctly uses
Curl_hexval(*p++)instead of manual hex digit extraction, which improves code consistency and maintainability.Also applies to: 181-182
235-243: Conditional compilation looks appropriate.The
curlx_str_casecomparefunction is properly guarded with#ifndef WITHOUT_LIBCURL, which makes sense as it depends onstrncasecomparefrom the libcurl library.deps/curl/lib/curlx/strparse.h (3)
26-26: Include path correctly updated.The relative path change properly reflects the header file reorganization.
105-110: Well-documented hex conversion utilities.The new
Curl_hexasciitablearray andCurl_hexvalmacro are properly documented with a clear warning about valid input requirements. The implementation using bitwise AND with 0x0f to extract the lower 4 bits is correct.
45-103: Consistent function renaming throughout the header.All function declarations have been systematically renamed from
Curl_str*tocurlx_str*, maintaining API consistency with the namespace reorganization.deps/curl/lib/connect.c (6)
70-71: Include paths properly updated for reorganized headers.The header includes have been updated to use the
curlx/subdirectory for reorganized utility headers.Also applies to: 73-74, 77-77
136-137: Timing function updates are consistent.All timing-related calls have been properly updated from
Curl_now()andCurl_timediff()to theircurlx_prefixed equivalents throughout the file.Also applies to: 142-143, 153-154
792-795: Verify error handling for missing DNS entry.Good defensive programming by checking if
dnsis NULL and returningCURLE_FAILED_INIT. This prevents potential null pointer dereferences.
1267-1268: Centralized DNS entry management looks correct.The DNS entry is now properly retrieved from
data->state.dns[cf->sockindex]with appropriate null checks. This centralization reduces parameter passing and simplifies the API.Also applies to: 1276-1277
1506-1508: Good DNS reference management.The code properly manages DNS entry references by:
- Unlinking the old DNS entry before assigning a new one
- Unlinking on error to prevent leaks
This ensures proper reference counting for DNS entries.Also applies to: 1528-1529
1183-1185:Details
✅ Verification successful
Memory management change needs verification.
The change from
Curl_safefree()to plainfree()with null checks is concerning.Curl_safefree()likely sets the pointer to NULL after freeing, which prevents use-after-free bugs.Let me verify if there are any safety implications of this change:
Also applies to: 1438-1440
🏁 Script executed:
#!/bin/bash # Check the definition of Curl_safefree to understand what safety it provides rg -A 5 "Curl_safefree" --glob "*.h" # Check if ctx is accessed after free in these functions ast-grep --pattern 'cf_happy_eyeballs_create($$$) { $$$ free(ctx); $$$ }' ast-grep --pattern 'cf_setup_create($$$) { $$$ free(ctx); $$$ }'Length of output: 730
To fully assess whether
ctxis ever used after being freed (and thus if replacingCurl_safefree()withfree()is safe), let’s pull the surrounding code for context:
🏁 Script executed:
#!/bin/bash # Show the definition and uses of Curl_safefree in connect.c rg -n "Curl_safefree" -A5 -B5 deps/curl/lib/connect.c # Show all free(ctx) occurrences with context rg -n "free(ctx)" -A5 -B5 deps/curl/lib/connect.c # Dump a larger slice around line 1185 to see the full cleanup block sed -n '1150,1250p' deps/curl/lib/connect.cLength of output: 3777
No issues found in the ctx free on error path
In the error‐out block of cf_happy_eyeballs_create,
ctxis a local variable that isn’t used after being freed, so a plainfree(ctx)is sufficient. The only place you need the null‐assignment behavior ofCurl_safefreeis for*pcf, which still usesCurl_safefree(*pcf)to prevent any dangling-use by the caller. No further changes are needed here.deps/curl/lib/asyn-ares.c (1)
279-284: Array-bounds contract unclear afternumsockremoval
Curl_async_getsock()no longer receives the size of thesocksarray, yet it
still delegates toCurl_ares_getsock()which internally assumesMAX_SOCKSPEREASY
(typically 2). If the caller allocates a smaller array the write will overrun
the buffer.Document the implicit upper bound (or re-introduce a
numsock
parameter) and add anassert(numsock >= MAX_SOCKSPEREASY)inside the helper.deps/curl/lib/hostip.h (1)
92-99:allowDOHparameter kept although DoH is handled elsewhere
Curl_resolv()still exposesallowDOH, but the new async abstraction moves
DoH handling into resolver back-ends. Verify that this flag is still honoured;
otherwise remove it to avoid dead parameters and inconsistent behaviour.deps/curl/lib/hostip.c (1)
820-823: Early failure whendata->multiis not yet attached
Curl_resolv()aborts withCURLE_COULDNT_RESOLVE_HOSTif
dnscache_get()returnsNULL, which happens before the easy handle is
added to a multi handle (e.g. duringCURLOPT_RESOLVEprocessing or
unit tests). Previous versions fell back to a per-easy cache.Request to restore the per-easy fallback or delay the NULL check until
the handle is guaranteed to be attached to a multi.
| #elif defined(USE_FAKE_GETADDRINFO) | ||
| { | ||
| const char *env = getenv("CURL_DNS_SERVER"); | ||
| if(env) | ||
| r_freeaddrinfo(freethis); | ||
| else | ||
| freeaddrinfo(freethis); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify environment variable usage in debug wrapper.
The integration of the fake getaddrinfo resolver backend into the debug wrapper is functionally correct. However, the use of getenv("CURL_DNS_SERVER") in a library function should be verified for thread safety and security implications.
🏁 Script executed:
#!/bin/bash
# Description: Check if CURL_DNS_SERVER environment variable usage is documented and consistent across the codebase
# Search for CURL_DNS_SERVER usage patterns
rg -A 3 -B 3 "CURL_DNS_SERVER"
# Check for documentation about this environment variable
fd -e md -e txt -e rst | xargs rg -l "CURL_DNS_SERVER" || echo "No documentation found"Length of output: 1769
Ensure thread safety and document CURL_DNS_SERVER usage
I located three direct calls to getenv("CURL_DNS_SERVER") with no accompanying docs:
- deps/curl/lib/fake_addrinfo.c
- deps/curl/lib/curl_addrinfo.c (lines 512–519)
- deps/curl/lib/asyn-ares.c
Because getenv isn’t guaranteed to be thread-safe on all platforms, invoking it inside library functions can lead to data races in multithreaded applications. To address this:
- Read and cache
CURL_DNS_SERVERonce during library initialization (before any threads are spawned), or serialize access via a mutex. - Add a section for
CURL_DNS_SERVERin your environment-variable documentation (format, behavior, platform support). - Consider exposing an API call to set custom DNS servers at runtime instead of relying solely on an env var.
🤖 Prompt for AI Agents
In deps/curl/lib/curl_addrinfo.c around lines 512 to 519, the
getenv("CURL_DNS_SERVER") call is used directly inside a library function, which
is not thread-safe and can cause data races. To fix this, modify the code to
read and cache the CURL_DNS_SERVER environment variable once during library
initialization before any threads start, then use the cached value here instead
of calling getenv. Additionally, add documentation for CURL_DNS_SERVER in the
environment-variable docs and consider providing an API to set custom DNS
servers at runtime rather than relying solely on the environment variable.
| const char *env = getenv("CURL_DNS_SERVER"); | ||
| if(env) { | ||
| rc = ares_set_servers_ports_csv(channel, env); | ||
| if(rc) { | ||
| fprintf(stderr, "ares_set_servers_ports_csv failed: %d", rc); | ||
| /* Cleanup */ | ||
| ares_destroy(channel); | ||
| return EAI_MEMORY; /* we can't run */ | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for DNS server configuration.
The code prints an error message to stderr but returns a generic EAI_MEMORY error code which doesn't accurately represent the configuration error.
Consider returning a more appropriate error code or logging the error through curl's error handling mechanism:
else {
const char *env = getenv("CURL_DNS_SERVER");
if(env) {
rc = ares_set_servers_ports_csv(channel, env);
if(rc) {
- fprintf(stderr, "ares_set_servers_ports_csv failed: %d", rc);
+ fprintf(stderr, "ares_set_servers_ports_csv failed: %d\n", rc);
/* Cleanup */
ares_destroy(channel);
- return EAI_MEMORY; /* we can't run */
+ return EAI_SYSTEM; /* Configuration error */
}
}
}📝 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.
| const char *env = getenv("CURL_DNS_SERVER"); | |
| if(env) { | |
| rc = ares_set_servers_ports_csv(channel, env); | |
| if(rc) { | |
| fprintf(stderr, "ares_set_servers_ports_csv failed: %d", rc); | |
| /* Cleanup */ | |
| ares_destroy(channel); | |
| return EAI_MEMORY; /* we can't run */ | |
| } | |
| } | |
| } | |
| const char *env = getenv("CURL_DNS_SERVER"); | |
| if(env) { | |
| rc = ares_set_servers_ports_csv(channel, env); | |
| if(rc) { | |
| fprintf(stderr, "ares_set_servers_ports_csv failed: %d\n", rc); | |
| /* Cleanup */ | |
| ares_destroy(channel); | |
| return EAI_SYSTEM; /* Configuration error */ | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In deps/curl/lib/fake_addrinfo.c around lines 177 to 187, the error handling for
ares_set_servers_ports_csv failure currently prints to stderr and returns a
generic EAI_MEMORY error code, which is misleading. Update the code to return a
more appropriate error code that reflects a configuration failure, such as
EAI_FAIL or a custom error code if available. Additionally, replace the direct
stderr fprintf call with curl's standard error logging mechanism to maintain
consistent error reporting.
| if(ctx.result) { | ||
| /* convert the c-ares version */ | ||
| *res = mk_getaddrinfo(ctx.result); | ||
| /* free the old */ | ||
| ares_freeaddrinfo(ctx.result); | ||
| } | ||
| else | ||
| rc = EAI_NONAME; /* got nothing */ |
There was a problem hiding this comment.
Check for allocation failure in mk_getaddrinfo.
The function mk_getaddrinfo returns NULL on allocation failure, but this case is not distinguished from the case where no results were found.
Apply this diff to properly handle allocation failures:
if(ctx.result) {
/* convert the c-ares version */
*res = mk_getaddrinfo(ctx.result);
/* free the old */
ares_freeaddrinfo(ctx.result);
+ if(!*res)
+ rc = EAI_MEMORY; /* allocation failed */
}
else
rc = EAI_NONAME; /* got nothing */🤖 Prompt for AI Agents
In deps/curl/lib/fake_addrinfo.c around lines 195 to 202, the code calls
mk_getaddrinfo but does not check if it returns NULL due to allocation failure,
which is currently treated the same as no results found. Modify the code to
check if mk_getaddrinfo returns NULL; if it does, set the return code to
EAI_MEMORY to indicate an allocation failure, otherwise proceed as normal. This
ensures allocation failures are properly distinguished and handled.
| if(is_chunked) | ||
| has_chunked = TRUE; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
has_chunked flag set after the validation block
has_chunked is updated only after the writer is created, yet the earlier validation
(lines 775-777) relies on it. If chunked is skipped (e.g., user disabled TE decoding)
the flag is never raised and the “no encoding after chunked” check is bypassed.
Move has_chunked = TRUE just after is_chunked is detected, before any early-exit paths.
🤖 Prompt for AI Agents
In deps/curl/lib/content_encoding.c around lines 834-835, the has_chunked flag
is set only after the writer creation, but the validation block at lines 775-777
depends on this flag. To fix this, move the assignment has_chunked = TRUE
immediately after detecting is_chunked, before any early-exit or validation
checks, ensuring the flag is correctly set for the validation logic.
| if(!r || | ||
| Curl_conn_meta_set(conn, CURL_META_RTMP_CONN, r, rtmp_conn_dtor)) | ||
| return CURLE_OUT_OF_MEMORY; |
There was a problem hiding this comment.
Memory leak when Curl_conn_meta_set() fails
RTMP_Alloc() memory is lost if Curl_conn_meta_set() returns non-zero.
- if(!r ||
- Curl_conn_meta_set(conn, CURL_META_RTMP_CONN, r, rtmp_conn_dtor))
- return CURLE_OUT_OF_MEMORY;
+ if(!r)
+ return CURLE_OUT_OF_MEMORY;
+ if(Curl_conn_meta_set(conn, CURL_META_RTMP_CONN, r, rtmp_conn_dtor)) {
+ RTMP_Free(r); /* prevent leak */
+ 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.
| if(!r || | |
| Curl_conn_meta_set(conn, CURL_META_RTMP_CONN, r, rtmp_conn_dtor)) | |
| return CURLE_OUT_OF_MEMORY; | |
| /* after RTMP_Alloc() into r */ | |
| if(!r) | |
| return CURLE_OUT_OF_MEMORY; | |
| if(Curl_conn_meta_set(conn, CURL_META_RTMP_CONN, r, rtmp_conn_dtor)) { | |
| RTMP_Free(r); /* prevent leak */ | |
| return CURLE_OUT_OF_MEMORY; | |
| } |
🤖 Prompt for AI Agents
In deps/curl/lib/curl_rtmp.c around lines 238 to 240, if Curl_conn_meta_set()
fails, the allocated RTMP_Alloc() memory pointed to by r is not freed, causing a
memory leak. To fix this, check the return value of Curl_conn_meta_set(), and if
it indicates failure, free the memory pointed to by r before returning
CURLE_OUT_OF_MEMORY.
| struct doh_probes *dohp = data->state.async.doh; | ||
| if(dohp) { | ||
| int i; | ||
| Curl_doh_close(data); | ||
| curl_slist_free_all(doh->req_hds); | ||
| data->req.doh->req_hds = NULL; | ||
| Curl_safefree(data->req.doh); | ||
| for(i = 0; i < DOH_SLOT_COUNT; ++i) { | ||
| curlx_dyn_free(&dohp->probe_resp[i].body); | ||
| } | ||
| Curl_safefree(data->state.async.doh); | ||
| } |
There was a problem hiding this comment.
Leaked state.async.hostname and dangling pointer after DoH cleanup
Curl_doh_cleanup() frees data->state.async.doh but never frees the
heap-allocated data->state.async.hostname.
As a result every DoH resolution leaks one hostname string and leaves
async.hostname pointing to freed memory.
@@
- Curl_safefree(data->state.async.doh);
+ Curl_safefree(data->state.async.hostname);
+ data->state.async.hostname = NULL;
+
+ Curl_safefree(data->state.async.doh);
+ data->state.async.doh = NULL;Without this fix long-running processes that perform many DoH look-ups
will continuously grow.
📝 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.
| struct doh_probes *dohp = data->state.async.doh; | |
| if(dohp) { | |
| int i; | |
| Curl_doh_close(data); | |
| curl_slist_free_all(doh->req_hds); | |
| data->req.doh->req_hds = NULL; | |
| Curl_safefree(data->req.doh); | |
| for(i = 0; i < DOH_SLOT_COUNT; ++i) { | |
| curlx_dyn_free(&dohp->probe_resp[i].body); | |
| } | |
| Curl_safefree(data->state.async.doh); | |
| } | |
| struct doh_probes *dohp = data->state.async.doh; | |
| if(dohp) { | |
| int i; | |
| Curl_doh_close(data); | |
| for(i = 0; i < DOH_SLOT_COUNT; ++i) { | |
| curlx_dyn_free(&dohp->probe_resp[i].body); | |
| } | |
| /* free and null out the hostname to avoid a leak and dangling pointer */ | |
| Curl_safefree(data->state.async.hostname); | |
| data->state.async.hostname = NULL; | |
| /* free and null out the doh probes struct */ | |
| Curl_safefree(data->state.async.doh); | |
| data->state.async.doh = NULL; | |
| } |
🤖 Prompt for AI Agents
In deps/curl/lib/doh.c around lines 1347 to 1355, the cleanup function frees
data->state.async.doh but does not free the heap-allocated
data->state.async.hostname, causing a memory leak and leaving async.hostname as
a dangling pointer. To fix this, add code to free data->state.async.hostname
before freeing data->state.async.doh, and then set async.hostname to NULL to
avoid dangling references.
| /* convert these functions if an asynch resolver is not used */ | ||
| #define Curl_resolver_cancel(x) Curl_nop_stmt | ||
| #define Curl_resolver_kill(x) Curl_nop_stmt | ||
| #define Curl_resolver_is_resolved(x,y) CURLE_COULDNT_RESOLVE_HOST | ||
| #define Curl_resolver_wait_resolv(x,y) CURLE_COULDNT_RESOLVE_HOST | ||
| #define Curl_resolver_duphandle(x,y,z) CURLE_OK | ||
| #define Curl_resolver_init(x,y) CURLE_OK | ||
| #define Curl_resolver_global_init() CURLE_OK | ||
| #define Curl_resolver_global_cleanup() Curl_nop_stmt | ||
| #define Curl_resolver_cleanup(x) Curl_nop_stmt | ||
| #define Curl_async_get_impl(x,y) (*(y) = NULL, CURLE_OK) | ||
| #define Curl_async_is_resolved(x,y) CURLE_COULDNT_RESOLVE_HOST | ||
| #define Curl_async_await(x,y) CURLE_COULDNT_RESOLVE_HOST | ||
| #define Curl_async_global_init() CURLE_OK | ||
| #define Curl_async_global_cleanup() Curl_nop_stmt | ||
|
|
There was a problem hiding this comment.
Fallback macros leave the dns out-param untouched → UB risk
When CURLRES_ASYNCH is not defined, Curl_async_is_resolved() and
Curl_async_await() devolve to simple one-line macros returning an error code.
They do not reset *dns, so callers that unconditionally dereference the
pointer will access uninitialised memory.
-#define Curl_async_is_resolved(x,y) CURLE_COULDNT_RESOLVE_HOST
-#define Curl_async_await(x,y) CURLE_COULDNT_RESOLVE_HOST
+#define Curl_async_is_resolved(x,y) (*(y)=NULL, CURLE_COULDNT_RESOLVE_HOST)
+#define Curl_async_await(x,y) (*(y)=NULL, CURLE_COULDNT_RESOLVE_HOST)📝 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.
| /* convert these functions if an asynch resolver is not used */ | |
| #define Curl_resolver_cancel(x) Curl_nop_stmt | |
| #define Curl_resolver_kill(x) Curl_nop_stmt | |
| #define Curl_resolver_is_resolved(x,y) CURLE_COULDNT_RESOLVE_HOST | |
| #define Curl_resolver_wait_resolv(x,y) CURLE_COULDNT_RESOLVE_HOST | |
| #define Curl_resolver_duphandle(x,y,z) CURLE_OK | |
| #define Curl_resolver_init(x,y) CURLE_OK | |
| #define Curl_resolver_global_init() CURLE_OK | |
| #define Curl_resolver_global_cleanup() Curl_nop_stmt | |
| #define Curl_resolver_cleanup(x) Curl_nop_stmt | |
| #define Curl_async_get_impl(x,y) (*(y) = NULL, CURLE_OK) | |
| #define Curl_async_is_resolved(x,y) CURLE_COULDNT_RESOLVE_HOST | |
| #define Curl_async_await(x,y) CURLE_COULDNT_RESOLVE_HOST | |
| #define Curl_async_global_init() CURLE_OK | |
| #define Curl_async_global_cleanup() Curl_nop_stmt | |
| /* convert these functions if an asynch resolver is not used */ | |
| #define Curl_async_get_impl(x,y) (*(y) = NULL, CURLE_OK) | |
| #define Curl_async_is_resolved(x,y) (*(y) = NULL, CURLE_COULDNT_RESOLVE_HOST) | |
| #define Curl_async_await(x,y) (*(y) = NULL, CURLE_COULDNT_RESOLVE_HOST) | |
| #define Curl_async_global_init() CURLE_OK | |
| #define Curl_async_global_cleanup() Curl_nop_stmt |
🤖 Prompt for AI Agents
In deps/curl/lib/asyn.h around lines 224 to 230, the macros
Curl_async_is_resolved and Curl_async_await do not reset the out-param pointer
*dns, which can lead to undefined behavior if callers dereference it. Modify
these macros to explicitly set *dns to NULL before returning the error code to
ensure the pointer is always initialized and safe to use.
| struct async_ares_ctx *ares = &data->state.async.ares; | ||
| *waitp = 0; /* default to synchronous response */ | ||
|
|
||
| res->hostname = strdup(hostname); | ||
| if(!res->hostname) | ||
| if(async_ares_init_lazy(data)) | ||
| return NULL; | ||
|
|
||
| data->state.async.port = port; | ||
| data->state.async.done = FALSE; /* not done */ | ||
| data->state.async.dns = NULL; /* clear */ | ||
| data->state.async.port = port; | ||
| data->state.async.ip_version = ip_version; | ||
| data->state.async.hostname = strdup(hostname); | ||
| if(!data->state.async.hostname) | ||
| return NULL; | ||
|
|
There was a problem hiding this comment.
state.async.hostname is strdup’ed but never freed – memory leak
Curl_async_getaddrinfo() allocates a private copy of the hostname:
data->state.async.hostname = strdup(hostname);Neither async_ares_cleanup() nor Curl_async_ares_destroy() releases that
buffer, so every resolve cycle leaks one hostname string.
static void async_ares_cleanup(struct Curl_easy *data)
{
struct async_ares_ctx *ares = &data->state.async.ares;
+ /* Free hostname copy */
+ free(data->state.async.hostname);
+ data->state.async.hostname = NULL;Double-check that the threaded backend performs the same deallocation.
🤖 Prompt for AI Agents
In deps/curl/lib/asyn-ares.c around lines 724 to 737, the hostname string
allocated with strdup for state.async.hostname is not freed, causing a memory
leak. To fix this, ensure that the allocated hostname string is properly freed
during cleanup by adding a free call for state.async.hostname in
async_ares_cleanup() and Curl_async_ares_destroy(). Also verify that the
threaded backend performs the same deallocation to prevent leaks there as well.
| static time_t | ||
| hostcache_prune(struct Curl_hash *hostcache, int cache_timeout, | ||
| time_t now) | ||
| dnscache_prune(struct Curl_hash *hostcache, int cache_timeout, | ||
| time_t now) | ||
| { | ||
| struct hostcache_prune_data user; | ||
| struct dnscache_prune_data user; | ||
|
|
||
| user.max_age_sec = cache_timeout; | ||
| user.now = now; | ||
| user.oldest = 0; | ||
|
|
||
| Curl_hash_clean_with_criterium(hostcache, | ||
| (void *) &user, | ||
| hostcache_entry_is_stale); | ||
| dnscache_entry_is_stale); | ||
|
|
||
| return user.oldest; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential infinite prune loop when timeout < 0
Curl_dnscache_prune() calls:
do { … } while(timeout && (hash_count > MAX_DNS_CACHE_SIZE));If dns_cache_timeout is -1, timeout is non-zero so the loop keeps
iterating while never deleting entries (after the fix above) ⇒ CPU spin.
- } while(timeout &&
+ } while(timeout > 0 &&
(Curl_hash_count(&dnscache->entries) > MAX_DNS_CACHE_SIZE));Also applies to: 285-289
🤖 Prompt for AI Agents
In deps/curl/lib/hostip.c around lines 217 to 232 and also lines 285 to 289, the
dnscache_prune function can cause an infinite loop if cache_timeout is negative
because the calling code loops while timeout is non-zero, but negative timeout
means no entries get pruned. Fix this by ensuring that if cache_timeout is less
than zero, the prune function returns immediately without attempting to prune,
or modify the calling loop to break if timeout is negative, preventing the
infinite loop and CPU spin.
| if(!ftp || | ||
| Curl_meta_set(data, CURL_META_FTP_EASY, ftp, ftp_easy_dtor)) | ||
| return CURLE_OUT_OF_MEMORY; | ||
|
|
||
| ftp = calloc(1, sizeof(struct FTP)); | ||
| if(!ftp) | ||
| ftpc = calloc(1, sizeof(*ftpc)); | ||
| if(!ftpc || | ||
| Curl_conn_meta_set(conn, CURL_META_FTP_CONN, ftpc, ftp_conn_dtor)) | ||
| return CURLE_OUT_OF_MEMORY; | ||
|
|
||
| /* clone connection related data that is FTP specific */ | ||
| if(data->set.str[STRING_FTP_ACCOUNT]) { | ||
| ftpc->account = strdup(data->set.str[STRING_FTP_ACCOUNT]); |
There was a problem hiding this comment.
Potential leak when Curl_meta_set/Curl_conn_meta_set fails
If either metadata registration fails, the freshly calloc’ed structures are left orphaned:
ftp = calloc(1, …);
if(!ftp || Curl_meta_set(...)) // `ftp` leaked when Curl_meta_set fails
return CURLE_OUT_OF_MEMORY;
ftpc = calloc(1, …);
if(!ftpc || Curl_conn_meta_set(...))// `ftp` *and* `ftpc` leaked here
return CURLE_OUT_OF_MEMORY;Proposed fix – free the allocations before bailing out:
ftp = calloc(1, sizeof(*ftp));
if(!ftp || Curl_meta_set(data, CURL_META_FTP_EASY, ftp, ftp_easy_dtor)) {
+ free(ftp);
return CURLE_OUT_OF_MEMORY;
}
ftpc = calloc(1, sizeof(*ftpc));
if(!ftpc || Curl_conn_meta_set(conn, CURL_META_FTP_CONN,
ftpc, ftp_conn_dtor)) {
+ free(ftpc);
+ Curl_meta_remove(data, CURL_META_FTP_EASY); /* undo earlier meta_set */
return CURLE_OUT_OF_MEMORY;
}(You may also consider letting the destructor passed to Curl_meta_set own the pointer immediately to avoid the manual free().)
This prevents silent leaks on low-memory paths and keeps the connection object clean.
🤖 Prompt for AI Agents
In deps/curl/lib/ftp.c around lines 4414 to 4425, the code leaks memory if
Curl_meta_set or Curl_conn_meta_set fails after calloc allocates ftp or ftpc. To
fix this, add free calls to release ftp if Curl_meta_set fails and free both ftp
and ftpc if Curl_conn_meta_set fails before returning CURLE_OUT_OF_MEMORY.
Alternatively, assign ownership of the allocated pointers to the destructors
immediately to avoid manual frees.
3dcf62d to
b40957a
Compare
PR-URL: #323 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
|
Landed in 0d614cc |
PR-URL: #323 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: #323 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
New APIs and Options
Chores
.checksrclists.Documentation