deps: update libcurl to 8.16.0 #370
Conversation
WalkthroughIntroduces a libcurl 8.16.0 update with extensive internal API refactors from getsock to pollset, new IP “Happy Eyeballs” connection filter, DNS/async resolver updates, new multi handle metrics/query, header/enum literal normalization, cookie and parsedate API changes, build/config updates, tracing additions, and multiple protocol handler adaptations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Multi as curl_multi
participant Easy as CURL easy
participant Poll as easy_pollset
participant Proto as Protocol handler
participant CF as Conn filters
participant OS as Sockets
App->>Multi: add_handle(easy)
Multi->>Easy: state machine tick
Easy->>Poll: Curl_pollset_init/reset
Easy->>Proto: proto_*_pollset(ps)
Proto->>CF: Curl_conn_cf_adjust_pollset(ps)
CF->>Poll: set/change sockets (IN/OUT)
Easy->>OS: Curl_pollset_poll(timeout)
OS-->>Easy: readiness events
Easy->>Proto: perform_*_pollset(ps)
Proto->>CF: send/recv via filters
CF-->>Proto: data/complete
Proto-->>Easy: progress/status
Easy-->>Multi: updates (xfers_running, pending)
Multi-->>App: events/callbacks
sequenceDiagram
autonumber
actor App
participant CFH as CF: IP Happy Eyeballs
participant T4 as Attempt IPv4
participant T6 as Attempt IPv6
participant Net as Network
App->>CFH: insert after (transport)
CFH->>T6: start_connect()
CFH-->>T4: delayed start_connect()
T6->>Net: connect()
T4->>Net: connect()
alt first success
T6-->>CFH: connected
CFH-->>T4: cancel/close
CFH-->>App: connected (winner)
else both fail/time out
T6-->>CFH: error
T4-->>CFH: error
CFH-->>App: CURLE_COULDNT_CONNECT
end
sequenceDiagram
autonumber
participant Easy as CURL easy
participant DNS as Resolver (sync/ares/thread)
participant PS as easy_pollset
participant Cache as DNS cache
Easy->>DNS: Curl_resolv(...)
DNS->>PS: resolv_pollset(ps)
Easy->>PS: poll(timeout)
PS-->>DNS: ready
DNS->>Cache: store (positive/negative)
DNS-->>Easy: result or error(detail)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
deps/curl/lib/krb5.c (1)
559-563: Critical: pass dynbuf length to conn->mech->decode (avoid passing zero 'len')len is decremented to 0 in the read loop; passing that 0 to conn->mech->decode can break GSS unwrap. Upstream curl 8.16.0 (rc1) still calls decode with the decremented len (not the dynbuf length), so apply the local fix below. (fossies.org)
File: deps/curl/lib/krb5.c
Lines: 559-563Apply:
- /* this decodes the dynbuf *in place* */ - nread = conn->mech->decode(conn->app_data, - curlx_dyn_ptr(&buf->buf), - len, conn->data_prot, conn); - if(nread < 0) + /* this decodes the dynbuf *in place* */ + { + size_t enc_sz = curlx_dyn_len(&buf->buf); + if(enc_sz > (size_t)INT_MAX) + return CURLE_TOO_LARGE; + nread = conn->mech->decode(conn->app_data, + curlx_dyn_ptr(&buf->buf), + curlx_uztosi(enc_sz), conn->data_prot, conn); + } + if(nread < 0) return CURLE_RECV_ERROR; Curl_dyn_setlen(&buf->buf, nread); buf->index = 0; return CURLE_OK;deps/curl/lib/curlx/dynbuf.c (1)
76-89: Prevent size_t overflow in fit calculation.
fit = len + idx + 1can wrap before thefit > s->toobigcheck. Guard with subtraction first.Apply:
- size_t fit = len + idx + 1; /* new string + old string + zero byte */ + size_t fit; /* new string + old string + zero byte */ + if(len > s->toobig - idx - 1) { + curlx_dyn_free(s); + return CURLE_TOO_LARGE; + } + fit = idx + len + 1;deps/curl/lib/cf-haproxy.c (1)
227-228: Likely typo: DYN_HAXPROXYAssuming the dynbuf tag is DYN_HAPROXY; current spelling may not compile if undefined.
- curlx_dyn_init(&ctx->data_out, DYN_HAXPROXY); + curlx_dyn_init(&ctx->data_out, DYN_HAPROXY);If the intended tag differs, please align with the defined constant in dynbuf tags.
deps/curl/lib/rtsp.c (1)
177-196: Free allocations on meta‑set failure (memory leak).If Curl_conn_meta_set() or Curl_meta_set() fails, the newly allocated rtspc/rtsp are leaked. Free them before returning.
Apply this diff:
static CURLcode rtsp_setup_connection(struct Curl_easy *data, struct connectdata *conn) { struct rtsp_conn *rtspc; struct RTSP *rtsp; rtspc = calloc(1, sizeof(*rtspc)); if(!rtspc) return CURLE_OUT_OF_MEMORY; curlx_dyn_init(&rtspc->buf, MAX_RTP_BUFFERSIZE); - if(Curl_conn_meta_set(conn, CURL_META_RTSP_CONN, rtspc, rtsp_conn_dtor)) - return CURLE_OUT_OF_MEMORY; + if(Curl_conn_meta_set(conn, CURL_META_RTSP_CONN, rtspc, rtsp_conn_dtor)) { + curlx_dyn_free(&rtspc->buf); + free(rtspc); + return CURLE_OUT_OF_MEMORY; + } rtsp = calloc(1, sizeof(struct RTSP)); - if(!rtsp || - Curl_meta_set(data, CURL_META_RTSP_EASY, rtsp, rtsp_easy_dtor)) - return CURLE_OUT_OF_MEMORY; + if(!rtsp || + Curl_meta_set(data, CURL_META_RTSP_EASY, rtsp, rtsp_easy_dtor)) { + if(rtsp) + free(rtsp); + /* rtspc is owned by conn meta and will be freed by its dtor */ + return CURLE_OUT_OF_MEMORY; + } return CURLE_OK; }deps/curl/lib/http.c (1)
1490-1497: Critical: Curl_compareheader only matches at start-of-value (regression).The loop condition uses a fixed upper bound, so it checks only the first position and never scans the whole header value. This breaks cases where the token is not at offset 0.
Apply this diff to scan the entire value:
- if(curlx_strlen(&val) >= clen) { - size_t len; - p = curlx_str(&val); - for(len = curlx_strlen(&val); len >= curlx_strlen(&val); len--, p++) { - if(curl_strnequal(p, content, clen)) - return TRUE; /* match! */ - } - } + if(curlx_strlen(&val) >= clen) { + size_t vlen = curlx_strlen(&val); + const char *vp = curlx_str(&val); + size_t i; + for(i = 0; i + clen <= vlen; ++i) { + if(curl_strnequal(vp + i, content, clen)) + return TRUE; /* match! */ + } + }
🧹 Nitpick comments (73)
deps/curl/lib/curlx/wait.c (1)
71-73: Clamp MSDOS delay to avoid unsigned int overflow
delay((unsigned int)timeout_ms);may wrap for large timeouts. Mirror the Windows path by clamping toUINT_MAX - 1.Apply this diff:
-#ifdef MSDOS - delay((unsigned int)timeout_ms); +#ifdef MSDOS +#if TIMEDIFF_T_MAX >= UINT_MAX + if(timeout_ms >= UINT_MAX) + timeout_ms = UINT_MAX-1; +#endif + delay((unsigned int)timeout_ms);deps/curl/lib/curlx/multibyte.h (1)
29-32: Doc-only tweak looks fineSingle-line comment and guard scope remain correct; no API/ABI impact.
Optionally keep trailing context on the guard for readability:
-#endif +#endif /* _WIN32 */deps/curl/lib/curl_gethostname.c (1)
84-84: Guard against namelen == 0 before writing the terminator.
name[namelen - 1] = '\0';can underflow whennamelenis 0. Add a cheap check.Apply this diff:
- name[namelen - 1] = '\0'; + if(namelen) + name[namelen - 1] = '\0'; + else + return 1;Please confirm callers never pass
namelen == 0; if guaranteed by contract, consider a DEBUGBUILD assert instead.deps/curl/lib/curl_addrinfo.c (1)
486-516: Fix mismatched trailing #endif comment.The block opens with
#if defined(CURLDEBUG) && defined(HAVE_GETADDRINFO) && defined(HAVE_FREEADDRINFO)but ends with/* CURLDEBUG && HAVE_FREEADDRINFO */. Align the comment for clarity.Apply:
-#endif /* CURLDEBUG && HAVE_FREEADDRINFO */ +#endif /* CURLDEBUG && HAVE_GETADDRINFO && HAVE_FREEADDRINFO */deps/curl/lib/curlx/timeval.c (2)
171-173: Make timebase init thread-safe.This lazy init on a static
timebasecan race under concurrent calls tocurlx_now(). Prefer one-time init.Apply this diff within the function, and add the helper outside (see snippet below):
- if(timebase.denom == 0) - (void)mach_timebase_info(&timebase); + pthread_once(&timebase_once, curlx_timebase_init);Add near the top of this block:
#include <pthread.h> static mach_timebase_info_data_t timebase; static pthread_once_t timebase_once = PTHREAD_ONCE_INIT; static void curlx_timebase_init(void) { (void)mach_timebase_info(&timebase); }
175-176: Guard against 64‑bit overflow in scale-to-ns step.
mach_absolute_time() * timebase.numercan overflow 64‑bit before the divide. Use 128‑bit when available, else divide first (precision loss is negligible at µs resolution).Apply this diff:
- usecs = mach_absolute_time(); - usecs *= timebase.numer; /* spellchecker:disable-line */ - usecs /= timebase.denom; - usecs /= 1000; + #if defined(__SIZEOF_INT128__) + { + __uint128_t n = ( (__uint128_t)mach_absolute_time() * timebase.numer ); /* spellchecker:disable-line */ + n /= timebase.denom; + usecs = (uint64_t)(n / 1000); + } + #else + usecs = mach_absolute_time(); + /* Reduce overflow risk by dividing first; µs rounding is acceptable. */ + usecs = (usecs / timebase.denom) * timebase.numer; /* spellchecker:disable-line */ + usecs /= 1000; + #endifdeps/curl/lib/idn.c (2)
155-167: Guard_WIN32_WINNT_VISTAabsence to keep portability with older SDKs.Some SDKs may not define
_WIN32_WINNT_VISTA; when that happens the comparison evaluates against0, potentially skipping these prototypes incorrectly. Use a fallback to0x0600in the expression and normalizeWINVERcomparison.Apply this diff:
-#if (!defined(_WIN32_WINNT) || _WIN32_WINNT < _WIN32_WINNT_VISTA) && \ - (!defined(WINVER) || WINVER < 0x600) +#if (!defined(_WIN32_WINNT) || \ + _WIN32_WINNT < (defined(_WIN32_WINNT_VISTA) ? _WIN32_WINNT_VISTA : 0x0600)) && \ + (!defined(WINVER) || WINVER < 0x0600)If you prefer not to use the ternary in a preproc expression, alternatively add a local fallback just above this block:
#ifndef _WIN32_WINNT_VISTA #define _WIN32_WINNT_VISTA 0x0600 #endifPlease confirm Windows CI across all supported VS/SDK combos still compiles this TU without relying on
<sdkddkver.h>.
326-333: Harden against unexpected NULL fromidn_decodeand clear output on error.Defensive null‑check prevents UB if invariants ever regress; also explicitly null the out‑param on error.
Apply this diff:
- if(!result) { - if(!d[0]) { /* ended up zero length, not acceptable */ - result = CURLE_URL_MALFORMAT; - free(d); - } - else - *output = d; - } + if(!result) { + if(!d || !d[0]) { /* ended up NULL/zero length, not acceptable */ + result = CURLE_URL_MALFORMAT; + free(d); + if(output) *output = NULL; + } + else { + *output = d; + } + }deps/curl/lib/bufq.c (2)
176-181: Good overflow guard before allocation in bufcp_take.Prevents size_t wrap when computing the allocation size. Optional: factor this pattern into a tiny helper to avoid duplication and keep intent consistent across sites.
310-314: Mirrored overflow guard in non‑pooled path looks correct.Returning NULL here propagates as CURLE_OUT_OF_MEMORY at the call site, which is appropriate for overflow. Optional: consolidate both checks via a shared helper to keep behavior uniform.
deps/curl/lib/httpsrr.h (1)
29-31: Optional: narrow the ares include to the HTTPSRR feature blockAvoid including <ares.h> when USE_HTTPSRR is off (but USE_ARES is on) to reduce header coupling.
Apply:
-#ifdef USE_ARES -#include <ares.h> -#endif +#ifdef USE_HTTPSRR + #ifdef USE_ARES + #include <ares.h> + #endifAlso applies to: 33-35
deps/curl/lib/curl_config.h.in (1)
68-69: Fix comment: says “alt-svc” but macro disables HSTSThe comment mismatches the macro name.
Apply:
-/* disable alt-svc */ +/* disable HSTS */ #undef CURL_DISABLE_HSTSdeps/curl/lib/progress.c (1)
287-317: Guard against negative byte deltas when counters reset.If d->cur_size < d->limit.start_size (e.g., counter reset/redirect), bytes becomes negative; should_ms then goes negative. It still returns 0, but add an explicit non‑negative guard to avoid surprising math.
- curl_off_t bytes = d->cur_size - d->limit.start_size; + curl_off_t bytes = d->cur_size - d->limit.start_size; timediff_t should_ms; timediff_t took_ms; - /* no limit or we did not get to any bytes yet */ - if(!bytes_per_sec || !bytes) + /* no limit or no positive progress yet */ + if(!bytes_per_sec || bytes <= 0) return 0;deps/curl/lib/curl_fnmatch.c (2)
81-99: Avoid counting ':' and ']' toward the keyword length (minor robustness)
iincrements every loop iteration, so:and]are included in the bound check. Not an issue today (all known keywords are ≤6 chars), but trivially avoidable. Incrementionly when appending a letter and check againstsizeof(keyword) - 1before writing.- size_t i; + size_t i; const unsigned char *p = *pattern; bool found = FALSE; - for(i = 0; !found; i++) { - char c = (char)*p++; - if(i >= sizeof(keyword)) - return SETCHARSET_FAIL; + for(i = 0; !found; ) { + char c = (char)*p++; switch(state) { case CURLFNM_PKW_INIT: - if(ISLOWER(c)) - keyword[i] = c; + if(ISLOWER(c)) { + if(i == sizeof(keyword) - 1) + return SETCHARSET_FAIL; + keyword[i++] = c; + } else if(c == ':') state = CURLFNM_PKW_DDOT; else return SETCHARSET_FAIL; break; case CURLFNM_PKW_DDOT: if(c == ']') found = TRUE; else return SETCHARSET_FAIL; + break; } }
101-101: Remove stale#undef KEYLEN
KEYLEN is not defined in this file; the#undefis a harmless no-op and confusing — delete it.
deps/curl/lib/curl_fnmatch.c:101deps/curl/lib/curl_sspi.c (1)
199-206: Zero sensitive password buffer before freeFreeing
identity->Passwordwithout clearing leaves credentials in process memory. Wipe it first.Note: if this file is vendored from upstream curl 8.16.0, consider submitting upstream or carrying as a minimal patch if acceptable.
void Curl_sspi_free_identity(SEC_WINNT_AUTH_IDENTITY *identity) { if(identity) { Curl_safefree(identity->User); - Curl_safefree(identity->Password); + if(identity->Password && identity->PasswordLength) { + /* bytes = characters * sizeof(TCHAR) */ + SecureZeroMemory(identity->Password, + (size_t)identity->PasswordLength * sizeof(TCHAR)); + } + Curl_safefree(identity->Password); Curl_safefree(identity->Domain); } }If you want, I can check upstream for an equivalent practice and open an issue/PR.
deps/curl/lib/mime.c (2)
2001-2026: Guard against partial bufq writes in the “small read” pathIf Curl_bufq_write ever stores fewer than requested bytes (unlikely, but future‑proof), subsequent cread would underflow expectations. Add a strict check and fail early.
else if(blen <= 4) { @@ - nread = Curl_mime_read(tmp, 1, sizeof(tmp), ctx->part); + nread = Curl_mime_read(tmp, 1, sizeof(tmp), ctx->part); if(nread <= sizeof(tmp)) { size_t n; - result = Curl_bufq_write(&ctx->tmpbuf, (unsigned char *)tmp, nread, &n); + result = Curl_bufq_write(&ctx->tmpbuf, (unsigned char *)tmp, nread, &n); if(result) { ctx->errored = TRUE; ctx->error_result = result; return result; } + if(n != nread) { + ctx->errored = TRUE; + ctx->error_result = CURLE_WRITE_ERROR; + return ctx->error_result; + } /* stored it, read again */ result = Curl_bufq_cread(&ctx->tmpbuf, buf, blen, &nread);
2152-2173: Return CURLE_UNKNOWN_OPTION for unknown control opcodesSearch shows callers only use CURL_CRCNTRL_REWIND / CURL_CRCNTRL_UNPAUSE / CURL_CRCNTRL_CLEAR_EOS and none appear to rely on an implicit OK for unknown opcodes; this change is safe and improves error visibility.
default: - break; + return CURLE_UNKNOWN_OPTION;deps/curl/lib/curl_ntlm_core.c (2)
54-66: Crypto backend selection and AWS-LC shim: confirm portability
- USE_OPENSSL_DES gating and OPENSSL_NO_DEPRECATED_3_0 handling look correct for OpenSSL 3.x.
- The AWS-LC alias for DES_set_key_unchecked is reasonable; verify OPENSSL_IS_AWSLC is present across targeted AWS-LC versions, or guard with defined(...) to avoid unknown-macro warnings on some compilers.
- WOLFSSL OPENSSL_COEXIST remaps are unchanged; OK.
If you hit portability warnings, consider a defensive check:
-# ifdef OPENSSL_IS_AWSLC /* for versions 1.2.0 to 1.30.1 */ +# if defined(OPENSSL_IS_AWSLC) /* for versions 1.2.0 to 1.30.1 */ # define DES_set_key_unchecked (void)DES_set_key # endifAlso applies to: 69-77, 83-93, 94-96
569-577: Minor: avoid msnprintf for binary blob header or check returnmsnprintf builds binary bytes via %c, which works, but silently truncates on unexpected sizes. Consider direct stores or at least validate the return.
- msnprintf((char *)ptr + HMAC_MD5_LENGTH, NTLMv2_BLOB_LEN, - "%c%c%c%c" - "%c%c%c%c" - "%c%c%c%c%c%c%c%c", - NTLMv2_BLOB_SIGNATURE[0], NTLMv2_BLOB_SIGNATURE[1], - NTLMv2_BLOB_SIGNATURE[2], NTLMv2_BLOB_SIGNATURE[3], - 0, 0, 0, 0, - LONGQUARTET(tw.dwLowDateTime), LONGQUARTET(tw.dwHighDateTime)); + unsigned char *b = ptr + HMAC_MD5_LENGTH; + memcpy(b, NTLMv2_BLOB_SIGNATURE, 4); + memset(b + 4, 0, 4); + b[8] = (unsigned char)(tw.dwLowDateTime & 0xFF); + b[9] = (unsigned char)((tw.dwLowDateTime >> 8) & 0xFF); + b[10] = (unsigned char)((tw.dwLowDateTime >> 16) & 0xFF); + b[11] = (unsigned char)((tw.dwLowDateTime >> 24) & 0xFF); + b[12] = (unsigned char)(tw.dwHighDateTime & 0xFF); + b[13] = (unsigned char)((tw.dwHighDateTime >> 8) & 0xFF); + b[14] = (unsigned char)((tw.dwHighDateTime >> 16) & 0xFF); + b[15] = (unsigned char)((tw.dwHighDateTime >> 24) & 0xFF);deps/curl/lib/krb5.c (2)
472-474: Nit: (void)nread likely unnecessary.nread is used as an out-param to Curl_GetFTPResponse; most compilers won’t warn. Safe to keep, but could be dropped.
- (void)nread; + /* nread intentionally unused beyond Curl_GetFTPResponse */
720-748: Remove redundant (void)data; it’s used later.data is used by Curl_debug on Line 747; the cast is confusing and unnecessary.
- (void)data; + /* data used below for Curl_debug */deps/curl/lib/curl_threads.h (1)
69-69: New API Curl_thread_cancel: document return semantics.It returns 0 on success (POSIX), 1 when not supported, and 0 when handle is null; that differs from Curl_thread_join’s boolean style. Add a short comment to avoid misuse.
int Curl_thread_cancel(curl_thread_t *hnd); +/* Returns: + * 0 on successful cancellation (POSIX with pthread_cancel), + * 1 when cancellation is not supported on the platform, + * 0 if the handle is null. + * Note: semantics differ from Curl_thread_join (boolean-style). + */deps/curl/lib/curl_threads.c (3)
44-60: Align thunk types with the public macro and silence potential warnings.Use CURL_THREAD_RETURN_T (CURL_STDCALL func)(void) in Curl_actual_call and cast the ignored return to void; also prefer returning NULL.
-struct Curl_actual_call { - unsigned int (*func)(void *); - void *arg; -}; +struct Curl_actual_call { + CURL_THREAD_RETURN_T (CURL_STDCALL *func)(void *); + void *arg; +}; @@ -static void *curl_thread_create_thunk(void *arg) +static void *curl_thread_create_thunk(void *arg) { struct Curl_actual_call *ac = arg; - unsigned int (*func)(void *) = ac->func; + CURL_THREAD_RETURN_T (CURL_STDCALL *func)(void *) = ac->func; void *real_arg = ac->arg; @@ - (*func)(real_arg); - - return 0; + (void)(*func)(real_arg); + return NULL; }
119-129: Curl_thread_cancel (POSIX): clarify status codes and remove redundant cast.Function returns 1 when not supported and 0 on null handle; consider documenting (see header comment) and drop the unused (void)hnd.
-int Curl_thread_cancel(curl_thread_t *hnd) +int Curl_thread_cancel(curl_thread_t *hnd) { - (void)hnd; if(*hnd != curl_thread_t_null) #ifdef USE_PTHREAD_CANCEL return pthread_cancel(**hnd); #else return 1; /* not supported */ #endif return 0; }
185-191: Curl_thread_cancel (Windows): explicit “not supported” return.Returns 1 when handle is valid. Consider mirroring the header doc comment for consistency.
-int Curl_thread_cancel(curl_thread_t *hnd) +int Curl_thread_cancel(curl_thread_t *hnd) { if(*hnd != curl_thread_t_null) { - return 1; /* not supported */ + return 1; /* not supported: Windows threads cannot be asynchronously canceled */ } return 0; }deps/curl/lib/memdebug.c (1)
374-377: Keep function-name parentheses to avoid macro interception and stay consistent.Elsewhere in this file you intentionally use
(socket),(send),(recv),(fdopen),(fopen),(accept4)to bypass possible macro wrappers. Dropping them foracceptis inconsistent and could bite ifacceptis ever macro-defined by a platform header.Apply for consistency:
- curl_socket_t sockfd = accept(s, addr, addrlen); + curl_socket_t sockfd = (accept)(s, addr, addrlen);deps/curl/lib/curlx/dynbuf.c (1)
231-236: Null-safe debug assert for format string.
DEBUGASSERT(strcmp(fmt, "%s"));dereferencesfmtin debug builds iffmt == NULL. Add a prior assert.Apply:
DEBUGASSERT(s->init == DYNINIT); DEBUGASSERT(!s->leng || s->bufr); - DEBUGASSERT(strcmp(fmt, "%s")); /* use curlx_dyn_add instead */ + DEBUGASSERT(fmt); + DEBUGASSERT(strcmp(fmt, "%s")); /* use curlx_dyn_add instead */deps/curl/lib/Makefile.am (1)
177-177: clang-tidy ArrayBound check disabled — confirm rationale.Disabling
-clang-analyzer-security.ArrayBoundcan hide real issues (esp. around dynamic buffers). If this was added due to false positives, consider scoping via NOLINT comments or path filters instead of blanket disable.Provide context (e.g., specific false positives) or consider narrowing with
--config-fileor per-file excludes.deps/curl/lib/sendf.h (1)
53-53: CLIENTWRITE_0LEN may trip Curl_client_write() debug assertsNew flag is sensible. However, Curl_client_write() currently only allows BODY/INFO combined with EOS in its debug asserts. Using CLIENTWRITE_0LEN with BODY/INFO would assert in debug builds. Either:
- allow CLIENTWRITE_0LEN in those asserts, or
- document it as HEADER‑only.
Suggested change outside this file (sendf.c):
--- a/deps/curl/lib/sendf.c +++ b/deps/curl/lib/sendf.c @@ -78,10 +78,10 @@ CURLcode Curl_client_write(struct Curl_easy *data, - DEBUGASSERT(!(type & CLIENTWRITE_BODY) || - ((type & ~(CLIENTWRITE_BODY|CLIENTWRITE_EOS)) == 0)); + DEBUGASSERT(!(type & CLIENTWRITE_BODY) || + ((type & ~(CLIENTWRITE_BODY|CLIENTWRITE_EOS|CLIENTWRITE_0LEN)) == 0)); /* INFO is only INFO (with optional EOS) */ - DEBUGASSERT(!(type & CLIENTWRITE_INFO) || - ((type & ~(CLIENTWRITE_INFO|CLIENTWRITE_EOS)) == 0)); + DEBUGASSERT(!(type & CLIENTWRITE_INFO) || + ((type & ~(CLIENTWRITE_INFO|CLIENTWRITE_EOS|CLIENTWRITE_0LEN)) == 0));deps/curl/include/curl/typecheck-gcc.h (1)
224-237: New multi option predicates (cb-data, char) — sanity check deprecated opts**Includes PIPELINING_*_BL; confirm these exist in bundled 8.16.0 headers for consistency.
deps/curl/lib/curl_trc.c (1)
301-314: Unit mismatch in timeout trace (ns vs timediff_us)Message says “ns” but uses curlx_timediff_us(). Either change label to “us” or switch to a nanosecond diff helper.
- CURL_TRC_M(data, "[TIMEOUT] %s expires in %" FMT_TIMEDIFF_T "ns", + CURL_TRC_M(data, "[TIMEOUT] %s expires in %" FMT_TIMEDIFF_T "us", CURL_TIMER_NAME(n->eid), curlx_timediff_us(n->time, now));deps/curl/lib/pingpong.h (1)
150-153: Add pollset contract docs; callers migratedNo remaining Curl_pp_getsock references found; callers now use Curl_pp_pollset. Add a short contract comment in deps/curl/lib/pingpong.h next to the Curl_pp_pollset declaration:
/* Contract:
- Populate ps with the sockets/events the pingpong state machine needs next.
- Must be non-blocking; return CURLE_OK on success.
- Return CURLE_AGAIN if no sockets yet (e.g., command queued), CURLE_ABORTED_BY_CALLBACK
- if a callback aborted, or another CURLcode on hard errors.
- ps may be set to "out-only" when write flush is pending.
*/deps/curl/lib/multiif.h (2)
75-77: Mark Curl_multi_pollset as WARN_UNUSED_RESULTCallers should not ignore failures. Align with Curl_update_timer’s annotation.
-CURLMcode Curl_multi_pollset(struct Curl_easy *data, - struct easy_pollset *ps, - const char *caller); +CURLMcode Curl_multi_pollset(struct Curl_easy *data, + struct easy_pollset *ps, + const char *caller) WARN_UNUSED_RESULT;
165-166: API addition looks good — no code references remainClear counterpart to Curl_multi_mark_dirty; naming and placement are consistent. Search found no calls to Curl_multi_getsock in deps/curl; only remaining mention is doc/internals/MULTI-EV.md:50 (
Curl_multi_getsock()) — update or remove that doc reference.deps/curl/lib/cf-haproxy.c (2)
69-77: Typo: cf_haproxy_date_out_set → cf_haproxy_data_out_setRename for clarity and consistency.
-static CURLcode cf_haproxy_date_out_set(struct Curl_cfilter*cf, +static CURLcode cf_haproxy_data_out_set(struct Curl_cfilter*cf, struct Curl_easy *data) @@ - result = cf_haproxy_date_out_set(cf, data); + result = cf_haproxy_data_out_set(cf, data);Also applies to: 127-129
182-193: Defensive check for invalid socket in adjust_pollsetAvoid setting poll on CURL_SOCKET_BAD.
static CURLcode cf_haproxy_adjust_pollset(struct Curl_cfilter *cf, struct Curl_easy *data, struct easy_pollset *ps) { if(cf->next->connected && !cf->connected) { /* If we are not connected, but the filter "below" is * and not waiting on something, we are sending. */ - return Curl_pollset_set_out_only( - data, ps, Curl_conn_cf_get_socket(cf, data)); + curl_socket_t s = Curl_conn_cf_get_socket(cf, data); + if(s == CURL_SOCKET_BAD) + return CURLE_OK; + return Curl_pollset_set_out_only(data, ps, s); } return CURLE_OK; }deps/curl/lib/cookie.h (1)
115-115: Document Curl_secure_context semanticsAdd brief doc on what constitutes “secure” (TLS, proxy TLS, Unix sockets?) for consistent callers.
deps/curl/lib/asyn-base.c (1)
88-89: Consider using a named constant for the socket array size.The hard-coded value of 16 sockets should be defined as a constant for better maintainability.
+#define ARES_MAX_SOCKETS 16 + CURLcode Curl_ares_pollset(struct Curl_easy *data, ares_channel channel, struct easy_pollset *ps) { struct timeval maxtime = { CURL_TIMEOUT_RESOLVE, 0 }; struct timeval timebuf; - curl_socket_t sockets[16]; /* ARES documented limit */ + curl_socket_t sockets[ARES_MAX_SOCKETS]; /* ARES documented limit */deps/curl/lib/conncache.c (1)
853-860: Remove misleading (void)data in cpool_mark_stale/reap helpers.cpool_reap_no_reuse marks
dataas unused but uses it later. Drop the cast to avoid confusion.Apply:
static int cpool_mark_stale(struct Curl_easy *data, struct connectdata *conn, void *param) { - (void)data; (void)param; conn->bits.no_reuse = TRUE; return 0; } static int cpool_reap_no_reuse(struct Curl_easy *data, struct connectdata *conn, void *param) { - (void)data; (void)param; if(!CONN_INUSE(conn) && conn->bits.no_reuse) { Curl_conn_terminate(data, conn, FALSE); return 1; } return 0; /* continue iteration */ }deps/curl/lib/mqtt.c (1)
110-112: Pollset migration LGTM; add bad-socket guard.Move from getsock→pollset is correct. Add a quick check for an invalid socket before registering.
Apply:
static CURLcode mqtt_pollset(struct Curl_easy *data, struct easy_pollset *ps) { - return Curl_pollset_add_in(data, ps, data->conn->sock[FIRSTSOCKET]); + curl_socket_t s = data->conn->sock[FIRSTSOCKET]; + if(s == CURL_SOCKET_BAD) + return CURLE_OK; + return Curl_pollset_add_in(data, ps, s); }Also applies to: 129-132, 216-221
deps/curl/include/curl/multi.h (1)
471-479: Typo in comment (CULRM_OK).Change to CURLM_OK.
Apply:
- * Returns: CULRM_OK or error when value could not be obtained. + * Returns: CURLM_OK or error when value could not be obtained.deps/curl/lib/md4.c (1)
55-60: mbedTLS config include may not exist on all builds.Use MBEDTLS_CONFIG_FILE or fallback to config.h to improve portability.
Apply:
-#include <mbedtls/mbedtls_config.h> +#ifdef MBEDTLS_CONFIG_FILE +#include MBEDTLS_CONFIG_FILE +#else +#include <mbedtls/config.h> +#endifdeps/curl/lib/cf-https-connect.c (2)
430-448: Avoid short‑circuiting pollset updates after the first failureEarly‑exit means one failing baller prevents the remaining active ballers from registering their fds, which can stall the happy‑eyeballs race. Prefer continuing to adjust the pollset while remembering the first error.
Apply:
static CURLcode cf_hc_adjust_pollset(struct Curl_cfilter *cf, struct Curl_easy *data, struct easy_pollset *ps) { - CURLcode result = CURLE_OK; + CURLcode result = CURLE_OK; + CURLcode rc = CURLE_OK; if(!cf->connected) { struct cf_hc_ctx *ctx = cf->ctx; size_t i; - for(i = 0; (i < ctx->baller_count) && !result; i++) { + for(i = 0; i < ctx->baller_count; i++) { struct cf_hc_baller *b = &ctx->ballers[i]; if(!cf_hc_baller_is_active(b)) continue; - result = Curl_conn_cf_adjust_pollset(b->cf, data, ps); + rc = Curl_conn_cf_adjust_pollset(b->cf, data, ps); + if(!result && rc) + result = rc; /* record first error, keep going */ } CURL_TRC_CF(data, cf, "adjust_pollset -> %d, %d socks", result, ps->n); } return result; }
689-697: Unify Curl_conn_may_http3 checks for readabilityTwo code paths check Curl_conn_may_http3 differently (one inline in condition, one storing to result). Consider a consistent pattern (store to a local CURLcode and branch) for clarity.
Also applies to: 716-727
deps/curl/lib/pingpong.c (1)
400-407: Guard against invalid socket when updating pollsetIf FIRSTSOCKET is CURL_SOCKET_BAD (early connect/teardown edge), Curl_pollset_change may still be called. Cheap guard avoids surprising errors.
Apply:
CURLcode Curl_pp_pollset(struct Curl_easy *data, struct pingpong *pp, struct easy_pollset *ps) { - int flags = pp->sendleft ? CURL_POLL_OUT : CURL_POLL_IN; - return Curl_pollset_change(data, ps, data->conn->sock[FIRSTSOCKET], - flags, 0); + int flags = pp->sendleft ? CURL_POLL_OUT : CURL_POLL_IN; + curl_socket_t s = data->conn->sock[FIRSTSOCKET]; + if(s == CURL_SOCKET_BAD) + return CURLE_OK; + return Curl_pollset_change(data, ps, s, flags, 0); }deps/curl/lib/connect.c (1)
439-441: Fix minor grammar in error message“not support” → “not supported”.
- failf(data, "haproxy protocol not support with SSL " - "encryption in place (QUIC?)"); + failf(data, "haproxy protocol not supported with SSL " + "encryption in place (QUIC?)");deps/curl/lib/imap.c (1)
1562-1569: Defensive check before pollset updateMirror the pingpong guard to skip updating the pollset when the socket is invalid.
static CURLcode imap_pollset(struct Curl_easy *data, struct easy_pollset *ps) { struct imap_conn *imapc = Curl_conn_meta_get(data->conn, CURL_META_IMAP_CONN); - return imapc ? Curl_pp_pollset(data, &imapc->pp, ps) : CURLE_OK; + if(!imapc) + return CURLE_OK; + if(data->conn->sock[FIRSTSOCKET] == CURL_SOCKET_BAD) + return CURLE_OK; + return Curl_pp_pollset(data, &imapc->pp, ps); }deps/curl/lib/select.c (6)
44-44: Guard curl_trc.h include for non-tracing builds.If curl_trc.h is not present or compiled out in some configs, this unconditional include can break non‑tracing builds.
Proposed guard:
-#include "curl_trc.h" +#if defined(DEBUGBUILD) || defined(CURL_ENABLE_TRACING) +#include "curl_trc.h" +#endif
522-529: Provide a destroy helper for symmetry.You added create/init/cleanup but not destroy. Exposing a small wrapper avoids allocator mismatches and reduces call‑site errors.
Proposed addition (outside this hunk, colocate with create/cleanup):
void Curl_pollset_destroy(struct easy_pollset *ps) { if(!ps) return; Curl_pollset_cleanup(ps); free(ps); }
547-566: Clarify init/reset interplay in move to avoid double work.After transferring ownership, you call Curl_pollset_init(from) which reallocates defaults and then calls reset (which again clears). Consider reset-only to save a memset of def_* twice.
Proposed tweak:
- Curl_pollset_init(from); + /* from now owns only its def_*; just clear visible entries */ + Curl_pollset_reset(from);The else-branch can keep Curl_pollset_init(from) for explicit re-init if preferred.
571-643: Minor: (void)data contradicts later use in trace; tighten overflow guard.
- (void)data precedes a CURL_TRC_M(data, ...) use. Drop the cast to avoid confusion.
- new_count computation is good; consider documenting the overflow guard path returning CURLE_OUT_OF_MEMORY.
- (void)data; + /* 'data' used for tracing */
656-694: OOM path returns -1 indistinguishable from syscall error.Returning -1 on calloc failure mirrors Curl_poll()’s error contract but loses errno context. If feasible, set errno = ENOMEM before returning to aid diagnostics.
- if(!pfds) - return -1; + if(!pfds) { + SET_ERRNO(ENOMEM); + return -1; + }
696-712: Harden pointer params.Add DEBUGASSERTs for pwant_read/pwant_write to catch misuse in debug builds.
- (void)data; + (void)data; +#ifdef DEBUGBUILD + DEBUGASSERT(pwant_read); + DEBUGASSERT(pwant_write); +#endifdeps/curl/lib/asyn.h (1)
74-83: Update comment to match new pollset API.Docstring still describes “getsock”-style arrays and bitmask return. Align it with Curl_async_pollset(struct Curl_easy*, struct easy_pollset*).
-/* Curl_async_pollset() - * - * This function is called from the Curl_multi_pollset() function. 'sock' is a - * pointer to an array to hold the file descriptors, with 'numsock' being the - * size of that array (in number of entries). This function is supposed to - * return bitmask indicating what file descriptors (referring to array indexes - * in the 'sock' array) to wait for, read/write. - */ +/* Curl_async_pollset() + * + * Called from Curl_multi_pollset(). Implementations append sockets and + * desired actions (CURL_POLL_IN/OUT) into the provided easy_pollset 'ps'. + * Returns a CURLcode; no bitmask or array index semantics remain. + */deps/curl/lib/multi.c (3)
1141-1155: Avoid noisy “WARNING: no socket in pollset or timer” in non‑debug builds.This infof() warns and asserts when expect_sockets==true but no fds/timeouts exist. In some edge transitions this can be transient and spam logs. Gate the message behind verbose or emit once per transfer.
- infof(data, "WARNING: no socket in pollset or timer, transfer may stall!"); - DEBUGASSERT(0); + if(CURL_TRC_M_is_verbose(data)) + infof(data, "WARNING: no socket in pollset or timer, transfer may stall!"); + DEBUGASSERT(0);
1888-1920: Rate‑limit transition: clear dirty before scheduling and document semantics.mspeed_check() moves to MSTATE_RATELIMITING and expires EXPIRE_TOOFAST, then clears dirty. That’s correct; consider a brief comment that clearing dirty prevents immediate re‑runs until the timeout fires to avoid oscillation.
3754-3789: Public counter API: define “added” semantics and overflow behavior.curl_multi_get_offt() exposes ADDED (xfers_total_ever). If adds that later fail (e.g., timer setup failure) are counted, callers may get surprises. Align with the fix above or clarify docs that ADDED counts attempted adds. Also ensure counters are 64‑bit long‑lived and won’t wrap in long‑running processes.
deps/curl/lib/rtsp.c (2)
118-122: Null‑check data->conn in rtsp_do_pollset.Accesses data->conn->sock[FIRSTSOCKET] without a guard. Mirror the other pollset helpers and early‑return if conn is NULL.
static CURLcode rtsp_do_pollset(struct Curl_easy *data, struct easy_pollset *ps) { - /* write mode */ - return Curl_pollset_add_out(data, ps, data->conn->sock[FIRSTSOCKET]); + if(data->conn && data->conn->sock[FIRSTSOCKET] != CURL_SOCKET_BAD) + return Curl_pollset_add_out(data, ps, data->conn->sock[FIRSTSOCKET]); + return CURLE_OK; }
759-769: Guard RTP channel mask indexing.idx is derived from an arbitrary byte and used to index rtp_channel_mask. Add a bounds check against sizeof(rtp_channel_mask) to avoid OOB reads with malformed interleaved data.
- int idx = ((unsigned char)buf[0]) / 8; + int idx = ((unsigned char)buf[0]) / 8; int off = ((unsigned char)buf[0]) % 8; DEBUGASSERT(curlx_dyn_len(&rtspc->buf) == 1); - if(!(data->state.rtp_channel_mask[idx] & (1 << off))) { + if(idx < 0 || idx >= (int)sizeof(data->state.rtp_channel_mask) || + !(data->state.rtp_channel_mask[idx] & (1 << off))) {deps/curl/lib/cf-socket.c (1)
2113-2120: Don’t gate accept() on getsockname() success.If getsockname() spuriously fails, we skip accept entirely. Try accept() regardless; use getsockname() only to (best‑effort) fill ‘add’, not to decide whether to accept.
- if(!getsockname(ctx->sock, (struct sockaddr *) &add, &size)) { - size = sizeof(add); -#ifdef HAVE_ACCEPT4 - s_accepted = accept4(ctx->sock, (struct sockaddr *) &add, &size, - SOCK_NONBLOCK | SOCK_CLOEXEC); -#else - s_accepted = CURL_ACCEPT(ctx->sock, (struct sockaddr *) &add, &size); -#endif - } + (void)getsockname(ctx->sock, (struct sockaddr *)&add, &size); /* best effort */ + size = sizeof(add); +#ifdef HAVE_ACCEPT4 + s_accepted = accept4(ctx->sock, (struct sockaddr *)&add, &size, + SOCK_NONBLOCK | SOCK_CLOEXEC); +#else + s_accepted = CURL_ACCEPT(ctx->sock, (struct sockaddr *)&add, &size); +#endifdeps/curl/lib/cf-ip-happy.c (3)
352-513: Happy Eyeballs runner: timeout scheduling looks correct; set explicit lower bound.You clamp next_expire_ms to min(connect‑timeleft, attempt delay), but if happy_eyeballs_timeout is configured very low (0/negative), expire scheduling may hammer the loop. Consider enforcing a small floor (e.g., 1 ms) when >0 attempts are ongoing.
- if(next_expire_ms <= 0) { + if(next_expire_ms <= 0) { failf(data, "Connection timeout after %" FMT_OFF_T " ms", curlx_timediff(now, data->progress.t_startsingle)); return CURLE_OPERATION_TIMEDOUT; } - Curl_expire(data, next_expire_ms, EXPIRE_HAPPY_EYEBALLS); + Curl_expire(data, CURLMAX(next_expire_ms, 1), EXPIRE_HAPPY_EYEBALLS);
748-789: Winner installation is tidy; ensure chain ownership is clear.You detach the winning cf from the attempt, install as cf->next, then clear and stop the EXPIRE_HAPPY_EYEBALLS timer. Good. Add a short comment that the winning sub‑filter must own the socket from here, and ballers’ remaining attempts are already freed in cf_ip_happy_ctx_clear().
930-951: Public insertion helper: validate transport and factory lookup is robust.get_cf_create() lookup and CURLE_UNSUPPORTED_PROTOCOL on unknown transport is correct. Consider logging the chosen transport name for traceability.
deps/curl/lib/hostip.c (5)
183-186: Use consistent time type across the structureThe struct uses
struct curltime nowbuttimediff_tfor age values. While functionally correct, consider whether all time-related fields should use the same representation for consistency.
202-211: Double negative entry aging may lead to premature evictionNegative DNS entries age twice as fast (Line 206:
age *= 2). This could cause legitimate negative entries to be evicted prematurely from the cache, potentially leading to increased DNS queries for non-existent domains.
219-234: Consider extracting cache size management logicThe cache pruning logic with dynamic timeout adjustment (Lines 282-294) is complex and could benefit from being extracted into a separate helper function for better testability and maintainability.
803-823: Consider rate limiting negative DNS cache entriesThe
store_negative_resolvefunction adds negative entries without any rate limiting. A malicious actor could potentially fill the DNS cache with negative entries by requesting many non-existent hostnames.
1643-1656: CURL_DNS_DELAY_MS: documented — add explicit upper bound and overflow guard in hostip.c
- Documentation exists: curl-upstream/docs/libcurl/libcurl-env-dbg.md →
CURL_DNS_DELAY_MS.- In deps/curl/lib/hostip.c (Curl_resolve_test_delay) the value is parsed with curlx_str_number(..., TIME_T_MAX) then passed to curlx_wait_ms((timediff_t)l); clamp/validate
lto a sane maximum and ensure the cast to timediff_t cannot overflow to prevent excessive test delays or undefined behavior.deps/curl/lib/asyn-thrdd.c (1)
231-284: Potential race condition in thread abort checkIn
getaddrinfo_thread, thedo_abortflag is checked after acquiring the mutex (Line 233), but the actual DNS resolution happens outside the mutex protection. This could lead to unnecessary work if abort is signaled during resolution.Consider checking the abort flag again after DNS resolution completes:
rc = Curl_getaddrinfo_ex(addr_ctx->hostname, service, &addr_ctx->hints, &addr_ctx->res); + Curl_mutex_acquire(&addr_ctx->mutx); + do_abort = addr_ctx->do_abort; + Curl_mutex_release(&addr_ctx->mutx); + + if(do_abort) { + if(addr_ctx->res) { + Curl_freeaddrinfo(addr_ctx->res); + addr_ctx->res = NULL; + } + } + else { if(rc) { addr_ctx->sock_error = SOCKERRNO ? SOCKERRNO : rc; if(addr_ctx->sock_error == 0) addr_ctx->sock_error = RESOLVER_ENOMEM; } else { Curl_addrinfo_set_port(addr_ctx->res, addr_ctx->port); } + }deps/curl/lib/ftp.c (1)
4186-4198: Recursive slash counting implementationThe
numof_slashesfunction counts slashes in a string. Consider using a simpler implementation:-static size_t numof_slashes(const char *str) -{ - const char *slashPos; - size_t num = 0; - do { - slashPos = strchr(str, '/'); - if(slashPos) { - ++num; - str = slashPos + 1; - } - } while(slashPos); - return num; -} +static size_t numof_slashes(const char *str) +{ + size_t num = 0; + while(*str) { + if(*str == '/') + num++; + str++; + } + return num; +}deps/curl/lib/curl_setup.h (1)
627-644: Safer SIZE_MAX/SSIZE_MAX fallbacks.Literal constants risk width mismatches on exotic ABIs. Prefer portable derivations.
Suggested tweak:
-#if defined(SIZEOF_SIZE_T) && (SIZEOF_SIZE_T > 4) -#define SIZE_MAX 18446744073709551615U -#else -#define SIZE_MAX 4294967295U -#endif +#ifndef SIZE_MAX +#define SIZE_MAX ((size_t)~(size_t)0) +#endif @@ -#if defined(SIZEOF_SIZE_T) && (SIZEOF_SIZE_T > 4) -#define SSIZE_MAX 9223372036854775807 -#else -#define SSIZE_MAX 2147483647 -#endif +#ifndef SSIZE_MAX +#define SSIZE_MAX ((ssize_t)(SIZE_MAX >> 1)) +#endifdeps/curl/lib/cfilters.c (1)
1064-1078: Return 1 (not 0) when sockindex invalid to preserve default concurrency.The function documents defaulting to non‑multiplexed “1”. Returning 0 may cause callers to treat it as “no capacity”.
Apply this diff:
- if(!CONN_SOCK_IDX_VALID(sockindex)) - return 0; + if(!CONN_SOCK_IDX_VALID(sockindex)) + return 1;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (149)
deps/curl/.editorconfig(1 hunks)deps/curl/include/Makefile.in(1 hunks)deps/curl/include/curl/Makefile.in(1 hunks)deps/curl/include/curl/curl.h(12 hunks)deps/curl/include/curl/curlver.h(3 hunks)deps/curl/include/curl/easy.h(2 hunks)deps/curl/include/curl/header.h(1 hunks)deps/curl/include/curl/mprintf.h(3 hunks)deps/curl/include/curl/multi.h(3 hunks)deps/curl/include/curl/options.h(1 hunks)deps/curl/include/curl/system.h(11 hunks)deps/curl/include/curl/typecheck-gcc.h(8 hunks)deps/curl/include/curl/urlapi.h(1 hunks)deps/curl/include/curl/websockets.h(2 hunks)deps/curl/lib/CMakeLists.txt(4 hunks)deps/curl/lib/Makefile.am(3 hunks)deps/curl/lib/Makefile.in(25 hunks)deps/curl/lib/Makefile.inc(3 hunks)deps/curl/lib/altsvc.c(2 hunks)deps/curl/lib/asyn-ares.c(5 hunks)deps/curl/lib/asyn-base.c(1 hunks)deps/curl/lib/asyn-thrdd.c(18 hunks)deps/curl/lib/asyn.h(6 hunks)deps/curl/lib/bufq.c(4 hunks)deps/curl/lib/cf-h1-proxy.c(2 hunks)deps/curl/lib/cf-h2-proxy.c(10 hunks)deps/curl/lib/cf-h2-proxy.h(1 hunks)deps/curl/lib/cf-haproxy.c(3 hunks)deps/curl/lib/cf-haproxy.h(1 hunks)deps/curl/lib/cf-https-connect.c(5 hunks)deps/curl/lib/cf-https-connect.h(2 hunks)deps/curl/lib/cf-ip-happy.c(1 hunks)deps/curl/lib/cf-ip-happy.h(1 hunks)deps/curl/lib/cf-socket.c(16 hunks)deps/curl/lib/cf-socket.h(0 hunks)deps/curl/lib/cfilters.c(21 hunks)deps/curl/lib/cfilters.h(9 hunks)deps/curl/lib/config-os400.h(0 hunks)deps/curl/lib/config-win32.h(1 hunks)deps/curl/lib/conncache.c(3 hunks)deps/curl/lib/conncache.h(1 hunks)deps/curl/lib/connect.c(5 hunks)deps/curl/lib/connect.h(0 hunks)deps/curl/lib/content_encoding.c(6 hunks)deps/curl/lib/cookie.c(14 hunks)deps/curl/lib/cookie.h(1 hunks)deps/curl/lib/cshutdn.c(3 hunks)deps/curl/lib/curl_addrinfo.c(4 hunks)deps/curl/lib/curl_config.h.cmake(0 hunks)deps/curl/lib/curl_config.h.in(1 hunks)deps/curl/lib/curl_fnmatch.c(1 hunks)deps/curl/lib/curl_gethostname.c(1 hunks)deps/curl/lib/curl_gssapi.c(1 hunks)deps/curl/lib/curl_hmac.h(1 hunks)deps/curl/lib/curl_md4.h(1 hunks)deps/curl/lib/curl_md5.h(1 hunks)deps/curl/lib/curl_mem_undef.h(1 hunks)deps/curl/lib/curl_memory.h(0 hunks)deps/curl/lib/curl_ntlm_core.c(11 hunks)deps/curl/lib/curl_ntlm_core.h(2 hunks)deps/curl/lib/curl_rtmp.c(9 hunks)deps/curl/lib/curl_sasl.c(4 hunks)deps/curl/lib/curl_setup.h(13 hunks)deps/curl/lib/curl_setup_once.h(0 hunks)deps/curl/lib/curl_sha256.h(1 hunks)deps/curl/lib/curl_sha512_256.c(21 hunks)deps/curl/lib/curl_sspi.c(1 hunks)deps/curl/lib/curl_sspi.h(3 hunks)deps/curl/lib/curl_threads.c(5 hunks)deps/curl/lib/curl_threads.h(2 hunks)deps/curl/lib/curl_trc.c(9 hunks)deps/curl/lib/curl_trc.h(3 hunks)deps/curl/lib/curlx/base64.c(0 hunks)deps/curl/lib/curlx/dynbuf.c(4 hunks)deps/curl/lib/curlx/dynbuf.h(1 hunks)deps/curl/lib/curlx/inet_ntop.c(1 hunks)deps/curl/lib/curlx/inet_ntop.h(1 hunks)deps/curl/lib/curlx/inet_pton.h(1 hunks)deps/curl/lib/curlx/multibyte.h(1 hunks)deps/curl/lib/curlx/nonblock.c(1 hunks)deps/curl/lib/curlx/strparse.c(1 hunks)deps/curl/lib/curlx/timeval.c(3 hunks)deps/curl/lib/curlx/wait.c(1 hunks)deps/curl/lib/curlx/warnless.c(3 hunks)deps/curl/lib/cw-out.c(8 hunks)deps/curl/lib/dict.c(4 hunks)deps/curl/lib/dllmain.c(1 hunks)deps/curl/lib/doh.c(13 hunks)deps/curl/lib/doh.h(1 hunks)deps/curl/lib/easy.c(12 hunks)deps/curl/lib/easy_lock.h(1 hunks)deps/curl/lib/easygetopt.c(1 hunks)deps/curl/lib/file.c(6 hunks)deps/curl/lib/fileinfo.c(2 hunks)deps/curl/lib/fopen.c(1 hunks)deps/curl/lib/formdata.c(2 hunks)deps/curl/lib/ftp.c(36 hunks)deps/curl/lib/ftp.h(2 hunks)deps/curl/lib/gopher.c(3 hunks)deps/curl/lib/hmac.c(2 hunks)deps/curl/lib/hostip.c(14 hunks)deps/curl/lib/hostip.h(5 hunks)deps/curl/lib/hostip4.c(4 hunks)deps/curl/lib/hostip6.c(2 hunks)deps/curl/lib/hsts.c(2 hunks)deps/curl/lib/http.c(32 hunks)deps/curl/lib/http.h(3 hunks)deps/curl/lib/http2.c(14 hunks)deps/curl/lib/http_aws_sigv4.c(2 hunks)deps/curl/lib/http_chunks.c(4 hunks)deps/curl/lib/http_digest.c(2 hunks)deps/curl/lib/http_ntlm.c(1 hunks)deps/curl/lib/http_proxy.c(2 hunks)deps/curl/lib/httpsrr.h(1 hunks)deps/curl/lib/idn.c(3 hunks)deps/curl/lib/if2ip.c(3 hunks)deps/curl/lib/imap.c(17 hunks)deps/curl/lib/krb5.c(4 hunks)deps/curl/lib/ldap.c(22 hunks)deps/curl/lib/libcurl.def(2 hunks)deps/curl/lib/md4.c(6 hunks)deps/curl/lib/md5.c(5 hunks)deps/curl/lib/memdebug.c(2 hunks)deps/curl/lib/memdebug.h(1 hunks)deps/curl/lib/mime.c(12 hunks)deps/curl/lib/mqtt.c(3 hunks)deps/curl/lib/multi.c(30 hunks)deps/curl/lib/multi_ev.c(9 hunks)deps/curl/lib/multihandle.h(2 hunks)deps/curl/lib/multiif.h(2 hunks)deps/curl/lib/netrc.c(1 hunks)deps/curl/lib/netrc.h(1 hunks)deps/curl/lib/noproxy.c(3 hunks)deps/curl/lib/openldap.c(5 hunks)deps/curl/lib/optiontable.pl(3 hunks)deps/curl/lib/parsedate.c(3 hunks)deps/curl/lib/parsedate.h(1 hunks)deps/curl/lib/pingpong.c(1 hunks)deps/curl/lib/pingpong.h(1 hunks)deps/curl/lib/pop3.c(13 hunks)deps/curl/lib/progress.c(2 hunks)deps/curl/lib/rand.c(4 hunks)deps/curl/lib/request.c(4 hunks)deps/curl/lib/request.h(0 hunks)deps/curl/lib/rtsp.c(9 hunks)deps/curl/lib/select.c(4 hunks)deps/curl/lib/select.h(1 hunks)deps/curl/lib/sendf.c(15 hunks)deps/curl/lib/sendf.h(5 hunks)
💤 Files with no reviewable changes (8)
- deps/curl/lib/request.h
- deps/curl/lib/cf-socket.h
- deps/curl/lib/config-os400.h
- deps/curl/lib/curl_config.h.cmake
- deps/curl/lib/connect.h
- deps/curl/lib/curl_memory.h
- deps/curl/lib/curlx/base64.c
- deps/curl/lib/curl_setup_once.h
👮 Files not reviewed due to content moderation or server errors (1)
- deps/curl/lib/Makefile.in
🧰 Additional context used
🧬 Code graph analysis (6)
deps/curl/lib/http.h (1)
deps/curl/lib/http.c (2)
Curl_http_method(1810-1852)Curl_http_do_pollset(1519-1524)
deps/curl/lib/http_proxy.c (1)
deps/curl/lib/cf-h2-proxy.c (1)
Curl_cf_h2_proxy_insert_after(1594-1617)
deps/curl/lib/cookie.c (1)
deps/curl/lib/hostip.c (1)
Curl_host_is_ipnum(729-742)
deps/curl/lib/http.c (2)
deps/curl/lib/cookie.c (2)
Curl_cookie_getlist(1311-1407)Curl_secure_context(1292-1298)deps/curl/lib/http2.c (3)
Curl_http2_switch(2886-2906)Curl_http2_may_switch(2868-2884)Curl_http2_request_upgrade(1799-1835)
deps/curl/lib/multi.c (1)
deps/curl/lib/hostip.c (2)
Curl_resolv_pollset(1569-1585)Curl_dnscache_clear(299-307)
deps/curl/lib/easy.c (1)
deps/curl/lib/multi.c (2)
Curl_multi_mark_dirty(3973-3977)Curl_update_timer(3396-3462)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test-tarball-linux
- GitHub Check: lint-js-and-md
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: test-macOS
| /* network has changed, adjust caches/connection reuse */ | ||
| CURLOPT(CURLMOPT_NETWORK_CHANGED, CURLOPTTYPE_LONG, 17), | ||
|
|
||
| CURLMOPT_LASTENTRY /* the last unused */ |
There was a problem hiding this comment.
Duplicate bit value for CLEAR_DNS — breaks flag semantics.
Both CLEAR_CONNS and CLEAR_DNS are (1L<<0). Assign a distinct bit and fix the comment.
Apply:
/* - CURLMNWC_CLEAR_CONNS tells libcurl to prevent further reuse of existing
connections. Connections that are idle will be closed. Ongoing transfers
will continue with the connection they have. */
#define CURLMNWC_CLEAR_CONNS (1L<<0)
-/* - CURLMNWC_CLEAR_DNS tells libcurl to prevent further reuse of existing
- connections. Connections that are idle will be closed. Ongoing transfers
- will continue with the connection they have. */
-#define CURLMNWC_CLEAR_DNS (1L<<0)
+/* - CURLMNWC_CLEAR_DNS tells libcurl to clear DNS-related caches (hosts,
+ Happy Eyeballs hints, etc.). Does not force-close active connections. */
+#define CURLMNWC_CLEAR_DNS (1L<<1)Also applies to: 404-415
🤖 Prompt for AI Agents
In deps/curl/include/curl/multi.h around lines 398-401 (and also review lines
404-415), two flags CLEAR_CONNS and CLEAR_DNS were assigned the same bit value
(1L<<0) breaking flag semantics; change CLEAR_DNS to use a distinct bit (e.g.,
1L<<1 or the next free bit), update any related enum/comment to reflect the
correct bit value and purpose, and ensure any masks or usage elsewhere in this
range are adjusted to use the new bit so flags are unique and the comment
accurately describes the bits.
| for(i = 0; i < ps.n; i++) { | ||
| #ifdef __DJGPP__ | ||
| #pragma GCC diagnostic push | ||
| #pragma GCC diagnostic ignored "-Warith-conversion" | ||
| #endif | ||
| if(ps.actions[i] & CURL_POLL_IN) | ||
| FD_SET(ps.sockets[i], read_fd_set); | ||
| if(ps.actions[i] & CURL_POLL_OUT) | ||
| FD_SET(ps.sockets[i], write_fd_set); | ||
| #if defined(__DJGPP__) | ||
| #ifdef __DJGPP__ | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| if((ps.actions[i] & (CURL_POLL_OUT | CURL_POLL_IN)) && | ||
| ((int)ps.sockets[i] > *maxfd)) | ||
| *maxfd = (int)ps.sockets[i]; | ||
| } | ||
| } | ||
| Curl_pollset_cleanup(&ps); |
There was a problem hiding this comment.
🧩 Analysis chain
Fix unintended behavior change from ps.num to ps.n.
The loop condition changed from ps.num to ps.n, but these appear to be different fields. The struct easy_pollset likely has both n (current count) and num (capacity). Using ps.n here could cause the loop to terminate early if the pollset isn't fully populated.
Please verify the correct field to use:
- for(i = 0; i < ps.n; i++) {
+ for(i = 0; i < ps.num; i++) {🏁 Script executed:
#!/bin/bash
# Find the definition of struct easy_pollset to understand the fields
ast-grep --pattern 'struct easy_pollset {
$$$
}'
# Check other usages of easy_pollset to understand the pattern
rg -n "easy_pollset" --type=c -A 3 -B 3 | head -50Length of output: 2752
Replace ps.n with ps.num in deps/curl/lib/cshutdn.c
struct easy_pollset uses the field named "num" for the current count (other code — e.g. lib/cfilters.c — uses ps->num); using ps.n here will truncate the loop and change behavior.
Location: deps/curl/lib/cshutdn.c (around lines 506–523).
- for(i = 0; i < ps.n; i++) {
+ for(i = 0; i < ps.num; i++) {📝 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.
| for(i = 0; i < ps.n; i++) { | |
| #ifdef __DJGPP__ | |
| #pragma GCC diagnostic push | |
| #pragma GCC diagnostic ignored "-Warith-conversion" | |
| #endif | |
| if(ps.actions[i] & CURL_POLL_IN) | |
| FD_SET(ps.sockets[i], read_fd_set); | |
| if(ps.actions[i] & CURL_POLL_OUT) | |
| FD_SET(ps.sockets[i], write_fd_set); | |
| #if defined(__DJGPP__) | |
| #ifdef __DJGPP__ | |
| #pragma GCC diagnostic pop | |
| #endif | |
| if((ps.actions[i] & (CURL_POLL_OUT | CURL_POLL_IN)) && | |
| ((int)ps.sockets[i] > *maxfd)) | |
| *maxfd = (int)ps.sockets[i]; | |
| } | |
| } | |
| Curl_pollset_cleanup(&ps); | |
| for(i = 0; i < ps.num; i++) { | |
| #ifdef __DJGPP__ | |
| #pragma GCC diagnostic push | |
| #pragma GCC diagnostic ignored "-Warith-conversion" | |
| #endif | |
| if(ps.actions[i] & CURL_POLL_IN) | |
| FD_SET(ps.sockets[i], read_fd_set); | |
| if(ps.actions[i] & CURL_POLL_OUT) | |
| FD_SET(ps.sockets[i], write_fd_set); | |
| #ifdef __DJGPP__ | |
| #pragma GCC diagnostic pop | |
| #endif | |
| if((ps.actions[i] & (CURL_POLL_OUT | CURL_POLL_IN)) && | |
| ((int)ps.sockets[i] > *maxfd)) | |
| *maxfd = (int)ps.sockets[i]; | |
| } | |
| } | |
| Curl_pollset_cleanup(&ps); |
🤖 Prompt for AI Agents
In deps/curl/lib/cshutdn.c around lines 506 to 523, the loop uses ps.n which is
the wrong field name; replace ps.n with ps.num throughout the loop so it
iterates over the actual count field of struct easy_pollset (update the for loop
and any other occurrences in this block), rebuild to ensure no warnings or
behavior changes, and run tests to confirm semantics match other uses (e.g.,
lib/cfilters.c) that reference ps->num.
| #ifndef SEC_E_POLICY_NLTM_ONLY | ||
| # define SEC_E_POLICY_NLTM_ONLY ((HRESULT)0x8009035FL) | ||
| #define SEC_E_POLICY_NLTM_ONLY ((HRESULT)0x8009035FL) | ||
| #endif |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Typo breaks the intended fallback: SEC_E_POLICY_NLTM_ONLY → SEC_E_POLICY_NTLM_ONLY.
The macro name is misspelled in both the #ifndef and #define, so the correct symbol remains undefined and the wrong one gets introduced.
Apply this diff:
-#ifndef SEC_E_POLICY_NLTM_ONLY
-#define SEC_E_POLICY_NLTM_ONLY ((HRESULT)0x8009035FL)
+#ifndef SEC_E_POLICY_NTLM_ONLY
+#define SEC_E_POLICY_NTLM_ONLY ((HRESULT)0x8009035FL)Run to ensure no residual uses of the misspelled symbol and that callers use the correct one:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 $'\bSEC_E_POLICY_NTLM_ONLY\b|\bSEC_E_POLICY_NLTM_ONLY\b'Length of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Check ripgrep availability:"
rg --version 2>/dev/null || echo "rg not available"
echo
echo "Searching repo for both macros (NTLM / NLTM):"
rg -n --hidden --no-ignore -S --color=never '\bSEC_E_POLICY_NTLM_ONLY\b' || true
rg -n --hidden --no-ignore -S --color=never '\bSEC_E_POLICY_NLTM_ONLY\b' || true
echo
echo "Fallback grep (recursive, exclude .git):"
grep -RIn --exclude-dir=.git -E 'SEC_E_POLICY_NTLM_ONLY|SEC_E_POLICY_NLTM_ONLY' . || true
echo
echo "Show snippet around the macro in deps/curl/lib/curl_sspi.h (if present):"
sed -n '288,308p' deps/curl/lib/curl_sspi.h || echo "deps/curl/lib/curl_sspi.h not found"Length of output: 1935
Fix macro typo and update all callers: SEC_E_POLICY_NLTM_ONLY → SEC_E_POLICY_NTLM_ONLY
The header macro is misspelled and is referenced in the tree — update the define and replace all usages found at:
- deps/curl/lib/curl_sspi.h:301-302
- deps/curl/lib/strerror.c:941
- curl-upstream/lib/curl_sspi.h:301-302
- curl-upstream/lib/strerror.c:941
Apply this header change and update callers accordingly:
-#ifndef SEC_E_POLICY_NLTM_ONLY
-#define SEC_E_POLICY_NLTM_ONLY ((HRESULT)0x8009035FL)
+#ifndef SEC_E_POLICY_NTLM_ONLY
+#define SEC_E_POLICY_NTLM_ONLY ((HRESULT)0x8009035FL)📝 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.
| #ifndef SEC_E_POLICY_NLTM_ONLY | |
| # define SEC_E_POLICY_NLTM_ONLY ((HRESULT)0x8009035FL) | |
| #define SEC_E_POLICY_NLTM_ONLY ((HRESULT)0x8009035FL) | |
| #endif | |
| #ifndef SEC_E_POLICY_NTLM_ONLY | |
| #define SEC_E_POLICY_NTLM_ONLY ((HRESULT)0x8009035FL) | |
| #endif |
🤖 Prompt for AI Agents
In deps/curl/lib/curl_sspi.h around lines 301-303 the macro name is misspelled
SEC_E_POLICY_NLTM_ONLY; change the define to SEC_E_POLICY_NTLM_ONLY and update
all callers to use the corrected macro name. Edit deps/curl/lib/strerror.c
(around line 941), curl-upstream/lib/curl_sspi.h (around lines 301-302) and
curl-upstream/lib/strerror.c (around line 941) to replace any occurrences of
SEC_E_POLICY_NLTM_ONLY with SEC_E_POLICY_NTLM_ONLY, verify includes still
resolve, and run a build/check to ensure no remaining references to the old
macro remain.
| CURLcode result = CURLE_OK; | ||
|
|
||
| if(curlx_dyn_len(&cwbuf->b)) { | ||
| if(curlx_dyn_len(&cwbuf->b) || (cwbuf->type == CW_OUT_BODY_0LEN)) { | ||
| size_t consumed; | ||
|
|
||
| result = cw_out_ptr_flush(ctx, data, cwbuf->type, flush_all, | ||
| curlx_dyn_ptr(&cwbuf->b), | ||
| curlx_dyn_len(&cwbuf->b), | ||
| &consumed); | ||
| if(result) | ||
| if(result && (result != CURLE_AGAIN)) | ||
| return result; | ||
|
|
There was a problem hiding this comment.
Bug: CURLE_AGAIN from buffered flush bubbles up as an error
cw_out_buf_flush may return CURLE_AGAIN, which cw_out_flush_chain propagates; callers (cw_out_do_write, cw_out_flush) then treat it as an error and set ctx->errored, dropping buffers and breaking pause/resume.
Handle CURLE_AGAIN like the direct write path (non‑error, keep buffers).
Apply these diffs:
--- a/deps/curl/lib/cw-out.c
+++ b/deps/curl/lib/cw-out.c
@@ -404,9 +404,11 @@ static CURLcode cw_out_do_write(struct cw_out_ctx *ctx,
- result = cw_out_flush_chain(ctx, data, &ctx->buf, flush_all);
- if(result)
- goto out;
+ result = cw_out_flush_chain(ctx, data, &ctx->buf, flush_all);
+ if(result && (result != CURLE_AGAIN))
+ goto out;
+ /* treat PAUSE as non-error; keep buffered data */
+ result = CURLE_OK;
@@ -488,12 +490,14 @@ static CURLcode cw_out_flush(struct Curl_easy *data,
- result = cw_out_flush_chain(ctx, data, &ctx->buf, flush_all);
- if(result) {
- ctx->errored = TRUE;
- cw_out_bufs_free(ctx);
- return result;
- }
- return result;
+ result = cw_out_flush_chain(ctx, data, &ctx->buf, flush_all);
+ if(result && (result != CURLE_AGAIN)) {
+ ctx->errored = TRUE;
+ cw_out_bufs_free(ctx);
+ return result;
+ }
+ /* OK or PAUSE -> not an error */
+ return CURLE_OK;📝 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.
| CURLcode result = CURLE_OK; | |
| if(curlx_dyn_len(&cwbuf->b)) { | |
| if(curlx_dyn_len(&cwbuf->b) || (cwbuf->type == CW_OUT_BODY_0LEN)) { | |
| size_t consumed; | |
| result = cw_out_ptr_flush(ctx, data, cwbuf->type, flush_all, | |
| curlx_dyn_ptr(&cwbuf->b), | |
| curlx_dyn_len(&cwbuf->b), | |
| &consumed); | |
| if(result) | |
| if(result && (result != CURLE_AGAIN)) | |
| return result; | |
| result = cw_out_flush_chain(ctx, data, &ctx->buf, flush_all); | |
| if(result && (result != CURLE_AGAIN)) | |
| goto out; | |
| /* treat PAUSE as non-error; keep buffered data */ | |
| result = CURLE_OK; |
| CURLcode result = CURLE_OK; | |
| if(curlx_dyn_len(&cwbuf->b)) { | |
| if(curlx_dyn_len(&cwbuf->b) || (cwbuf->type == CW_OUT_BODY_0LEN)) { | |
| size_t consumed; | |
| result = cw_out_ptr_flush(ctx, data, cwbuf->type, flush_all, | |
| curlx_dyn_ptr(&cwbuf->b), | |
| curlx_dyn_len(&cwbuf->b), | |
| &consumed); | |
| if(result) | |
| if(result && (result != CURLE_AGAIN)) | |
| return result; | |
| result = cw_out_flush_chain(ctx, data, &ctx->buf, flush_all); | |
| if(result && (result != CURLE_AGAIN)) { | |
| ctx->errored = TRUE; | |
| cw_out_bufs_free(ctx); | |
| return result; | |
| } | |
| /* OK or PAUSE -> not an error */ | |
| return CURLE_OK; |
| if(hashparams->hinit(ctxt->hashctxt1)) | ||
| return NULL; | ||
| hashparams->hupdate(ctxt->hashctxt1, key, keylen); |
There was a problem hiding this comment.
Free ctxt on early-return to avoid a leak.
If hinit fails here, ctxt is leaked.
Apply this diff:
- if(hashparams->hinit(ctxt->hashctxt1))
- return NULL;
+ if(hashparams->hinit(ctxt->hashctxt1)) {
+ free(ctxt);
+ return NULL;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if(hashparams->hinit(ctxt->hashctxt1)) | |
| return NULL; | |
| hashparams->hupdate(ctxt->hashctxt1, key, keylen); | |
| if(hashparams->hinit(ctxt->hashctxt1)) { | |
| free(ctxt); | |
| return NULL; | |
| } | |
| hashparams->hupdate(ctxt->hashctxt1, key, keylen); |
🤖 Prompt for AI Agents
In deps/curl/lib/hmac.c around lines 76 to 78, the function returns NULL when
hashparams->hinit(ctxt->hashctxt1) fails but does not free the allocated ctxt,
leaking memory; fix by freeing ctxt (and any of its owned allocations if
applicable) before returning NULL so the early-return cleans up allocated
resources and then return NULL as before.
| case CF_QUERY_ALPN_NEGOTIATED: { | ||
| const char **palpn = pres2; | ||
| DEBUGASSERT(palpn); | ||
| *palpn = NULL; | ||
| return CURLE_OK; | ||
| } |
There was a problem hiding this comment.
CF_QUERY_ALPN_NEGOTIATED should forward downstream instead of returning NULL.
Current code returns CURLE_OK with NULL, shadowing a negotiated ALPN from lower filters. Forward the query to cf->next and only default to unknown when there is no next.
Apply:
case CF_QUERY_ALPN_NEGOTIATED: {
- const char **palpn = pres2;
- DEBUGASSERT(palpn);
- *palpn = NULL;
- return CURLE_OK;
+ return cf->next ?
+ cf->next->cft->query(cf->next, data, query, pres1, pres2) :
+ CURLE_UNKNOWN_OPTION;
}🤖 Prompt for AI Agents
In deps/curl/lib/http_proxy.c around lines 395-400, the CF_QUERY_ALPN_NEGOTIATED
case currently forces *palpn = NULL and returns CURLE_OK, which hides any ALPN
negotiated by downstream filters; change it to forward the
CF_QUERY_ALPN_NEGOTIATED query to the next filter when cf->next is present (by
invoking the next filter's query handler with the same parameters) and return
its result, and only set *palpn = NULL and return CURLE_OK when there is no next
filter to consult.
| /* add the easy handle to the process set */ | ||
| Curl_uint_bset_add(&multi->process, data->mid); | ||
| ++multi->xfers_alive; | ||
| ++multi->xfers_total_ever; | ||
|
|
||
| Curl_cpool_xfer_init(data); | ||
| multi_warn_debug(multi, data); | ||
|
|
||
| /* Make sure the new handle will run */ | ||
| Curl_multi_mark_dirty(data); | ||
| /* Necessary in event based processing, where dirty handles trigger | ||
| * a timeout callback invocation. */ | ||
| rc = Curl_update_timer(multi); | ||
| if(rc) { | ||
| data->multi = NULL; /* not anymore */ | ||
| Curl_uint_tbl_remove(&multi->xfers, data->mid); | ||
| data->mid = UINT_MAX; | ||
| return rc; | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix rollback when Curl_update_timer() fails in curl_multi_add_handle.
If Curl_update_timer() returns non‑zero, we detach the easy and remove it from xfers, but:
- multi->xfers_alive is not decremented (it was incremented at Line 481).
- The easy’s bits remain set in multi->process and multi->dirty.
- xfers_total_ever was already incremented at Line 482, so CURLMINFO_XFERS_ADDED will overcount failed adds.
This leaves inconsistent counters/bitsets and can cause spurious scheduling and incorrect metrics. Either (a) move the increments after the successful timer update, or (b) fully roll back on error.
Apply one of the following diffs.
Option A (preferred: increment only after success):
@@
- Curl_uint_bset_add(&multi->process, data->mid);
- ++multi->xfers_alive;
- ++multi->xfers_total_ever;
+ Curl_uint_bset_add(&multi->process, data->mid);
+ /* xfers_alive / xfers_total_ever are updated after timer install succeeds */
@@
- rc = Curl_update_timer(multi);
+ rc = Curl_update_timer(multi);
if(rc) {
data->multi = NULL; /* not anymore */
- Curl_uint_tbl_remove(&multi->xfers, data->mid);
- data->mid = UINT_MAX;
+ {
+ unsigned int mid = data->mid;
+ Curl_uint_tbl_remove(&multi->xfers, mid);
+ Curl_uint_bset_remove(&multi->process, mid);
+ Curl_uint_bset_remove(&multi->dirty, mid);
+ data->mid = UINT_MAX;
+ }
return rc;
}
+ ++multi->xfers_alive;
+ ++multi->xfers_total_ever;Option B (complete rollback on error):
@@
- rc = Curl_update_timer(multi);
+ rc = Curl_update_timer(multi);
if(rc) {
- data->multi = NULL; /* not anymore */
- Curl_uint_tbl_remove(&multi->xfers, data->mid);
- data->mid = UINT_MAX;
+ unsigned int mid = data->mid;
+ --multi->xfers_alive;
+ Curl_uint_bset_remove(&multi->process, mid);
+ Curl_uint_bset_remove(&multi->dirty, mid);
+ data->multi = NULL; /* not anymore */
+ Curl_uint_tbl_remove(&multi->xfers, mid);
+ data->mid = UINT_MAX;
+ /* Optionally also revert xfers_total_ever if you intend it to track only
+ successfully added transfers. */
return rc;
}| unsigned int xfers_alive; /* amount of added transfers that have | ||
| not yet reached COMPLETE state */ | ||
| curl_off_t xfers_total_ever; /* total of added transfers, ever. */ | ||
| struct uint_tbl xfers; /* transfers added to this multi */ | ||
| /* Each transfer's mid may be present in at most one of these */ | ||
| struct uint_bset process; /* transfer being processed */ |
There was a problem hiding this comment.
🧩 Analysis chain
New accounting field must be initialized and maintained.
Ensure xfers_total_ever is zeroed on multi init and incremented exactly once per added transfer (and never decremented).
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Where xfers_total_ever is initialized/updated:"
rg -n 'xfers_total_ever'
echo
echo "Check multi initialization:"
rg -nP '(multi|Curl_multi)_init|Curl_multi_handle|multi_new|multi_init'Length of output: 15414
Initialize and maintain xfers_total_ever correctly
xfers_total_ever is declared in multihandle.h and currently incremented in curl_multi_add_handle (++multi->xfers_total_ever at deps/curl/lib/multi.c:482) and exposed via CURLMINFO_XFERS_ADDED (deps/curl/lib/multi.c:3783). The increment happens before the function completes; if the add later rolls back (e.g. Curl_update_timer fails and the transfer is removed) the code removes the transfer but does not undo the ++, causing the “total ever” to overcount failed adds. Either move the increment to after the add is guaranteed to succeed, or decrement on all rollback paths so the counter strictly reflects one successful increment per added transfer.
| static CURLcode rtsp_setup_body(struct Curl_easy *data, | ||
| Curl_RtspReq rtspreq, | ||
| struct dynbuf *reqp) | ||
| { | ||
| CURLcode result; | ||
| if(rtspreq == RTSPREQ_ANNOUNCE || | ||
| rtspreq == RTSPREQ_SET_PARAMETER || | ||
| rtspreq == RTSPREQ_GET_PARAMETER) { | ||
| curl_off_t req_clen; /* request content length */ | ||
|
|
||
| if(data->state.upload) { | ||
| req_clen = data->state.infilesize; | ||
| data->state.httpreq = HTTPREQ_PUT; | ||
| result = Curl_creader_set_fread(data, req_clen); | ||
| if(result) | ||
| return result; | ||
| } | ||
| else { | ||
| if(data->set.postfields) { | ||
| size_t plen = strlen(data->set.postfields); | ||
| req_clen = (curl_off_t)plen; | ||
| result = Curl_creader_set_buf(data, data->set.postfields, plen); | ||
| } | ||
| else if(data->state.infilesize >= 0) { | ||
| req_clen = data->state.infilesize; | ||
| result = Curl_creader_set_fread(data, req_clen); | ||
| } | ||
| else { | ||
| req_clen = 0; | ||
| result = Curl_creader_set_null(data); | ||
| } | ||
| if(result) | ||
| return result; | ||
| } | ||
|
|
||
| if(req_clen > 0) { | ||
| /* As stated in the http comments, it is probably not wise to | ||
| * actually set a custom Content-Length in the headers */ | ||
| if(!Curl_checkheaders(data, STRCONST("Content-Length"))) { | ||
| result = curlx_dyn_addf(reqp, "Content-Length: %" FMT_OFF_T"\r\n", | ||
| req_clen); | ||
| if(result) | ||
| return result; | ||
| } | ||
|
|
||
| if(rtspreq == RTSPREQ_SET_PARAMETER || | ||
| rtspreq == RTSPREQ_GET_PARAMETER) { | ||
| if(!Curl_checkheaders(data, STRCONST("Content-Type"))) { | ||
| result = curlx_dyn_addn(reqp, | ||
| STRCONST("Content-Type: " | ||
| "text/parameters\r\n")); | ||
| if(result) | ||
| return result; | ||
| } | ||
| } | ||
|
|
||
| if(rtspreq == RTSPREQ_ANNOUNCE) { | ||
| if(!Curl_checkheaders(data, STRCONST("Content-Type"))) { | ||
| result = curlx_dyn_addn(reqp, | ||
| STRCONST("Content-Type: " | ||
| "application/sdp\r\n")); | ||
| if(result) | ||
| return result; | ||
| } | ||
| } | ||
| } | ||
| else if(rtspreq == RTSPREQ_GET_PARAMETER) { | ||
| /* Check for an empty GET_PARAMETER (heartbeat) request */ | ||
| data->state.httpreq = HTTPREQ_HEAD; | ||
| data->req.no_body = TRUE; | ||
| } | ||
| } | ||
| else | ||
| result = Curl_creader_set_null(data); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Respect CURLOPT_POSTFIELDSIZE for RTSP request bodies.
rtsp_setup_body() uses strlen() on postfields and ignores postfieldsize, preventing binary payloads and truncating data with embedded NULs.
Apply this diff:
- if(data->set.postfields) {
- size_t plen = strlen(data->set.postfields);
- req_clen = (curl_off_t)plen;
- result = Curl_creader_set_buf(data, data->set.postfields, plen);
+ if(data->set.postfields) {
+ size_t plen;
+ if(data->set.postfieldsize >= 0)
+ plen = (size_t)data->set.postfieldsize;
+ else
+ plen = strlen(data->set.postfields);
+ req_clen = (curl_off_t)plen;
+ result = Curl_creader_set_buf(data, data->set.postfields, plen);📝 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.
| static CURLcode rtsp_setup_body(struct Curl_easy *data, | |
| Curl_RtspReq rtspreq, | |
| struct dynbuf *reqp) | |
| { | |
| CURLcode result; | |
| if(rtspreq == RTSPREQ_ANNOUNCE || | |
| rtspreq == RTSPREQ_SET_PARAMETER || | |
| rtspreq == RTSPREQ_GET_PARAMETER) { | |
| curl_off_t req_clen; /* request content length */ | |
| if(data->state.upload) { | |
| req_clen = data->state.infilesize; | |
| data->state.httpreq = HTTPREQ_PUT; | |
| result = Curl_creader_set_fread(data, req_clen); | |
| if(result) | |
| return result; | |
| } | |
| else { | |
| if(data->set.postfields) { | |
| size_t plen = strlen(data->set.postfields); | |
| req_clen = (curl_off_t)plen; | |
| result = Curl_creader_set_buf(data, data->set.postfields, plen); | |
| } | |
| else if(data->state.infilesize >= 0) { | |
| req_clen = data->state.infilesize; | |
| result = Curl_creader_set_fread(data, req_clen); | |
| } | |
| else { | |
| req_clen = 0; | |
| result = Curl_creader_set_null(data); | |
| } | |
| if(result) | |
| return result; | |
| } | |
| if(req_clen > 0) { | |
| /* As stated in the http comments, it is probably not wise to | |
| * actually set a custom Content-Length in the headers */ | |
| if(!Curl_checkheaders(data, STRCONST("Content-Length"))) { | |
| result = curlx_dyn_addf(reqp, "Content-Length: %" FMT_OFF_T"\r\n", | |
| req_clen); | |
| if(result) | |
| return result; | |
| } | |
| if(rtspreq == RTSPREQ_SET_PARAMETER || | |
| rtspreq == RTSPREQ_GET_PARAMETER) { | |
| if(!Curl_checkheaders(data, STRCONST("Content-Type"))) { | |
| result = curlx_dyn_addn(reqp, | |
| STRCONST("Content-Type: " | |
| "text/parameters\r\n")); | |
| if(result) | |
| return result; | |
| } | |
| } | |
| if(rtspreq == RTSPREQ_ANNOUNCE) { | |
| if(!Curl_checkheaders(data, STRCONST("Content-Type"))) { | |
| result = curlx_dyn_addn(reqp, | |
| STRCONST("Content-Type: " | |
| "application/sdp\r\n")); | |
| if(result) | |
| return result; | |
| } | |
| } | |
| } | |
| else if(rtspreq == RTSPREQ_GET_PARAMETER) { | |
| /* Check for an empty GET_PARAMETER (heartbeat) request */ | |
| data->state.httpreq = HTTPREQ_HEAD; | |
| data->req.no_body = TRUE; | |
| } | |
| } | |
| else | |
| result = Curl_creader_set_null(data); | |
| return result; | |
| } | |
| static CURLcode rtsp_setup_body(struct Curl_easy *data, | |
| Curl_RtspReq rtspreq, | |
| struct dynbuf *reqp) | |
| { | |
| CURLcode result; | |
| if(rtspreq == RTSPREQ_ANNOUNCE || | |
| rtspreq == RTSPREQ_SET_PARAMETER || | |
| rtspreq == RTSPREQ_GET_PARAMETER) { | |
| curl_off_t req_clen; /* request content length */ | |
| if(data->state.upload) { | |
| req_clen = data->state.infilesize; | |
| data->state.httpreq = HTTPREQ_PUT; | |
| result = Curl_creader_set_fread(data, req_clen); | |
| if(result) | |
| return result; | |
| } | |
| else { | |
| if(data->set.postfields) { | |
| size_t plen; | |
| if(data->set.postfieldsize >= 0) | |
| plen = (size_t)data->set.postfieldsize; | |
| else | |
| plen = strlen(data->set.postfields); | |
| req_clen = (curl_off_t)plen; | |
| result = Curl_creader_set_buf(data, data->set.postfields, plen); | |
| } | |
| else if(data->state.infilesize >= 0) { | |
| req_clen = data->state.infilesize; | |
| result = Curl_creader_set_fread(data, req_clen); | |
| } | |
| else { | |
| req_clen = 0; | |
| result = Curl_creader_set_null(data); | |
| } | |
| if(result) | |
| return result; | |
| } | |
| if(req_clen > 0) { | |
| /* As stated in the http comments, it is probably not wise to | |
| * actually set a custom Content-Length in the headers */ | |
| if(!Curl_checkheaders(data, STRCONST("Content-Length"))) { | |
| result = curlx_dyn_addf(reqp, "Content-Length: %" FMT_OFF_T"\r\n", | |
| req_clen); | |
| if(result) | |
| return result; | |
| } | |
| if(rtspreq == RTSPREQ_SET_PARAMETER || | |
| rtspreq == RTSPREQ_GET_PARAMETER) { | |
| if(!Curl_checkheaders(data, STRCONST("Content-Type"))) { | |
| result = curlx_dyn_addn(reqp, | |
| STRCONST("Content-Type: " | |
| "text/parameters\r\n")); | |
| if(result) | |
| return result; | |
| } | |
| } | |
| if(rtspreq == RTSPREQ_ANNOUNCE) { | |
| if(!Curl_checkheaders(data, STRCONST("Content-Type"))) { | |
| result = curlx_dyn_addn(reqp, | |
| STRCONST("Content-Type: " | |
| "application/sdp\r\n")); | |
| if(result) | |
| return result; | |
| } | |
| } | |
| } | |
| else if(rtspreq == RTSPREQ_GET_PARAMETER) { | |
| /* Check for an empty GET_PARAMETER (heartbeat) request */ | |
| data->state.httpreq = HTTPREQ_HEAD; | |
| data->req.no_body = TRUE; | |
| } | |
| } | |
| else | |
| result = Curl_creader_set_null(data); | |
| return result; | |
| } |
🤖 Prompt for AI Agents
In deps/curl/lib/rtsp.c around lines 282 to 357, rtsp_setup_body() currently
uses strlen(data->set.postfields) and ignores CURLOPT_POSTFIELDSIZE; change it
to respect data->set.postfieldsize when the caller provided it (use that size as
the payload length and pass it to Curl_creader_set_buf), falling back to
strlen() only when postfieldsize is not set/negative; ensure you cast/validate
the curl_off_t to size_t safely and set req_clen from the postfieldsize value so
binary payloads and embedded NULs are handled correctly.
| if(err != -1) | ||
| /* successful rewind */ | ||
| return CURLE_OK; | ||
| } |
There was a problem hiding this comment.
Bug: incorrect fseek() success check (treats errors as success)
fseek returns 0 on success and non‑zero on failure. if (err != -1) will wrongly treat most failures as success. Fix to if (err == 0).
- if(err != -1)
- /* successful rewind */
- return CURLE_OK;
+ if(err == 0)
+ /* successful rewind */
+ return CURLE_OK;📝 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(err != -1) | |
| /* successful rewind */ | |
| return CURLE_OK; | |
| } | |
| if(err == 0) | |
| /* successful rewind */ | |
| return CURLE_OK; | |
| } |
🤖 Prompt for AI Agents
In deps/curl/lib/sendf.c around lines 885 to 888, the fseek() success check is
inverted: it currently treats any err value not equal to -1 as success. Change
the conditional to check for zero (err == 0) since fseek returns 0 on success
and non‑zero on failure, so that only successful rewinds return CURLE_OK; leave
failure handling untouched.
santigimeno
changed the base branch from
node-v22.x-nsolid-v5.x
to
node-v22.x-nsolid-v6.x
2 participants
Summary by CodeRabbit