deps: update libcurl to 8.13.0 #307
Conversation
|
I ran into an unexpected issue while reviewing this PR. Please try again later. |
WalkthroughThis update introduces a comprehensive refactoring and extension of libcurl’s internal connection, event, and shutdown management. It replaces legacy socket hash and shutdown queue systems with new subsystems for event-based socket tracking ( Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Multi as Curl_multi
participant Admin as Admin Easy Handle
participant MultiEv as multi_ev (event subsystem)
participant CPool as cpool (connection pool)
participant CShut as cshutdn (shutdown subsystem)
participant Conn as connectdata
App->>Multi: Add easy handle
Multi->>Admin: Initialize admin handle
Multi->>MultiEv: Initialize event subsystem
Multi->>CPool: Initialize connection pool
Multi->>CShut: Initialize shutdown subsystem
App->>Multi: Perform transfer
Multi->>MultiEv: Assess transfer pollset
MultiEv->>App: Invoke socket callback if needed
App->>Multi: Remove easy handle
Multi->>MultiEv: Cleanup transfer pollset
Multi->>CShut: Add connection to shutdown list if needed
loop Connection shutdown
CShut->>Conn: Run shutdown handler
CShut->>Conn: Attempt socket shutdown
alt Shutdown complete
CShut->>CPool: Remove and free connection
else Timeout or error
CShut->>Conn: Force close connection
end
end
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: 38
🔭 Outside diff range comments (15)
deps/curl/lib/ftplistparser.c (1)
445-452: 🛠️ Refactor suggestionEnsure numeric value follows the “total ” token
Curl_str_passblanks()now skips the whitespace, but the subsequentwhile(ISDIGIT(*endptr))loop silently allows a zero–length digit run (e.g. a line that says justtotal <eol>).
Because no explicit “no-digits” check is performed, such malformed listings are accepted and will advance the state machine as if the directory size were valid.
Consider verifying that at least one digit was consumed and flagging an error otherwise.- Curl_str_passblanks(&endptr); - while(ISDIGIT(*endptr)) - endptr++; - if(*endptr) { + Curl_str_passblanks(&endptr); + const char *digits_start = endptr; + while(ISDIGIT(*endptr)) + endptr++; + if(digits_start == endptr || *endptr) { parser->error = CURLE_FTP_BAD_FILE_LIST; goto fail; }deps/curl/lib/cf-h2-proxy.c (1)
621-628:⚠️ Potential issuePotential off-by-one write when the GOAWAY reason is
CURL_ARRAYSIZEbytesWhen
frame->goaway.opaque_data_len < s_lenthe variablelencan become
128(the full buffer size). Writingscratch[len] = '\0'then writes
past the end ofscratch[0..127].A simpler, always-safe variant:
-size_t s_len = CURL_ARRAYSIZE(scratch); -size_t len = (frame->goaway.opaque_data_len < s_len) ? - frame->goaway.opaque_data_len : s_len-1; +size_t len = CURLMIN(frame->goaway.opaque_data_len, + CURL_ARRAYSIZE(scratch) - 1);This keeps the copy bounded and guarantees a NUL terminator.
deps/curl/lib/mqtt.c (1)
274-290: 🛠️ Refactor suggestionMQTT remaining-length check is off by the fixed header
The spec caps the “Remaining Length” field at
268 435 455
(0x0FFFFFFF), not the total packet length_.
You comparepacketlenagainst that limit, which already includes the
fixed header and therefore rejects certain legal frames (e.g. a CONNECT
with large payload but small fixed header).Fix by checking
payloadleninstead:- if(packetlen > 0x0FFFFFFF) + if(payloadlen > 0x0FFFFFFF) return CURLE_WEIRD_SERVER_REPLY;deps/curl/lib/sendf.c (1)
431-457: 🛠️ Refactor suggestion
⚠️ Potential issueEnsure writer-stack cleanup on early failures to avoid leaks and inconsistent state
do_init_writer_stack()builds the writer stack incrementally, but on any failure after the initial OUT-writer is attached (data->req.writer_stack), the function returns immediately without tearing the partial stack down.
That leaves:
- A memory leak (the already-allocated writer(s) remain allocated).
- A half-initialised stack that later calls implicitly treat as fully-initialised, leading to missing stages (
pause,download,raw) and undefined behaviour.@@ do_init_writer_stack(struct Curl_easy *data) result = Curl_cwriter_create(&data->req.writer_stack, data, &Curl_cwt_out, CURL_CW_CLIENT); if(result) - return result; + return result; /* nothing to clean up yet */ ... - if(result) - return result; + if(result) { + cl_reset_writer(data); /* <<< free the partial stack */ + return result; + } ... - if(result) - return result; + if(result) { + cl_reset_writer(data); /* <<< free the partial stack */ + return result; + }Applying the same pattern for every early-exit in this function guarantees that the caller either receives a fully-constructed stack or none at all.
deps/curl/lib/curl_setup_once.h (1)
298-302: 🛠️ Refactor suggestionMissing
<assert.h>include for new DEBUGASSERT macro
DEBUGASSERT(x)now expands toassert(x)but this header doesn’t include<assert.h>. While most TU’s will pick it up transitively, a standalone include may fail in debug builds.+#include <assert.h>just before the
#ifdef DEBUGBUILDblock keeps the header self-contained.deps/curl/lib/asyn-thread.c (2)
161-180: 🛠️ Refactor suggestionPotential double-cleanup & missing NULL-guard for
hostname
destroy_thread_sync_data()callsfree(tsd->hostname)unconditionally and subsequentlymemset()s the entire struct to zero.
Two issues worth tightening up:
- If the function is (accidentally) invoked twice for the same
tsd, the second call will pass an already–zeroed pointer tofree(), resulting in undefined behaviour.free()on a NULL pointer is safe but the extra defensive check costs nothing and avoids latent bugs ifmemset()some day moves below thefree().- Curl_mutex_destroy(&tsd->mutx); - - free(tsd->hostname); + Curl_mutex_destroy(&tsd->mutx); + + if(tsd->hostname) + free(tsd->hostname);Consider also setting
tsd->hostname = NULLbefore the finalmemset()to guard against future edits that might move thememset()call.
193-199:⚠️ Potential issueRace-window while flagging
done
tsd->doneis set toTRUEhere so that early cleanup works, but it is flipped back toFALSElater ininit_resolve_thread()(line 451).
If the spawning thread is descheduled between these two writes the worker thread could start, seedone == TRUEand immediately free the sync data – leading to a use-after-free when the creator resumes.A safer pattern is:
- /* Treat the request as done until the thread actually starts ... */ - tsd->done = TRUE; + /* Mark as *not* done; the creator owns the lifetime until the thread flips + this to TRUE. */ + tsd->done = FALSE;and drop the later reset at line 451 entirely.
deps/curl/lib/doh.c (2)
1017-1032:⚠️ Potential issueGuard against NULL
*bufto avoid a potential seg-fault
doh_decode_rdata_name()validatesbufitself, but not the value it points to.
If the caller passes aNULLbuffer pointer (*buf == NULL),cp = *buf;will dereference a NULL pointer and crash.- DEBUGASSERT(buf && remaining && dnsname); - if(!buf || !remaining || !dnsname || !*remaining) + DEBUGASSERT(buf && remaining && dnsname); + /* Verify that not only the container pointers are non-NULL, + * but that the pointed-to buffer is valid as well. */ + if(!buf || !*buf || !remaining || !dnsname || !*remaining) return CURLE_OUT_OF_MEMORY;A very small check, but it prevents an exploitable crash path for malformed packets or fuzzing inputs.
1110-1124: 🛠️ Refactor suggestionOff-by-one check in HTTPS RR option loop
expected_min_pcodestarts at 0 and after each option you set
expected_min_pcode = pcode + 1;.If the record contains the very last legal code
65535,pcode + 1will overflow the
uint16_tdomain and quietly wrap to 0, allowing a second round-trip through the loop
without triggering the< expected_min_pcodeguard.Consider promoting
pcodetouint32_tbefore the+ 1, or clamp atUINT16_MAX:- expected_min_pcode = pcode + 1; + expected_min_pcode = (pcode == 0xFFFF) ? 0x10000 : (uint32_t)pcode + 1;This guarantees strict monotone increase even at the edge.
deps/curl/lib/cw-out.c (1)
430-441:⚠️ Potential issueIncorrect casting –
ctxdoes not point to the writer’s context
Curl_cwriter_get_by_type()returns astruct Curl_cwriter *, not the
writer’s context itself. Casting the returned pointer directly to
struct cw_out_ctx *invokes UB and currently masks real bugs because
the first member ofCurl_cwriterhappens to be the embedded
CURLcode (*write)()function pointer.Fix by dereferencing the
ctxfield explicitly everywhere:- ctx = (struct cw_out_ctx *)cw_out; + ctx = (struct cw_out_ctx *)cw_out->ctx;Apply the same change in:
cw_out_flush()(line 447)Curl_cw_out_unpause()(line 472)- struct cw_out_ctx *ctx = (struct cw_out_ctx *)cw_out; + struct cw_out_ctx *ctx = (struct cw_out_ctx *)cw_out->ctx; ... - struct cw_out_ctx *ctx = (struct cw_out_ctx *)cw_out; + struct cw_out_ctx *ctx = (struct cw_out_ctx *)cw_out->ctx;Failing to do so will break once the layout of
struct Curl_cwriter
changes or when ASan/UBSan is enabled.Also applies to: 444-456, 467-476
deps/curl/lib/imap.c (1)
1864-1879: 🛠️ Refactor suggestionPotential leak on early exit in
imap_atom()When any
Curl_dyn_addn()fails the function returnsNULLwithout
callingCurl_dyn_free(&line), leaking the temporary buffer.if((*str == '\\' || *str == '"') && Curl_dyn_addn(&line, "\\", 1)) - return NULL; + { + Curl_dyn_free(&line); + return NULL; + }Repeat the pattern for every early-return inside the loop and for the
initial/closing quote additions.deps/curl/lib/hash.c (1)
364-402: 🛠️ Refactor suggestionIterator skips first element in every bucket
Curl_hash_next_elementadvancesiter->currentbefore returning it,
which means the very first call afterCurl_hash_start_iteratereturns
the second element of the first non-empty bucket (or NULL).Move the
iter->current = iter->current->next;statement after
processing the current pointer.deps/curl/lib/cfilters.c (1)
347-360:⚠️ Potential issueWrong pointer comparison removes the wrong filter and corrupts the chain
Curl_conn_cf_discard_subcompares againstcfinstead ofdiscardwhen
traversing the sub-chain, so the target filter is never found.
Whendestroy_alwaysis true this leads to freeingdiscardwhile it is
still linked, leaving a dangling pointer and use-after-free on the next
access.- if(*pprev == cf) { + if(*pprev == discard) {This is a critical memory-safety issue: please patch immediately and add a
unit-test that discards a middle filter.deps/curl/lib/ftp.c (1)
916-944:⚠️ Potential issuePossible false-negative when parsing
[ipv6]:port-rangeThe new call to
curlx_inet_pton(AF_INET6, string_ftpport, …)still feeds in
string_ftpport, which (for bracketed literals) includes the leading'['and
trailing']'.
curlx_inet_pton()expects a pure address literal; brackets will make it
return0(failure) so the code falls back to the port-range path even
though the address was perfectly valid.- if(curlx_inet_pton(AF_INET6, string_ftpport, &sa6->sin6_addr) == 1) { + if(curlx_inet_pton(AF_INET6, ip_start, &sa6->sin6_addr) == 1) {
ip_startalready points to the first character after'['and*ip_endis
']', so the call succeeds without the need for an intermediate
memcpy+NUL-terminate.deps/curl/lib/conncache.c (1)
80-83:⚠️ Potential issueIncorrect type for
destturns the “flexible array” into an array of pointers
char *dest[1];declares one pointer tochar, not a byte buffer that can
store the destination string.
Every subsequentmemcpy()treatsdestas a contiguous byte array, thereby
over-writing the pointer itself and whatever follows it in memory ‑ classic
heap corruption territory.- char *dest[1]; /* destination of bundle, allocated to keep dest_len bytes */ + char dest[]; /* flexible array member holding <dest_len> bytes incl. NUL */The rest of the file already allocates the extra storage, so replacing the
pointer with a flexible array is a drop-in fix that eliminates the UB.
🧹 Nitpick comments (65)
deps/curl/lib/sendf.h (1)
185-185: New function declaration added without documentationThe new function
Curl_cwriter_is_content_decodinghas been added without documentation, unlike many other functions in this file that have clear documentation blocks. Consider adding a comment to explain its purpose and usage.+/** + * Return TRUE iff any content decoding writer is active in the client writer stack. + */ bool Curl_cwriter_is_content_decoding(struct Curl_easy *data);deps/curl/include/curl/curlver.h (1)
35-42: Prefer derivingLIBCURL_VERSION_NUMfrom the _MAJOR/MINOR/PATCH macrosHard-coding the hex literal is error-prone when someone bumps only one of the three numeric macros. Consider:
-#define LIBCURL_VERSION_NUM 0x080d00 +#define LIBCURL_VERSION_NUM CURL_VERSION_BITS( \ + LIBCURL_VERSION_MAJOR, \ + LIBCURL_VERSION_MINOR, \ + LIBCURL_VERSION_PATCH)Using the already-defined helper keeps the four values in sync and prevents future mismatches.
Also applies to: 62-62
deps/curl/lib/content_encoding.c (1)
753-754: ASCII–only space check may mis-treat signed chars
if(*enclist > ' ')assumescharis unsigned or the input is strictly ASCII. A defensive alternative that avoids UB with negativecharvalues:- if(*enclist > ' ') + if((unsigned char)*enclist > ' ')Not critical (HTTP tokens are ASCII), but it narrows the risk surface.
deps/curl/lib/krb5.c (2)
218-221:remote_addrcast is still IPv4-only
(struct sockaddr_in *)CURL_UNCONST(&conn->remote_addr->curl_sa_addr)will mis-cast IPv6 addresses coming fromsockaddr_in6. Consider switching tosockaddr_storageor branching onconn->remote_addr->family.
925-936:Curl_sec_conn_destroy()leavesconn->buffer_sizeuntouchedAfter tearing down the security layer the negotiated
buffer_sizeis still cached. If the connection object is re-used, later logic may rely on an out-of-sync size. Consider resetting it alongsidedata_prot:conn->data_prot = PROT_CLEAR; + conn->buffer_size = 0;deps/curl/lib/ftplistparser.c (1)
629-641: PropagateCurl_str_numblanksparse failures
Curl_str_numblanks()already returns non-zero on failure.
The current code only checks that the remainder of the string is empty and that
fsize != CURL_OFF_T_MAX, but silently accepts the value if
Curl_str_numblanks()itself fails with another error condition.- if(!Curl_str_numblanks(&p, &fsize)) { - if(p[0] == '\0' && fsize != CURL_OFF_T_MAX) { + if(Curl_str_numblanks(&p, &fsize) == 0 && + p[0] == '\0' && + fsize != CURL_OFF_T_MAX) { …This makes the intention explicit and prevents partially-parsed sizes from
slipping through.deps/curl/lib/httpsrr.h (1)
59-67: Consider decorating new API symbols for visibility / misuseThe three new helpers are library-internal but are declared in a public header.
On Windows builds withCURL_STATICLIBundefined they will be exported unless
explicitly marked. Additionally, callers could ignore the returnedCURLcode
fromCurl_httpsrr_set, losing error information.-CURLcode Curl_httpsrr_set(struct Curl_easy *data, - struct Curl_https_rrinfo *hi, - uint16_t rrkey, const uint8_t *val, size_t vlen); +/* mark as internal and warn if the return value is dropped */ +CURL_EXTERN CURLcode Curl_httpsrr_set(struct Curl_easy *data, + struct Curl_https_rrinfo *hi, + uint16_t rrkey, const uint8_t *val, + size_t vlen) + CURLX_WARN_UNUSED_RESULT;(Uses
CURL_EXTERNandCURLX_WARN_UNUSED_RESULTconventions already applied
in other libcurl headers.)deps/curl/lib/multihandle.h (1)
136-138:curl_multi_ev evmember increases struct size – watch for stack copies
curl_multi_evlooks non-trivial. Becausestruct Curl_multican still be
allocated on the stack by a few unit tests and internal helpers, the growth
(~hundreds of bytes?) might blow past small-stack assumptions on embedded
platforms. Consider heap-allocating the event sub-object (pointer in the
struct) if the size is significant or provide a compile-time option (e.g.
CURL_DISABLE_MULTI_EV).deps/curl/lib/asyn.h (2)
44-45: Spelling / naming consistency
curl_mutex_t mutx;– every other field in the code base usesmutexor
mtx. The abbreviationmutxis unique and easy to miss during a grep
for “mutex”. Renaming it tomtxkeeps consistency with the threaded
resolver backend (Curl_thread_create()etc.).
61-64:struct curltime start&CURLcode resultdefault initialisation
thread_datais typicallycallocd, but on some code paths (e.g. stack
allocation in unit tests) it may be uninitialised.
Explicitly set:td->start = Curl_now(); /* or zero */ td->result = CURLE_OK; td->init = FALSE;in the thread-initialisation routine to avoid time/math underflows and
spurious error propagation.deps/curl/lib/cf-h2-proxy.c (1)
1238-1260: Pollset calculation: guard against negative window sizes
nghttp2_session_get_stream_remote_window_size()returnsint32_t. If the
peer sends an invalid negative window update, the helper may yield a
negative value that is later compared to0in boolean context.
Consider clamping to>=0before using it to decidewant_send/
want_recv, otherwise large negative values are treated as truthy.deps/curl/lib/multiif.h (1)
88-90: Missing documentation block forCurl_multi_getsockThe new helper has no doxygen comment, unlike most of the multi-if
surface. A terse description of the contract (returned bitmap semantics,
thread safety, etc.) will help downstream maintainers.deps/curl/lib/mqtt.c (1)
746-749: I/O loop starvation risk when leftovers never drain
mqtt_doing()resends the dynbuf once then immediately proceeds to read
from the socket.
If the socket is still not writable, you will spin on EAGAIN inside
Curl_xfer_recvinstead of re-arming POLLOUT/POLLOIN properly.Consider:
- Attempt to flush
sendbuf.- If
CURLE_AGAIN, return early so the multi-state machine can poll for
write readiness instead of dropping into read-phase.result = mqtt_send(...); if(result == CURLE_AGAIN || result) /* bail out early */ return (result == CURLE_AGAIN) ? CURLE_OK : result;deps/curl/lib/hsts.c (2)
61-67: Return-code handling ofCurl_str_numberlooks inverted
Curl_str_number()returnsSTRE_OK(==0) on success and an error code
otherwise.
The current check addsdeltatimeonly when the call succeeds and
returns non-zero:if(!Curl_str_number(×tr, &val, TIME_T_MAX)) val += deltatime;If a parsing error occurs
valremains uninitialised. Consider
initialisingvalto0beforehand or flipping the condition to guard
against bad inputs.
420-424: Error path silently ignored inhsts_addpre-parsingThe
if(...) ; else { ... }construct discards any parsing error without
reporting it to the caller, making debugging of malformed HSTS files
difficult. PropagateCURLE_BAD_FUNCTION_ARGUMENT(or a new code) when
Curl_str_*helpers fail so the loader can warn the user.deps/curl/lib/rtsp.c (1)
870-877: Guard against 0-length writes withCLIENTWRITE_EOSWhen
blen == 0butis_eosisTRUE, the current call:Curl_client_write(data, CLIENTWRITE_BODY | (is_eos ? CLIENTWRITE_EOS : 0), buf, blen);invokes
Curl_client_write()with a length of 0.
Most writers silently ignore this, but some (or future) stages might assert on a zero-length body write.- if(!result && (is_eos || blen)) { + if(!result && (blen || is_eos)) { result = Curl_client_write(data, CLIENTWRITE_BODY | (is_eos ? CLIENTWRITE_EOS : 0), buf, blen); }This preserves current behaviour for normal data, yet skips the call entirely when both
blen == 0andis_eos == FALSE.deps/curl/lib/mprintf.c (2)
183-185: Macro parameters should be fully parenthesised to prevent subtle bugs
is_arg_usedandmark_arg_useddo not parenthesise their parameters, so an expression such as
mark_arg_used(used, idx + 1)would expand toused[idx + 1/8], which is not what is intended.-#define is_arg_used(x,y) ((x)[(y)/8] & (1 << ((y)&7))) -#define mark_arg_used(x,y) ((x)[y/8] |= (unsigned char)(1 << ((y)&7))) +#define is_arg_used(x, y) ((x)[(y) / 8] & (1U << ((y) & 7))) +#define mark_arg_used(x, y) ((x)[(y) / 8] |= (unsigned char)(1U << ((y) & 7)))Adding the missing parentheses makes the macros safe for all argument expressions.
173-181:dollarstring()fails to update*endon parse-errorWhen either
Curl_str_number()orCurl_str_single()fails,*endremains uninitialised, which the caller does not expect.
Consider setting*end = p;before returning-1so the caller always receives a valid pointer.deps/curl/lib/openldap.c (1)
560-563: Possible truncation when parsingCURL_OPENLDAP_TRACEenv-var
Curl_str_number(&env, &e, INT_MAX)limits the parsed value toINT_MAX, yet you store the result in acurl_off_t(e).
If a user supplies a value larger thanINT_MAX, the function silently caps it, which might surprise them and/or differ from previous behaviour (strtolwould have returnedLONG_MAXandERANGE).If you want to preserve the full range allowed by OpenLDAP (an
int), castetointdirectly and drop theINT_MAXlimiter, or document the cap explicitly.deps/curl/lib/cw-pause.c (2)
140-165:cw_pause_flushbecomes O(n²) due to repeated tail-scansInside the flush loop you traverse the singly-linked list to its tail (
while((*plast)->next)) every iteration, making the whole flush quadratic in the number of buffered chunks.
For heavy pause / resume workloads this can degrade performance noticeably.A minimal change is to maintain a
struct cw_pause_buf **tailpointer in the context, updated on push/pop, and use it directly.
219-222: Trace message uses stale variable – misleading diagnostics
ctx->buf_total + wlenis logged, but at this pointwlenstill contains the length that was direct-written earlier, not thenwrittenthat has just been buffered.
This produces wrong byte counts in debug logs.- "total=%zu -> %d", nwritten, type, ctx->buf_total + wlen, + "total=%zu -> %d", nwritten, type, ctx->buf_total + nwritten,deps/curl/lib/curl_trc.h (1)
120-135: Minor macro readability nitThe new
CURL_TRC_M/CURL_TRC_DNShelpers are very welcome, but because they silently wrap the underlying functions, newcomers may overlook that these macros evaluate their arguments more than once. It might help to add a short comment above each macro reminding contributors to avoid function calls with side-effects in the var-arg list (e.g.CURL_TRC_M(data, "msg=%d", expensive_fn())).deps/curl/lib/cf-https-connect.c (1)
688-690: Avoid redundant capability probes
Curl_conn_may_http3()is called here only to discard its return value when it signals “not possible”. The same probe is executed for real a few lines later. Removing this first call spares an unnecessary DNS / settings lookup.- if(Curl_conn_may_http3(data, conn)) - break; /* not possible */ + /* defer capability check until we actually intend to add h3 */deps/curl/lib/pingpong.c (1)
102-103: Typo in trace messageThe debug line says
"pp_statematch"but the function isCurl_pp_statemach(). While harmless, consistent naming eases grep-based debugging.- DEBUGF(infof(data, "pp_statematch, timeout=%" FMT_TIMEDIFF_T, timeout_ms)); + DEBUGF(infof(data, "pp_statemach, timeout=%" FMT_TIMEDIFF_T, timeout_ms));deps/curl/lib/cfilters.h (1)
402-404:Curl_conn_http_versionnow takesconn– update header docThe function comment (lines 398-401) still states “…used on the FIRSTSOCKET connection filters” but does not mention the new parameter. A one-liner keeps doxygen/IDE hints accurate.
deps/curl/lib/ldap.c (2)
886-891:Curl_str_until()fixed buffer size may truncate long attributesThe call caps attribute length at 1024 bytes – silently truncating longer
names. RFC 4516 does not set an upper limit; although unlikely, this can
produce hard-to-diagnose search failures.Consider allocating dynamically based on
strlen(atp)when it exceeds the
static buffer size, or at minimum returnLDAP_INVALID_SYNTAXon overflow so
the caller sees a clear error.
731-734: Environment parsing: handle malformedCURL_TRACEgracefully
Curl_str_number()leavesenvadvanced even on failure, sodo_trace
remains ‑1 ifCURL_TRACE=abc. You then dereferenceewhich is uninitialised.
Recommend initialisinge = 0and settingdo_trace = 0when the conversion
fails to avoid UB in strict builds.deps/curl/lib/hostip.c (1)
586-589: Spurious cast hides type mismatch
curlx_inet_ptonexpects avoid *destination whose real type must match the requested family.
Passing(char *)&ipv4(whereipv4is anunsigned int) works but loses type safety and
triggers −Wstrict-aliasing on some compilers. Use the correct pointer type instead:- if(curlx_inet_pton(AF_INET, "127.0.0.1", (char *)&ipv4) < 1) + if(curlx_inet_pton(AF_INET, "127.0.0.1", &ipv4) < 1)deps/curl/lib/memdebug.h (1)
69-76: Minor: broken line continuation hampers grep-abilitySplitting the return-type macro and the function name on two lines makes simple
signature greps harder. Keeping them on a single logical line is usually more
readable:-CURL_EXTERN ALLOC_FUNC ALLOC_SIZE(1) - void *curl_dbg_malloc(size_t size, int line, const char *source); +CURL_EXTERN ALLOC_FUNC ALLOC_SIZE(1) void * +curl_dbg_malloc(size_t size, int line, const char *source);deps/curl/lib/asyn-thread.c (2)
467-468: Lost error fromresolve_httpsrr()
resolve_httpsrr()can return a hard error but the caller merely logs it and still returnsTRUE, letting the main resolver continue as if nothing happened.
If HTTPS RR data is a hard requirement this silently degrades behaviour.Consider bubbling the error up:
- if(resolve_httpsrr(data, async)) - infof(data, "Failed HTTPS RR operation"); + result = resolve_httpsrr(data, async); + if(result) { + err = CURLE_FAILED_INIT; /* or result */ + goto err_exit; + }
647-666: RedundantCurl_expire()loop – opportunity for simplificationThe back-off calculation duplicates logic present earlier (lines 601-621).
Extracting it into a helper (e.g.calc_poll_backoff()) would remove duplication and ensure the two callers cannot diverge over time.
This also improves unit-testability of the timing algorithm.deps/curl/lib/httpsrr.c (1)
152-156: Shallow copy risks withCurl_httpsrr_dup_move()
Curl_memdup()performs a shallow copy, so the returned structure shares all
pointed-to buffers with the original.
Because the original is zeroed, ownership is effectively transferred, but if callers ever keep both pointers alive (e.g. on error unwind paths) double-free will occur.Document the transfer semantics clearly in the header and consider using a dedicated “move” helper that takes a pointer to pointer and sets the source to NULL to make the ownership transfer explicit.
deps/curl/lib/http.h (2)
56-61: Macro type-safety & namespaceThe bitmask constants expand to plain
int, yet they are stored in an
unsigned char. Using wider macros risks accidental truncation when OR’ing
several values.-#define CURL_HTTP_V1x (1 << 0) -#define CURL_HTTP_V2x (1 << 1) -#define CURL_HTTP_V3x (1 << 2) +#define CURL_HTTP_V1x ((http_majors)1 << 0) +#define CURL_HTTP_V2x ((http_majors)1 << 1) +#define CURL_HTTP_V3x ((http_majors)1 << 2)Also consider the
CURL_HTTP_prefix may collide with future public API
macros; using an internal prefix such asCHTTP_V1xavoids that.
220-227: Non-standard flexible array – preferchar method[]Declaring
char method[1]signals a flexible array but is not C99-conformant
(the standard form ischar method[]). Some static analysers flag this as a
“one-element array” antipattern.- char method[1]; + char method[];This change is layout-compatible (size remains
sizeof(struct httpreq)),
clarifies intent and avoids compiler warnings.deps/curl/lib/hash_offt.h (1)
29-33: Remove superfluous header dependency to cut compile time and suppress ODR risk
llist.his included but no symbol from that header is referenced in this file. Keeping unnecessary includes expands the compilation-unit graph and may accidentally expose unrelated macros/symbols to the rest of the TU.-#include "llist.h"Unless
hash_offt.ctruly needs the declaration transitively, please drop it (or move the include to the.cfile that actually needs it).deps/curl/lib/memdebug.c (1)
439-452: Guard against vsnprintf-truncation & negative return values
mvsnprintf()returns the number of characters that would have been written or a negative value on error.
Current code:nchars = mvsnprintf(buf, sizeof(buf), format, ap); … if(nchars > (int)sizeof(buf) - 1) nchars = (int)sizeof(buf) - 1; if(nchars > 0) fwrite(buf, 1, (size_t)nchars, curl_dbg_logfile);Issues
- When
nchars >= sizeof(buf)the terminating NUL is outsidebuf–‐ we subsequently writencharsbytes that were never produced.- A negative
ncharsquietly disables logging but still leaves the buffer uninitialised.Suggested defensive tweak:
nchar = mvsnprintf(buf, sizeof(buf), format, ap); - if(nchars > (int)sizeof(buf) - 1) - nchars = (int)sizeof(buf) - 1; - if(nchars > 0) - fwrite(buf, 1, (size_t)nchars, curl_dbg_logfile); + if(nchars < 0) + return; /* encoding error – bail out */ + + if((size_t)nchars >= sizeof(buf)) + /* string truncated, make sure it is NUL-terminated */ + nchars = (int)(sizeof(buf) - 1); + + fwrite(buf, 1, (size_t)nchars, curl_dbg_logfile);This guarantees we never read beyond what was actually written and keeps the log consistent.
deps/curl/lib/Makefile.in (2)
5775-5780:sed -Eportability – avoid breaking thetidytarget on non-GNU systemsThe updated
tidyrecipe relies onsed -E, which is BSD-sed specific.
On many Linux distributions the extended-regex flag is-r; on macOS both-Eand-rwork, but BusyBox and Alpine only ship-r.To make the rule truly portable, prefer POSIX-compliant parameter expansion or keep the previous approach.
A minimal, shell-only alternative:- _csources=`echo ' $(CSOURCES)' | sed -E -e 's/ +$//' -e 's/ +/ /g' -e 's| | $(srcdir)/|g'`; \ + _csources="$(printf '%s\n' $(CSOURCES) | sed 's|^|$(srcdir)/|')"; \This avoids dependence on any
sedflavour.
Not critical for CI (which uses GNU tools) but could bite downstream packagers.
2434-2440: Redundant explicit pattern rules may be removedAutomake already generates an implicit compilation rule for each
*.cinCSOURCES.
The hand-written rule introduced forcshutdn.c(and likewise forcw-pause.c,multi_ev.c, etc.) is therefore redundant unless you need per-file CFLAGS or dependencies.Trimming the Makefile helps maintenance ‑ upstream’s recent versions rely solely on the autogenerated patterns.
Safe to remove unless a special flag is planned.
deps/curl/lib/doh.c (2)
304-306: Double-check the handle used for the verbose flag
Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns)inspects the parent easy handle (data), yet the resultingCURLOPT_VERBOSE
is set on the DoH handle (doh).
That is probably intentional (inherit log level), but if the purpose was to honour the probe’sstate.featflag
(which you just set ondohon line 280), then the call should usedohinstead ofdata.No change requested if inheritance is intentional, otherwise consider:
- if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns)) + if(Curl_trc_ft_is_verbose(doh, &Curl_trc_feat_dns)) ERROR_CHECK_SETOPT(CURLOPT_VERBOSE, 1L);
188-191: DEBUG-only macro could live inside the function to limit scope
LOCAL_PB_HEXMAXis only consumed bydoh_print_buf().
Placing the#defineinside the function keeps the identifier out of the
translation unit’s global namespace and avoids accidental reuse.Purely stylistic – feel free to ignore.
deps/curl/lib/hash.h (2)
50-56: Use a genuine C99 flexible array memberThe new bucket element stores the key with
char key[1];.
On modern compilers the canonical idiom ischar key[];which:
- avoids undefined behaviour around
sizeof(element)calculations,- suppresses -Wpedantic / -Wgnu-zero-variadic-size-type warnings, and
- conveys intent more clearly.
- char key[1]; /* allocated memory following the struct */ + char key[]; /* flexible array – memory follows the struct */No functional change, but improves portability and compiler happiness.
31-32:llist.hinclude looks staleThe hash buckets are no longer
Curl_llists; keeping#include "llist.h"therefore adds an unnecessary dependency and slows
down compilation.Safe to delete unless some inline function below still relies on it.
deps/curl/lib/cookie.c (1)
162-178: Micro-optimisation: avoid repeatedstrlen
strlen(uri_path)is computed twice; storing it once makes the branch
cheaper and keeps the code easier to read:- if(0 == strlen(uri_path) || uri_path[0] != '/') + if(!*uri_path || uri_path[0] != '/') uri_path = "/"; - uri_path_len = strlen(uri_path); + uri_path_len = strlen(uri_path);Very minor, feel free to skip.
deps/curl/lib/connect.c (1)
611-613:SOCKETIMEDOUTmay be undefined on several platformsThe new timeout handling relies on the macro being defined, but it is only wrapped in
#ifdef SOCKETIMEDOUT.
On platforms where the macro is missing you will silently skip settingballer->error, which later leads todata->state.os_errnobeing 0 and an incorrect error path.Consider providing a portable fallback, e.g.
#ifndef SOCKETIMEDOUT #define SOCKETIMEDOUT ETIMEDOUT /* or WSAETIMEDOUT on Win32 */ #endifOr map the platform specific error inside
curl_setup_once.hwhere the otherSOCKE*aliases live.deps/curl/lib/hash_offt.c (1)
109-116: Typo in helper name (hash_offtr_elem_link)The helper is spelled
hash_offtr_elem_link(missing the second “t”), which is easy to miss during future maintenance and breaks the otherwise consistenthash_offt_*prefix scheme.-static void hash_offtr_elem_link( +static void hash_offt_elem_link(Update the single call site at line 148 accordingly.
deps/curl/lib/cshutdn.h (1)
54-61: Inconsistent terminology & unclear dependency onBITmacro
- The comment block refers to “a
cshutdown” while the struct is namedcshutdn.
Readers may look for a non-existent type.- The struct relies on the
BIT()macro/typedef, yet this header does not include the file that defines it (usuallycurl_setup.horcurl_def.h). Any TU that includescshutdn.hbefore those headers will fail to compile.-/* A `cshutdown` is always owned by a multi handle … +/* A `cshutdn` is always owned by a multi handle …and consider adding the proper include (or an explicit forward declaration) for
BIT.Please build a TU that includes only this header to confirm the missing dependency.
deps/curl/lib/http_aws_sigv4.c (1)
49-58: Macro silently hijacksresultsymbol
HMAC_SHA256expands to code that writes to a variable namedresultthat must be
visible in the caller’s scope. This implicit coupling:
- Obscures control flow (unexpected
goto fail).- Makes the macro unusable from scopes where
resultis not the expected
CURLcode.Consider turning it into an inline helper instead:
static CURLcode hmac_sha256_wrap(const void *k, size_t kl, const void *d, size_t dl, unsigned char *o) { return Curl_hmacit(&Curl_HMAC_SHA256, (const unsigned char *)k, kl, (const unsigned char *)d, dl, o); }and handle the
failpath explicitly where it’s invoked.deps/curl/lib/cf-socket.c (1)
725-737: Possible truncation of IPv6 scope-id
scope_idis parsed into acurl_off_t, then cast tounsigned int:curl_off_t scope_id; if(Curl_str_number(..., &scope_id, UINT_MAX)) return CURLE_UNSUPPORTED_PROTOCOL; si6->sin6_scope_id = (unsigned int)scope_id;While the upper bound prevents overflow, any value >
UINT_MAXcauses an early
CURLE_UNSUPPORTED_PROTOCOL, even though the kernel supports a 32-bit field.
If that is intentional, please document the limitation; otherwise pass
UINT32_MAXas upper limit (or validate againstUINT_MAXonly on 16-bit
platforms).deps/curl/lib/hash.c (1)
185-201: Potential unbounded slots array growth check
Curl_hash_add2lazily allocatesh->table = calloc(h->slots …)once, but when
callers manually changeh->slotslater (API allows it), the table is not
re-allocated. Subsequent look-ups then index past the originally-allocated
array.If the API guarantees
h->slotsis immutable, add a comment +DEBUGASSERT.
Otherwise re-allocate whenh->slotsgrows.deps/curl/lib/cfilters.c (2)
408-487: Hard-coded small on-stackpollfdarray risks overflow
CF_CONN_NUM_POLLS_ON_STACKis fixed to 5, but the aggregate poll set can
grow when multiple filters contribute sockets (e.g. QUIC + Happy Eyeballs).
Curl_pollfds_add_psdoes not currently check against this bound.Either:
- Grow the stack array to the documented
MAX_SOCKSPEREASYHANDLE
(currently 6), or- Detect overflow in
Curl_pollfds_add_psand fall back to heap allocation.
938-947:CURL_SMALLSENDSparsing silently ignores overflow
Curl_str_number(&p, &altsize, write_len)limits the parsed value to
write_len. When an environment variable bigger thanSIZE_T_MAXis supplied
it wraps silently to a small positive number, which is surprising.Consider rejecting out-of-range values and emitting a debug warning instead.
deps/curl/lib/altsvc.c (1)
196-206:priofield is parsed but never stored
altsvc_add()parses thepriovalue into the local variableprio, butas->priois unconditionally set to zero:as->prio = 0; /* not supported to just set zero */Either honour the parsed value or remove the parsing to avoid confusion and dead code paths.
- as->prio = 0; /* not supported to just set zero */ + as->prio = (unsigned short)prio;If priority handling is not yet implemented, consider deleting the parsing logic altogether and documenting that the field is ignored.
deps/curl/lib/multi_ev.c (1)
246-264:entry->readers + entry->writersassertion may fire when last user is removedThe DEBUGASSERT on line 250 requires at least one reader or writer:
DEBUGASSERT(entry->writers + entry->readers);Immediately after a transfer removes its last interest, both counters can legitimately be zero before
mev_forget_socket()is called.
This will trigger the assertion in debug builds although the state is valid.Proposal: drop the assertion or gate it behind
comboactioncomputation where at least one bit is guaranteed to be set.deps/curl/include/curl/curl.h (2)
930-937: Enumcurl_usesslreduced to a single member – consider deprecating or markingenumas anonymousThe option values are now supplied exclusively via the
CURLUSESSL_*macros and the enum holds only the sentinelCURLUSESSL_LAST. While this preserves ABI, it also:
- defeats type-safety in C++ (
curl_usesslis no longer a meaningful type);- is inconsistent with neighbouring enums that retained all values.
Unless there is a compelling reason, consider keeping the real members inside the enum and aliasing the macros to them (similar to how
CURL_SSLVERSION_*is handled).
2257-2260:CURLOPT_UPLOAD_FLAGSlands without validation helpersThe new option introduces a bit-mask yet no convenience macros/functions to validate or stringify the flag set (compare with
CURL_HTTP_VERSION_*,CURL_RTSPREQ_*, etc.). Providing at least aCURLULFLAG_ALLmask and documenting mutually exclusive combinations would ease adoption and avoid undefined behaviour when unknown bits are supplied.deps/curl/lib/ftp.c (5)
543-553: Consider avoiding side-effects on the input pointer
Curl_str_number(&line, &status, 999)advances the local variablelinebehind
the parsed digits.
Becauselineis a copy of the original parameter this is harmless, but the
double indirection (&line) tends to read as “I intend to modify the caller’s
pointer”. Forwarding a const input as a mutable out-param is a little
counter-intuitive when skimming the code.- const char *p = line; - if((len > 3) && LASTLINE(p) && !Curl_str_number(&p, &status, 999)) { + const char *p = line; + if((len > 3) && LASTLINE(p) && !Curl_str_number(&p, &status, 999)) { *code = (int)status; return TRUE; }Keeping the original parameter unchanged improves readability and removes the
need for the/* (void)line */feeling.
1066-1084: Error-code portability: good move, but wrap legacy macrosReplacing raw
EADDRINUSE/EACCES/...with libcurl’sSOCKE*abstraction is
excellent for cross-platform builds.
However, several private helpers (e.g.Curl_strerror) may still emit the
legacy constants when compiled on POSIX-only build slaves. Consider leaving a
compat alias for out-of-tree consumers:#ifndef SOCKEADDRINUSE #define SOCKEADDRINUSE EADDRINUSE #endifThat prevents surprises for packagers that include only
ftp.c& friends in
embedded builds.
1770-1777: Bounds checking for octets in PASV reply
Curl_str_number()correctly enforces the0xffupper bound, yet the helper
still happily accepts octets such as000256.
Many servers never misbehave, but adding a second guard keeps us safe:if(Curl_str_number(&p, &num, 0xff) || num > 255) return FALSE;This matches the strictness of the original
strtoul-based check.
1808-1818: Avoid allocatingnewhosttwiceIn the EPSV branch we now
strdup(control_address(conn))into
ftpc->newhost.
If the earlier call toCurl_safefree(ftpc->newhost)happened to set the
pointer toNULL(it does), everything is fine.
In the PASV branch, though,ftpc->newhostis created after the free, and
below (l. 1955) we immediatelyfree(conn->secondaryhostname)and then
strdup(ftpc->newhost)– doubling the work.Not a correctness issue, but you can elide the second allocation by moving the
free(conn->secondaryhostname)above the protocol-specific block and simply
Curl_assign_str(&conn->secondaryhostname, ftpc->newhost);afterwards.
3141-3143: Log-spam risk inside tight loop
ftp_block_statemach()now printsif(ftpc->shutdown) CURL_TRC_FTP(data, "in shutdown, waiting for server response");on every iteration while waiting for the final 221.
In verbose or trace builds this can flood logs and slow the loop.Recommend gating by a once-flag or elevating to
Curl_debug_once().deps/curl/lib/curl_trc.c (1)
83-90:TRC_LINE_MAXis respected, butbufis still written past bounds when the line is exactly full
trc_end_buf()forceslen = maxlen - 5and then writes four extra bytes
(“...\n”) plus the final NUL → totalmaxlenbytes OK.
However, the caller often passesmaxlen == sizeof(buf)wherebufis
char buf[TRC_LINE_MAX]; if the formatted string already consumed
TRC_LINE_MAXbytestrc_print_ids()wrote the trailing NUL, solen == TRC_LINE_MAX-1, andlen >= maxlen - 2is true.
The function then rewrites inside the buffer but leaves the previous NUL in
place, resulting in two NULs mid-string.Harmless but confusing when debugging; consider simplifying:
if(len >= maxlen - (addnl ? 2 : 1)) { - len = maxlen - 5; + len = maxlen - (addnl ? 5 : 4);and always rewrite the final NUL.
deps/curl/lib/multi.c (1)
977-1005: Guard expensive TRACE in non-debug builds
TheseCURL_TRC_Mcalls inCurl_multi_getsock()are inside the hot path executed for every socket assessment. In release builds they compile to nothing, but in DEBUG or user builds that defineCURL_TRC_Mto real logging they can flood logs and slow things down.
Wrap the wholeswitch(ps->num){…}block inside#ifdef DEBUGBUILDor add an early‐outif(!CURLTRACE_ENABLED) return;to avoid impacting production users who build with custom trace macros.deps/curl/lib/conncache.h (1)
38-50: Document thread-safety & lifecycle ofCurl_conn_terminate
The new helper shifts responsibility from conn-cache to the shutdown subsystem. Clarify (in the doxy-comment and header) that:
• The caller must own either the cpool lock or be inside the multi’scshutdnthread-context.
•connmemory is freed asynchronously – dereferencing it afterwards is UB.
Without this, external contributors may call the function from signal handlers or other threads assuming it is synchronous.deps/curl/lib/CMakeLists.txt (2)
26-27: Adopt modern CMake compile definition handling
Switching fromadd_definitionstoset_property(... COMPILE_DEFINITIONS)is an improvement for CMake best practices, as it scopes definitions at directory-level and avoids global side-effects.Consider further scoping these definitions to specific targets using
target_compile_definitions(<target> PUBLIC|PRIVATE ...), which can reduce unintended propagation to subdirectories or external dependencies.
107-109: Introduce object library for shared/static reuse
The optionallibcurl_objectOBJECTlibrary target promotes build efficiency by compiling sources once. Guarding it on CMake ≥3.12 is correct.Ensure your project’s
cmake_minimum_required(VERSION ...)is bumped to ≥3.12, or explicitly document that older CMake versions will fall back to separate compilations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (144)
deps/curl/curl.gyp(8 hunks)deps/curl/include/curl/curl.h(12 hunks)deps/curl/include/curl/curlver.h(3 hunks)deps/curl/include/curl/system.h(3 hunks)deps/curl/lib/.checksrc(1 hunks)deps/curl/lib/CMakeLists.txt(4 hunks)deps/curl/lib/Makefile.am(2 hunks)deps/curl/lib/Makefile.in(74 hunks)deps/curl/lib/Makefile.inc(9 hunks)deps/curl/lib/altsvc.c(8 hunks)deps/curl/lib/amigaos.c(1 hunks)deps/curl/lib/asyn-ares.c(11 hunks)deps/curl/lib/asyn-thread.c(22 hunks)deps/curl/lib/asyn.h(4 hunks)deps/curl/lib/base64.c(2 hunks)deps/curl/lib/bufref.c(2 hunks)deps/curl/lib/cf-h1-proxy.c(5 hunks)deps/curl/lib/cf-h2-proxy.c(3 hunks)deps/curl/lib/cf-haproxy.c(2 hunks)deps/curl/lib/cf-https-connect.c(8 hunks)deps/curl/lib/cf-socket.c(29 hunks)deps/curl/lib/cfilters.c(9 hunks)deps/curl/lib/cfilters.h(10 hunks)deps/curl/lib/config-os400.h(0 hunks)deps/curl/lib/config-plan9.h(0 hunks)deps/curl/lib/config-riscos.h(0 hunks)deps/curl/lib/config-win32.h(7 hunks)deps/curl/lib/conncache.c(28 hunks)deps/curl/lib/conncache.h(4 hunks)deps/curl/lib/connect.c(13 hunks)deps/curl/lib/connect.h(2 hunks)deps/curl/lib/content_encoding.c(6 hunks)deps/curl/lib/cookie.c(22 hunks)deps/curl/lib/cshutdn.c(1 hunks)deps/curl/lib/cshutdn.h(1 hunks)deps/curl/lib/curl_addrinfo.c(5 hunks)deps/curl/lib/curl_config.h.cmake(1 hunks)deps/curl/lib/curl_config.h.in(0 hunks)deps/curl/lib/curl_ctype.h(1 hunks)deps/curl/lib/curl_fnmatch.c(7 hunks)deps/curl/lib/curl_gssapi.c(1 hunks)deps/curl/lib/curl_krb5.h(1 hunks)deps/curl/lib/curl_memory.h(0 hunks)deps/curl/lib/curl_memrchr.c(1 hunks)deps/curl/lib/curl_memrchr.h(0 hunks)deps/curl/lib/curl_multibyte.c(5 hunks)deps/curl/lib/curl_multibyte.h(2 hunks)deps/curl/lib/curl_ntlm_core.c(5 hunks)deps/curl/lib/curl_range.c(2 hunks)deps/curl/lib/curl_rtmp.c(1 hunks)deps/curl/lib/curl_sasl.c(0 hunks)deps/curl/lib/curl_setup.h(14 hunks)deps/curl/lib/curl_setup_once.h(9 hunks)deps/curl/lib/curl_sha512_256.c(0 hunks)deps/curl/lib/curl_sspi.c(3 hunks)deps/curl/lib/curl_sspi.h(2 hunks)deps/curl/lib/curl_threads.c(1 hunks)deps/curl/lib/curl_threads.h(1 hunks)deps/curl/lib/curl_trc.c(16 hunks)deps/curl/lib/curl_trc.h(7 hunks)deps/curl/lib/curlx.h(1 hunks)deps/curl/lib/cw-out.c(8 hunks)deps/curl/lib/cw-pause.c(1 hunks)deps/curl/lib/cw-pause.h(2 hunks)deps/curl/lib/dict.c(4 hunks)deps/curl/lib/doh.c(9 hunks)deps/curl/lib/doh.h(0 hunks)deps/curl/lib/dynbuf.c(5 hunks)deps/curl/lib/dynbuf.h(2 hunks)deps/curl/lib/easy.c(7 hunks)deps/curl/lib/easyoptions.c(2 hunks)deps/curl/lib/escape.c(0 hunks)deps/curl/lib/file.c(2 hunks)deps/curl/lib/fopen.c(1 hunks)deps/curl/lib/formdata.c(1 hunks)deps/curl/lib/ftp.c(25 hunks)deps/curl/lib/ftp.h(3 hunks)deps/curl/lib/ftplistparser.c(5 hunks)deps/curl/lib/getenv.c(1 hunks)deps/curl/lib/getinfo.c(6 hunks)deps/curl/lib/gopher.c(2 hunks)deps/curl/lib/hash.c(8 hunks)deps/curl/lib/hash.h(2 hunks)deps/curl/lib/hash_offt.c(1 hunks)deps/curl/lib/hash_offt.h(1 hunks)deps/curl/lib/headers.c(3 hunks)deps/curl/lib/hostasyn.c(1 hunks)deps/curl/lib/hostip.c(20 hunks)deps/curl/lib/hostip.h(0 hunks)deps/curl/lib/hostip4.c(1 hunks)deps/curl/lib/hostip6.c(1 hunks)deps/curl/lib/hsts.c(8 hunks)deps/curl/lib/http.c(44 hunks)deps/curl/lib/http.h(4 hunks)deps/curl/lib/http2.c(40 hunks)deps/curl/lib/http_aws_sigv4.c(20 hunks)deps/curl/lib/http_aws_sigv4.h(1 hunks)deps/curl/lib/http_chunks.c(6 hunks)deps/curl/lib/http_digest.c(3 hunks)deps/curl/lib/http_negotiate.c(4 hunks)deps/curl/lib/http_ntlm.c(2 hunks)deps/curl/lib/http_proxy.c(6 hunks)deps/curl/lib/httpsrr.c(2 hunks)deps/curl/lib/httpsrr.h(3 hunks)deps/curl/lib/idn.c(2 hunks)deps/curl/lib/if2ip.c(3 hunks)deps/curl/lib/imap.c(11 hunks)deps/curl/lib/imap.h(1 hunks)deps/curl/lib/inet_ntop.c(4 hunks)deps/curl/lib/inet_ntop.h(2 hunks)deps/curl/lib/inet_pton.c(1 hunks)deps/curl/lib/inet_pton.h(2 hunks)deps/curl/lib/krb5.c(3 hunks)deps/curl/lib/ldap.c(13 hunks)deps/curl/lib/llist.c(2 hunks)deps/curl/lib/md4.c(2 hunks)deps/curl/lib/md5.c(2 hunks)deps/curl/lib/memdebug.c(9 hunks)deps/curl/lib/memdebug.h(3 hunks)deps/curl/lib/mime.c(1 hunks)deps/curl/lib/mprintf.c(18 hunks)deps/curl/lib/mqtt.c(13 hunks)deps/curl/lib/mqtt.h(1 hunks)deps/curl/lib/multi.c(44 hunks)deps/curl/lib/multi_ev.c(1 hunks)deps/curl/lib/multi_ev.h(1 hunks)deps/curl/lib/multihandle.h(4 hunks)deps/curl/lib/multiif.h(3 hunks)deps/curl/lib/netrc.c(4 hunks)deps/curl/lib/noproxy.c(6 hunks)deps/curl/lib/openldap.c(8 hunks)deps/curl/lib/parsedate.c(7 hunks)deps/curl/lib/pingpong.c(6 hunks)deps/curl/lib/pingpong.h(1 hunks)deps/curl/lib/pop3.c(4 hunks)deps/curl/lib/psl.c(1 hunks)deps/curl/lib/rename.c(1 hunks)deps/curl/lib/request.c(5 hunks)deps/curl/lib/rtsp.c(14 hunks)deps/curl/lib/rtsp.h(1 hunks)deps/curl/lib/select.c(6 hunks)deps/curl/lib/select.h(3 hunks)deps/curl/lib/sendf.c(6 hunks)deps/curl/lib/sendf.h(1 hunks)
💤 Files with no reviewable changes (11)
- deps/curl/lib/hostip.h
- deps/curl/lib/config-riscos.h
- deps/curl/lib/curl_memory.h
- deps/curl/lib/escape.c
- deps/curl/lib/doh.h
- deps/curl/lib/curl_sha512_256.c
- deps/curl/lib/curl_sasl.c
- deps/curl/lib/config-plan9.h
- deps/curl/lib/curl_memrchr.h
- deps/curl/lib/curl_config.h.in
- deps/curl/lib/config-os400.h
🧰 Additional context used
🪛 Cppcheck (2.10-2)
deps/curl/lib/http_chunks.c
[error] 260-260: failed to expand 'Curl_dyn_addn', Wrong number of parameters for macro 'Curl_dyn_addn'.
(preprocessorErrorDirective)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test-tarball-linux
- GitHub Check: test-macOS
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: test-linux (ubuntu-24.04)
🔇 Additional comments (223)
deps/curl/lib/hostasyn.c (1)
78-80: Hostname source changed for cache lookupThe change from using
data->state.async.hostnametodata->conn->host.dispnamealigns with the broader refactoring where references to async hostname are updated to use the connection's display name directly. This ensures consistent hostname usage across resolver and host cache modules.deps/curl/lib/curl_ctype.h (1)
40-40: LGTM: New octal digit check macroThe addition of the
ISODIGIT(x)macro follows the same pattern as existing character classification macros and provides a dedicated function for checking octal digits (0-7), which is useful for parsing contexts where octal values are relevant.deps/curl/lib/rtsp.h (1)
56-56: LGTM: Added initialization tracking flagAdding the
initialisedbit field to track RTSP connection initialization state helps prevent redundant buffer initialization and ensures proper cleanup. This supports the enhanced connection state management introduced in this update.deps/curl/lib/imap.h (1)
88-88: LGTM: Added initialization tracking flagThe new
initialisedbit field provides proper state tracking for IMAP connections, allowing code to guard against repeated initialization or improper cleanup. This change aligns with the broader connection management improvements in the libcurl update.deps/curl/lib/.checksrc (1)
6-8: Good addition of banned functionsAdding
strtoul,strtol, andstrtok_rto the list of banned functions is a positive change. This aligns with libcurl's strategy of avoiding platform-dependent functions that might not be available or safe across all supported systems.deps/curl/lib/bufref.c (2)
33-35: Proper isolation of debug-only codeMoving the
SIGNATUREmacro definition inside theDEBUGBUILDconditional block improves code organization by properly isolating debug-only code.
64-64: Improved const-correctness with CURL_UNCONSTGood use of the
CURL_UNCONSTmacro instead of direct C-style casting. This improves const-correctness and provides a standardized way to remove const qualifiers, which is safer and more maintainable than direct casts.deps/curl/lib/hostip6.c (1)
129-130: Standardized IP address handling with curlx_ prefixGood replacement of
Curl_inet_ptonwithcurlx_inet_pton. This change is part of a broader standardization effort for IP address parsing and conversion functions, which ensures consistent API usage throughout the codebase and likely improves error handling.deps/curl/lib/getenv.c (1)
34-35: Improved Windows CE platform detectionGood update to use
UNDER_CEinstead of_WIN32_WCEfor Windows CE platform detection. This ensures consistency with other platform checks across the codebase, making it more maintainable and reliable.deps/curl/lib/curl_rtmp.c (1)
347-347: Improved const-correctness in RTMP_Write callThe change correctly casts the buffer to
const char *instead ofchar *, properly preserving the const qualifier of the input buffer. This is a good improvement to maintain type safety without changing functionality.deps/curl/lib/psl.c (1)
43-43: Using CURL_UNCONST macro for safer const-castingGood improvement replacing the direct C-style cast with the standardized
CURL_UNCONSTmacro. This change provides a more consistent approach to handling const-casting throughout the codebase and makes it easier to audit places where constness is removed.deps/curl/lib/curl_gssapi.c (1)
49-53: Consistently using CURL_UNCONST macro for string literalsGood improvement replacing direct C-style casts with the standardized
CURL_UNCONSTmacro for the string literals in OID structures. This properly acknowledges the const-ness of string literals and provides a consistent approach throughout the codebase.deps/curl/lib/http_aws_sigv4.h (1)
29-29: Simplified function signature by removing the proxy parameterThe function signature has been simplified by removing the
bool proxyparameter. This is likely part of a broader refactoring effort where the AWS SigV4 signing logic has been updated to derive this information from the data structure directly. This simplification is a good practice when parameters can be derived from existing inputs.deps/curl/lib/curl_multibyte.h (2)
28-28: Simplified conditional syntax.The change from
#if defined(_WIN32)to#ifdef _WIN32improves code readability while maintaining the same functionality.
87-87: Added proper const handling with CURL_UNCONST.The use of
CURL_UNCONSTsafely casts away constness before callingfree(), improving const-correctness without altering functionality. This is part of a broader pattern of const-correctness improvements across the codebase.deps/curl/lib/mime.c (1)
1745-1745: Improved array size calculation using CURL_ARRAYSIZE macro.Replacing the manual calculation
sizeof(ctts) / sizeof(ctts[0])with theCURL_ARRAYSIZE(ctts)macro improves code maintainability and prevents potential errors in manual size calculations. This is consistent with similar changes throughout the codebase.deps/curl/lib/hostip4.c (1)
289-289: Improved const-correctness using CURL_UNCONST.The replacement of the generic
(void *)cast with the more explicitCURL_UNCONST(hostname)macro improves const-correctness while maintaining the same functionality. This aligns with similar changes across the codebase for safer pointer handling.deps/curl/lib/curl_sspi.c (3)
45-45: Updated Windows CE platform detection macro.The change from
_WIN32_WCEtoUNDER_CEstandardizes Windows CE platform detection throughout the codebase, making platform-specific code more consistent and maintainable.
157-158: Removed unnecessary type cast for improved const-correctness.Removing the explicit cast from
(char *)userpallows the function to properly handle const parameters, improving type safety without changing functionality. Thecurlx_convert_UTF8_to_tcharfunction now properly accepts the const input parameter.
201-202: Removed unnecessary type cast for improved const-correctness.Similar to the earlier change, removing the explicit cast from
(char *)passwdpimproves const-correctness and type safety for the password parameter.deps/curl/lib/formdata.c (1)
240-240: Good improvement in const-correctnessThe change from direct casting with
(char *)to using theCURL_UNCONSTmacro improves const-correctness and type safety, making the code more maintainable across different platforms and compilers.deps/curl/lib/curlx.h (2)
64-66: Improved string parsing utilities inclusionReplacing the older string parsing utilities with the newer
strparse.hand adding clear documentation about the available functions improves code maintainability and clarity.
67-67: Added consistent memory freeing macroAdding the
curlx_safefreemacro as an alias toCurl_safefreeenhances consistency in memory management across the codebase.deps/curl/lib/gopher.c (2)
165-165: Improved const-correctness with CURL_UNCONSTUsing the
CURL_UNCONSTmacro instead of a direct cast improves const-correctness and follows the more modern and safer approach being implemented throughout the codebase.
239-239: Removed unnecessary castRemoving the redundant cast in the
Curl_client_writecall improves code clarity without affecting functionality.deps/curl/lib/rename.c (2)
43-43: Platform-specific refinement: Windows CE exclusionGood improvement to exclude Windows CE from the Windows-specific file rename implementation. This ensures Windows CE will use the standard
rename()function instead of the Windows-specificMoveFileEx()approach, which may not be available or might behave differently on CE.
49-50: Const-correctness improvementRemoved unnecessary casts from
const char *to(char *)in calls tocurlx_convert_UTF8_to_tchar(). This indicates the function signature has been updated to properly accept const parameters, which is good practice for maintaining proper const-correctness.deps/curl/lib/http_ntlm.c (2)
44-44: Added strparse.h inclusionGood addition of the strparse.h header which provides string parsing utilities used by the new implementation.
74-74: Enhanced parsing with standardized utilityReplaced manual whitespace-skipping logic with a call to
Curl_str_passblanks(), which improves code readability and consistency across the codebase. This standardizes string parsing and reduces potential for bugs.deps/curl/lib/amigaos.c (1)
187-187: Improved platform-specific error handlingUpdated the error condition check to use the platform-specific
SOCKEINTRmacro instead ofEINTRfor detecting interrupted system calls. This change aligns the error handling with AmigaOS socket semantics and is part of the broader effort to standardize socket error codes across platforms usingSOCKE*macros.deps/curl/lib/file.c (2)
243-243: Simplified memory managementChanged from
Curl_safefree(file->freepath)to directfree(file->freepath). This simplification is safe in this context becausefile->freepathis assigned a value just before this at line 244, so it should never be NULL at this point.
550-551: Extended platform compatibilityThe AmigaOS-specific
lseekcast logic has been extended to also apply to the__MINGW32CE__platform. This improves cross-platform compatibility by ensuring consistent type handling for thelseekoffset argument on Windows CE when compiled with MinGW.deps/curl/lib/select.h (3)
96-97: Socket error code standardization looks goodThis change standardizes the error code to
SOCKEINVALfor invalid sockets on Windows platforms, improving error handling consistency across the codebase.
108-109: Socket error code standardization looks goodSimilar to the Windows case, this change standardizes the error code to
SOCKEINVALfor invalid sockets on non-Windows platforms, ensuring consistent error handling.
125-125: New function supports the event-driven architectureThe addition of
Curl_pollfds_resetfunction declaration aligns with the new event-based socket and transfer management system introduced in this update. This function will reset the count of active poll file descriptors, supporting the event-driven model for efficient socket management.deps/curl/lib/llist.c (2)
35-39: Debug macro consolidation improves code structureGood refactoring to consolidate all debug macro definitions under a single
#ifdef DEBUGBUILDblock. This improves code readability and maintainability by grouping related conditional compilation directives.
88-88: Improved const-correctness with CURL_UNCONSTGood change to use the
CURL_UNCONSTmacro instead of a direct cast. This improves const-correctness and makes the intention of removing the const qualifier more explicit and safer.deps/curl/lib/if2ip.c (2)
70-71: Improved type safety in pointer castingThe updated cast adds an explicit
(const void *)step before casting to the specific socket structure type. This improves type safety by following proper C casting practices and avoids potential compiler warnings related to strict aliasing rules.
165-165: Consistent naming for IP address conversion functionsThe change from
Curl_inet_ntoptocurlx_inet_ntopaligns with the broader codebase updates to standardize IP address parsing and formatting functions. This ensures consistent API usage across all libcurl files.Also applies to: 238-238
deps/curl/lib/base64.c (2)
35-35: Improved build configuration conditionThe change from
defined(BUILDING_CURL)to!defined(BUILDING_LIBCURL)aligns with the updated build definition introduced indeps/curl/lib/CMakeLists.txt. This provides more precise control over header inclusion based on the build target.
194-194: Enhanced const-correctnessGood improvement to add the
constqualifier to theinpointer, properly preserving the constness of theinputbuffparameter. This prevents accidental modification of the input buffer and aligns with the broader effort to improve const-correctness throughout the codebase.deps/curl/lib/inet_pton.h (1)
29-29: Function renamed for consistency across the codebaseThe function and macros have been renamed from
Curl_inet_ptontocurlx_inet_pton, consistent with the broader standardization of naming conventions in the libcurl codebase. This brings the function naming in line with other utility functions that use thecurlx_prefix.For the Amiga platform, the implementation now uses
CURL_UNCONSTinstead of a direct cast, which improves const-correctness by using a standardized macro for casting away const qualifiers.Also applies to: 41-44
deps/curl/lib/dynbuf.h (1)
84-85:Details
✅ Verification successful
Added maximum buffer size definitions for dynamic buffers
These new macro definitions establish maximum size limits for different types of dynamic buffers:
MAX_DYNBUF_SIZE: Sets a general upper bound at half of SIZE_T_MAXDYN_MQTT_SEND: Defines a large limit for MQTT send operations (0xFFFFFFF)DYN_CRLFILE_SIZE: Sets a 400MB limit for CRL filesThese additions help prevent excessive memory allocation and establish consistent size constraints across the codebase.
Also applies to: 100-101
🏁 Script executed:
#!/bin/bash # Check if these buffer size limits are properly enforced in the code # Search for MAX_DYNBUF_SIZE usage echo "Checking MAX_DYNBUF_SIZE usage:" rg "MAX_DYNBUF_SIZE" --type c # Search for the enforcement of MQTT buffer size echo -e "\nChecking MQTT buffer size enforcements:" rg "DYN_MQTT_SEND" --type cLength of output: 507
Let’s verify that
DYN_CRLFILE_SIZEis actually referenced in C sources:
🏁 Script executed:
#!/bin/bash # Check usage of the CRL file buffer size macro echo "Checking DYN_CRLFILE_SIZE usage:" rg "DYN_CRLFILE_SIZE" --type c -nLength of output: 273
Buffer size limits are properly enforced
Verified that each macro is used to cap dynamic‐buffer allocations:
- deps/curl/lib/dynbuf.c
• DEBUGASSERT(toobig <= MAX_DYNBUF_SIZE)- deps/curl/lib/mqtt.c
• Curl_dyn_init(&mq->sendbuf, DYN_MQTT_SEND)- deps/curl/lib/vtls/rustls.c (line 695)
• Curl_dyn_init(&crl_contents, DYN_CRLFILE_SIZE)All three limits are applied in the relevant initialization and sanity‐check code.
deps/curl/lib/inet_pton.c (1)
75-75: Improved error handling and renamed function for consistencyTwo key improvements were made:
The function was renamed from
Curl_inet_ptontocurlx_inet_ptonto maintain consistency with the header file changes and the broader naming convention in the codebase.Error handling for unsupported address families now uses the
CURL_SETERRNO(SOCKEAFNOSUPPORT)macro instead of directly settingerrnotoEAFNOSUPPORT. This improves platform independence by using a standardized error-setting mechanism.Also applies to: 83-83
deps/curl/lib/cf-haproxy.c (1)
106-109:Details
✅ Verification successful
Removed blocking parameter from connect function signature
The
blockingparameter has been removed from thecf_haproxy_connectfunction signature, and the corresponding call to the next filter'sdo_connectmethod has been updated to omit this parameter. This change is part of a larger refactoring effort to simplify the connection filter API by eliminating the blocking parameter across various connection filters.This simplification suggests a shift in how connections are established, likely moving toward a more consistently non-blocking approach managed at a higher level in the connection handling stack.
Also applies to: 120-120
🏁 Script executed:
#!/bin/bash # Verify this pattern across other connection filters echo "Checking other connection filter implementations for similar changes:" # Look for connection filter connect signatures across the codebase rg "static CURLcode cf_.+_connect\(" --type c deps/curl/lib/ -A 3Length of output: 4482
Confirm uniform removal of
blockingparameter across connection filtersVerified that all
cf_*_connectimplementations (e.g. cf_h2_connect, cf_he_connect, cf_haproxy_connect, cf_h1_proxy_connect, cf_tcp_connect, cf_quiche_connect, etc.) now consistently use the simplified signature:static CURLcode cf_<filter>_connect(struct Curl_cfilter *cf, struct Curl_easy *data, bool *done)This aligns with the refactoring to centralize non-blocking behavior at a higher level in the stack; no further changes are required.
• Affected file:
– deps/curl/lib/cf-haproxy.c (lines 106–109)
• Change applies uniformly across all connection filters.deps/curl/lib/idn.c (2)
67-67: Good use of CURL_UNCONST macro for improving const-correctness.The change from direct casting
(char *)into usingCURL_UNCONSTmacro improves code clarity and standardizes const-casting across the codebase. This approach explicitly signals intentional const-casting required by the iconv API rather than casual disregard for const-correctness.
74-75: Appropriate static analysis suppression for errno usage.Adding
/* !checksrc! disable ERRNOVAR 1 */comments properly acknowledges the intentional direct use of errno after iconv failures. This correctly informs static analyzers that this errno usage pattern has been reviewed and approved.Also applies to: 84-85
deps/curl/lib/easyoptions.c (1)
356-356: Proper addition of the new UPLOAD_FLAGS option.The new CURLOPT_UPLOAD_FLAGS option is correctly added to the options array in alphabetical order with appropriate type (CURLOT_LONG) and flags (0). This cleanly expands libcurl's API capabilities.
deps/curl/lib/pingpong.h (2)
72-72: Improved const-correctness in endofresp function pointer.Changing from
char *ptrtoconst char *ptrappropriately enforces that the callback doesn't modify the input buffer. This is a good practice that makes the function's intent clearer and aligns with the broader const-correctness improvements in the codebase.
73-73: Good addition of initialization tracking.Adding the
initialisedbitfield member improves robustness by explicitly tracking the initialization state of the pingpong structure. This supports the new connection shutdown subsystem and helps prevent issues like use-before-initialization or double-initialization.deps/curl/lib/http_digest.c (3)
33-33: Appropriate header inclusion for string parsing utilities.Adding the include for "strparse.h" properly provides access to the Curl_str_passblanks function used in the refactored code.
66-66: Good refactoring to use centralized utility function.Replacing manual blank-skipping with
Curl_str_passblanks(&header)improves code maintainability and consistency. This follows DRY principles and aligns with similar refactoring throughout the codebase.
148-151: Improved const-correctness in string operations.Changing casts from
(char *)to(const char *)when working with theuripathparameter properly preserves the const qualifier. This improves code correctness by explicitly indicating that these operations don't modify the source string.Also applies to: 156-156
deps/curl/lib/fopen.c (1)
108-114: Platform-specific file status check enhancement.The conditional file status check code improves Windows CE compatibility by using
stat()on the filename instead offstat()on the file descriptor. This is a good adjustment for platform-specific behavior that maintains functionality across different environments.deps/curl/lib/curl_memrchr.c (1)
56-56: Improved type safety using CURL_UNCONST macro.Replacing the direct C-style cast with the
CURL_UNCONSTmacro enhances type safety and maintains consistent casting patterns throughout the codebase. This is a good modernization improvement.deps/curl/lib/http_negotiate.c (5)
35-35: Added string parsing utility header.Good addition of the string parsing utility header that supports standardized string handling functions used later in the code.
90-90: Improved string parsing with standardized function.Replacing manual whitespace skipping with
Curl_str_passblanks(&header)improves code quality and safety by using a standardized function instead of ad-hoc string manipulation.
112-124: Restructured conditional compilation for better clarity.This reorganization of conditional compilation logic better separates GSSAPI initialization from SSL-specific code. The channel binding data initialization is now performed for all GSSAPI builds, while SSL-specific operations are correctly nested within their own conditional.
130-132: Improved cleanup logic for GSSAPI builds.Proper cleanup of channel binding data is now consistently performed for all GSSAPI builds, improving resource management.
208-209: Standardized memory management pattern.Replacing
Curl_safefree()with directfree()calls aligns with modern memory management patterns in the codebase. This change maintains proper deallocation while potentially simplifying the code base by reducing indirection.Also applies to: 213-214
deps/curl/lib/curl_sspi.h (2)
73-291: Added missing SSPI-related constants for MinGW32CE compatibility.This comprehensive addition of security-related constants (
ISC_RET_*flags andSEC_E_*error codes) enhances MinGW32CE platform support. Each definition is properly guarded with#ifndefto prevent redefinition conflicts, making this a thorough and careful enhancement for cross-platform compatibility.
302-327: Added missing informational status codes for MinGW32CE.The addition of
SEC_I_*informational status codes completes the necessary Windows security definitions for MinGW32CE. These constants are essential for proper functioning of the SSPI authentication mechanisms on this platform.deps/curl/lib/ftp.h (3)
128-130: Improved documentation for newhost fieldThe added comment clarifies that
newhostis for the data connection, making it clear this is an allocated IP address or hostname specifically for establishing FTP data connections rather than control connections.
143-144: Better documentation for newport fieldThis comment helpfully clarifies that
newportis associated withnewhostfor data connection establishment, making the relationship between these fields explicit.
163-163: Added shutdown flag to improve connection state trackingThe new
shutdownbit flag enhances connection state management by explicitly tracking when an FTP connection is being shut down (e.g., during QUIT command processing). This supports the library's new non-blocking connection shutdown subsystem.deps/curl/lib/noproxy.c (5)
32-32: Added strparse.h inclusion for string parsing utilitiesIncluding the string parsing utilities header enables the use of standardized string handling functions throughout the code.
57-60: Improved IPv4 address parsing with curlx_inet_ptonReplacing
Curl_inet_ptonwithcurlx_inet_ptonstandardizes IP address parsing, which improves platform abstraction and error handling consistency.
95-98: Improved IPv6 address parsing with curlx_inet_ptonSimilar to the IPv4 change, using
curlx_inet_ptonfor IPv6 addresses provides consistent handling across the codebase.
181-182: Standardized blank-skipping with Curl_str_passblanksReplacing manual character-by-character blank skipping with the standardized
Curl_str_passblanksfunction improves code consistency and reduces the chance of subtle parsing bugs.
250-251: Standardized blank-skipping with Curl_str_passblanksAnother instance of standardizing blank character skipping with
Curl_str_passblanks, maintaining consistent string parsing patterns throughout the codebase.deps/curl/curl.gyp (6)
21-21: Added connection shutdown subsystem source fileIncluding
cshutdn.cis necessary for the new non-blocking connection shutdown management that replaces the legacy shutdown queue system.
40-40: Added pause writer implementationThe
cw-pause.cfile implements a client writer that buffers outgoing data during paused transfers, enhancing the library's pause/resume functionality.
59-59: Added integer-keyed hash table implementationThe
hash_offt.cfile provides a specialized hash table implementation keyed bycurl_off_tintegers, which is particularly useful for HTTP/2 stream management.
93-93: Added event-based socket management systemIncluding
multi_ev.cintroduces the new event-based socket tracking system that replaces the legacy socket hash, improving the multi interface's event handling capabilities.
122-122: Added string equality utilitiesAdding
strequal.cprovides standardized string comparison functions used throughout the codebase.
35-35: Added additional source files for various enhancementsThe build configuration properly includes several other new source files to support features like SHA-512/256 hashing, HTTP specific resource records, string parsing, and TLS session cache management.
Also applies to: 39-39, 76-76, 104-104, 124-124, 162-163
deps/curl/lib/connect.h (2)
35-35: Improved const-correctness for ALPN protocol name handlingChanging the
Curl_alpn2alpnidfunction to accept aconst char *instead ofchar *properly indicates that the input string won't be modified, improving code clarity and safety.
48-48: Enhanced connection shutdown with explicit timeout parameterAdding an explicit
timeout_msparameter toCurl_shutdown_startsupports the new connection shutdown subsystem by allowing precise control over shutdown timeouts. This change aligns with the introduction of the non-blocking, incremental shutdown management in the library.deps/curl/lib/Makefile.am (5)
67-72: Proper debug build flag configurationNew debug build flags ensure appropriate instrumentation during development. The addition of separate
DEBUGBUILDandCURLDEBUGflags allows for more granular control over debug features.
77-83: Improved Windows platform-specific file handlingThe conditional inclusion of
dllmain.cspecifically for Windows and Cygwin builds improves platform-specific optimizations and avoids unnecessary compilation on other platforms.
89-93: More precise debug build exclusion conditionsThe change from
DEBUGBUILDtoCURLDEBUGfor excluding memory debugging sources from unity builds shows improved specificity in debug configurations, separating general debug builds from curl-specific debugging features.
94-98: Proper Cygwin build isolationExcluding
dllmain.cfrom unity builds on Cygwin prevents Windows.h header inclusion in other compilation units, avoiding potential symbol conflicts and compilation issues.
184-185: Enhanced static analysis toolingThe tidy target improvements prepend source directory paths correctly, enabling more accurate source file location for static analysis tooling.
deps/curl/lib/curl_config.h.cmake (1)
472-473: Added support for stdint.hAdding
HAVE_STDINT_Hindicates improved usage of standard fixed-width integer types, which enhances portability across different platforms and architectures.deps/curl/lib/curl_threads.h (1)
56-57: Updated Windows CE platform detectionThe conditional was updated from
_WIN32_WCEtoCURL_WINDOWS_UWP || UNDER_CE, aligning with platform-specific macro standardization throughout the codebase.deps/curl/include/curl/system.h (2)
39-40: Strengthened curl_off_t requirementThe documentation now emphasizes that
curl_off_tMUST be a 64-bit wide signed integral type without exceptions, reinforcing the importance of consistent type definitions across platforms.
137-152: Improved Windows CE platform detection and type definitionThe platform detection was refined from
_WIN32_WCEtoUNDER_CEwith specialized handling for__MINGW32CE__. This change standardizes platform detection macros while accommodating compiler-specific differences, ensuring appropriate 64-bit type definitions across environments.deps/curl/lib/curl_multibyte.c (2)
87-87: Improved platform compatibility with Windows CEThe addition of the
#ifndef UNDER_CEand corresponding#endifdirectives properly excludes this Windows-specific path handling code from Windows CE builds, which have different capabilities and limitations.Also applies to: 358-358
272-273: Enhanced error handling standardizationThe code now uses the
CURL_SETERRNO(EINVAL)macro instead of directerrno = EINVALassignments, which provides a more consistent approach to error handling across different platforms. The added comments disable static analysis warnings for these specific cases.Also applies to: 303-304, 340-341
deps/curl/lib/curl_ntlm_core.c (3)
74-81: Support for AWS LibCrypto addedThe conditional for
OPENSSL_IS_AWSLCproperly handles compatibility with the AWS LibCrypto implementation of OpenSSL, ensuring correct function usage via macro definitions.
324-333: Improved const-correctness with CURL_UNCONSTThe use of the
CURL_UNCONST()macro properly handles the casting of const pointers when callingDES_ecb_encrypt(), maintaining type safety while working with legacy APIs that don't properly support const parameters.Also applies to: 378-383
456-457: Better code organization for NTLMv2 constantsMoving the definitions of
NTLMv2_BLOB_SIGNATUREandNTLMv2_BLOB_LENcloser to their usage point improves code readability and maintainability.deps/curl/lib/select.c (3)
77-77: Standardized socket error handling across platformsThe code now uses
SOCKEINVALandSOCKEINTRmacros instead of directEINVALandEINTRerror codes, which provides better cross-platform compatibility by abstracting platform-specific error handling details.Also applies to: 315-315, 363-363
89-89: Proper type casting for Windows Sleep functionChanged from
(ULONG)to(DWORD)for the WindowsSleepfunction parameter, which is more correct as the Sleep function expects a DWORD parameter type.
413-417: New utility function for resetting poll file descriptor setsThe new
Curl_pollfds_resetfunction allows resetting the count of active poll file descriptors without deallocating memory, which is useful for reusing existing poll structures in successive operations.deps/curl/lib/mqtt.h (1)
52-59: Improved MQTT buffer management with dynbufThe structure has been reorganized to use the more structured
dynbuftype instead of raw pointers and size variables for buffer management. This provides better encapsulation, more consistent memory management, and groups related fields together (send/receive buffers).deps/curl/lib/http_chunks.c (3)
37-37: Modernized string parsing with improved header inclusion.The header inclusion change from
strtoofft.htostrparse.hreflects codebase-wide refactoring to use more robust string parsing utilities.
161-175: Improved chunk size parsing with better error handling.The chunk size parsing has been enhanced with proper const-correctness and using the safer
Curl_str_hex()function instead ofcurlx_strtoofft(). This change improves code safety and error reporting.
137-137: Removed unnecessary casts for improved type safety.Unnecessary casts to
(char *)for buffer pointers have been removed in the calls toCurl_client_write(). This improves const-correctness without changing functionality.Also applies to: 216-216
deps/curl/lib/curl_krb5.h (1)
43-44: Improved connection lifecycle management for Kerberos security contexts.The previous
Curl_sec_end()function has been split into initialization and destruction phases withCurl_sec_conn_init()andCurl_sec_conn_destroy(). This change better aligns with the connection lifecycle model and improves resource management by separating initialization from cleanup.Also applies to: 50-51
deps/curl/lib/netrc.c (3)
42-42: Added modern string parsing utilities.Including
strparse.henables the use of safer, more centralized string parsing functions throughout the code.
89-90: Simplified blank space handling with dedicated helper.Replaced manual loops for skipping blank characters with the centralized
Curl_str_passblanks()function, making the code more consistent and easier to maintain.Also applies to: 146-147
141-144: Improved const-correctness and token parsing logic.Changed pointer variables from
char *toconst char *to better reflect their usage as read-only access to data. The token parsing loop condition has been refined from checking all whitespace to checking specifically for characters greater than space, providing more precise token boundary detection.Also applies to: 164-167
deps/curl/lib/md5.c (2)
255-259: Enhanced platform compatibility for MinGW CE.Added a platform-specific conditional for MinGW CE to properly cast the input pointer using
CURL_UNCONSTwhen callingCryptHashData(). This addresses API compatibility issues specific to this platform.
363-363: Improved const-correctness in MD5 implementation.The
MD5_SETmacro now properly usesconst MD5_u32plus *for the pointer cast, maintaining const-correctness throughout the hashing process.deps/curl/lib/dynbuf.c (5)
35-37: Security improvement: Debug pattern only defined in debug buildsThe
DYNINITmacro is now only defined in debug builds, which is a good practice to ensure that debug initialization patterns don't leak into production code.
46-46: Enhanced debugging: Added buffer size limit checkThis additional assertion helps catch potential issues with buffer sizes early in development, preventing hard-to-diagnose buffer overflows.
63-63: Improved robustness: Added initialization verificationThis assertion ensures that the dynamic buffer was properly initialized before being freed, which helps catch memory management bugs.
188-188: Enhanced type safety: Improved const-correctnessThe explicit cast to
const unsigned char *improves type safety by properly preserving constness when passing string data to internal functions.
214-214: Enhanced type safety: Improved const-correctnessSimilar to the previous change, this const cast ensures proper handling of string data types in the non-libcurl build path.
deps/curl/lib/dict.c (4)
216-216: Code refactoring: Simplified default value handlingThe conditional expression now handles the default word value directly at the usage site rather than having separate assignment statements, making the code more concise.
229-231: Code refactoring: Consolidated default parameter handlingThe conditional expression for database and strategy parameters replaces previous conditional assignments, resulting in cleaner code without changing behavior.
259-259: Code refactoring: Simplified default value handlingSimilar to the earlier change, this uses an inline conditional to handle the default word case, making the code more consistent.
271-271: Code refactoring: Consolidated default parameter handlingThis change continues the pattern of using inline conditionals for default parameter handling, improving code consistency.
deps/curl/lib/curl_addrinfo.c (6)
121-121: Standardization: Using platform abstraction macroReplaced direct
getaddrinfocall with theCURL_GETADDRINFOmacro, which improves portability and allows for centralized handling of platform-specific implementations.
187-187: Standardization: Using platform abstraction macroSimilar to the previous change, this uses
CURL_FREEADDRINFOmacro instead of direct function call, enhancing platform abstraction.
433-433: API standardization: Using consistent IP conversion functionChanged from
Curl_inet_ptontocurlx_inet_pton, which standardizes the naming convention for platform abstraction functions.
439-439: API standardization: Using consistent IP conversion functionSimilar to the IPv4 case, this standardizes the IPv6 function call to use
curlx_inet_ptonfor consistent naming across the codebase.
512-512: Code cleanup: Removed unnecessary parenthesesSimplified the function call by removing superfluous parentheses, improving code readability without affecting behavior.
537-537: Code cleanup: Removed unnecessary parenthesesSimilar to the previous change, this removes extra parentheses from the function call, making the code cleaner.
deps/curl/lib/inet_ntop.h (3)
29-29: API standardization: Function prototype renamedThe function was renamed from
Curl_inet_ntoptocurlx_inet_ntopfor consistency with other platform abstraction functions, following a unified naming convention.
41-44: API standardization and type safety: Updated Amiga macroThe Amiga-specific macro was renamed for consistency and now uses the
CURL_UNCONSTmacro for safer const removal when casting, improving type safety.
46-47: API standardization: Updated standard macro nameThe standard inet_ntop wrapper macro was renamed to match the new naming convention, maintaining consistent API naming throughout the codebase.
deps/curl/lib/cw-pause.h (1)
29-38: Approve the architectural change to the pause client writerThis header file has been completely rewritten to remove the previous thread-safe
strtok_rwrapper, replacing it with declarations for the new client writer typeCurl_cwt_pauseand its flush function. This architectural change aligns with the broader addition of pause client writer functionality that buffers outgoing data during paused transfers.deps/curl/lib/headers.c (4)
32-32: Good addition of the strparse.h includeAdding support for the new string parsing utilities from
strparse.haligns with the broader effort across the codebase to replace manual string parsing with safer, consistent utilities.
212-214: Improved whitespace handling with ISBLANKReplacing
ISSPACEwithISBLANKnarrows whitespace handling to spaces and tabs only, which is more appropriate for header parsing and aligns with HTTP standards.
218-220: Consistent use of ISBLANK for trailing spacesConsistently using
ISBLANKinstead ofISSPACEfor trimming trailing whitespace.
238-240: Consistent use of ISBLANK in unfold_valueConsistently using
ISBLANKinstead ofISSPACEfor trimming trailing whitespace in the unfold_value function.deps/curl/lib/getinfo.c (6)
34-34: Good addition of the strparse.h includeAdding support for the new string parsing utilities from
strparse.haligns with the broader effort to improve numeric parsing safety across the codebase.
101-101: Removed unnecessary castRemoved an unnecessary cast to
(const char *)sincedata->state.urlis already of the correct type.
207-211: Improved numeric parsing safetyReplaced direct usage of
strtoulwith the saferCurl_str_numberfunction that includes explicit bounds checking (TIME_T_MAX), reducing the risk of numeric overflow.
220-224: Consistent use of Curl_str_number for env var parsingUsing
Curl_str_numberwith explicit bounds checking (LONG_MAX) for theCURL_DEBUG_SIZEenvironment variable, consistent with other numeric parsing improvements.
384-388: Improved numeric parsing safety in getinfo_offtUsing
Curl_str_numberwith explicit bounds checking (CURL_OFF_T_MAX) for theCURL_TIMEenvironment variable ingetinfo_offt.
483-487: Improved numeric parsing safety in getinfo_doubleUsing
Curl_str_numberwith explicit bounds checking (CURL_OFF_T_MAX) for theCURL_TIMEenvironment variable ingetinfo_double.deps/curl/lib/curl_threads.c (3)
106-111: Improved platform-specific conditional compilationUpdated conditional compilation to check for both
CURL_WINDOWS_UWPandUNDER_CEinstead of_WIN32_WCE, unifying the handling of Windows CE and Windows UWP platforms.
114-126: Consistent platform-specific thread handlingUpdated conditional compilation for thread handle type and thread creation functions, ensuring consistent platform detection between the type definitions and usage.
128-135: Improved error handling for Windows CEAdded more detailed error handling for Windows CE with appropriate error code mapping, and included a static analysis warning suppression for the unused errno variable.
deps/curl/lib/inet_ntop.c (5)
41-41: Removed INADDRSZ macro that's no longer needed.The INADDRSZ macro has been commented out as it's no longer used in the code, improving code cleanliness by removing unused definitions.
77-81: Improved platform-specific error handling.The error handling now uses the CURL_SETERRNO macro with platform-specific error codes (WSAEINVAL for Windows, ENOSPC for other platforms) instead of direct errno assignments, making the error reporting more consistent and platform-aware.
177-181: Consistent error handling for IPv6 address conversion.The same platform-specific error handling is applied for IPv6 address conversion, ensuring consistent behavior across different address formats.
199-199: Renamed function from Curl_inet_ntop to curlx_inet_ntop.This function rename follows the library's pattern of using the
curlx_prefix for certain utility functions, improving naming consistency throughout the codebase.
207-207: Using CURL_SETERRNO macro for error reporting.Replaced direct errno assignment with the CURL_SETERRNO macro, aligning with the library's error handling patterns.
deps/curl/lib/parsedate.c (6)
86-86: Added strparse.h include for safer string parsing utilities.Including strparse.h provides access to safer string manipulation and parsing functions, particularly Curl_str_number, which will replace manual string-to-number conversions.
104-106: Clarified conditional compilation for PARSEDATE_SOONER.Improved readability of the conditional compilation for the PARSEDATE_SOONER definition, making the code behavior more explicit based on time_t properties.
259-259: Using CURL_ARRAYSIZE macro instead of hardcoded size.Replaced hardcoded array size checks with the CURL_ARRAYSIZE macro, improving code maintainability and reducing potential errors if the array size changes.
415-432: Replaced manual string-to-number conversion with safer Curl_str_number.This change improves safety by using the Curl_str_number function instead of manual strtol/strtoul conversions, providing better error handling and bounds checking.
342-342: Improved const correctness with CURL_UNCONST.Added CURL_UNCONST cast to properly handle the conversion from const to non-const pointer, maintaining proper const semantics.
454-457: Clarified digit counting and validation logic.Improved the logic for checking the number of digits in a date/time string for better readability and maintainability.
deps/curl/lib/http_proxy.c (5)
46-46: Added strparse.h include for string parsing utilities.Including strparse.h provides access to string parsing utilities like Curl_str_passblanks, which will be used to replace manual whitespace handling.
64-64: Improved const correctness for ptr variable.Changed 'ptr' from 'char *' to 'const char *' since it's only being read, not modified, improving type safety and const correctness.
112-112: Replaced manual whitespace skipping with Curl_str_passblanks.Eliminated manual loops for skipping whitespace by using the centralized Curl_str_passblanks utility, making the code more maintainable and consistent with other similar operations in the codebase.
Also applies to: 134-134
307-309: Simplified function signature by removing blocking parameter.The HTTP proxy connection function no longer requires the 'blocking' parameter, simplifying the API. This change is part of a broader refactoring of connection filter implementations.
321-321: Updated do_connect call to match new function signature.The call to the next filter's do_connect method now correctly omits the 'blocking' parameter that was removed from the function signature.
deps/curl/lib/request.c (5)
37-37: Added strparse.h include for string parsing utilities.Including strparse.h provides access to safer string parsing functions, particularly Curl_str_number, which will be used for environment variable parsing.
72-72: Initialized sendbuf_hds_len in soft reset function.Added proper initialization of the sendbuf_hds_len field during request soft reset, preventing potential issues with uninitialized values.
146-146: Initialized sendbuf_hds_len in hard reset function.Ensuring the sendbuf_hds_len field is properly reset when performing a hard reset of the request structure.
201-207: Improved environment variable parsing with Curl_str_number.Replaced manual strtoul parsing of the CURL_SMALLREQSEND environment variable with the safer Curl_str_number function, which provides better error handling and bounds checking.
231-235: Improved debug output calls with better const handling.Removed unnecessary casts in debug output calls, passing buffer pointers directly as const char*, which improves code cleanliness and readability.
deps/curl/include/curl/curlver.h (1)
73-73: Confirm release timestamp
"2025-04-02"does not match the upstream 8.13.0 tag (2024-??-??). If this is intentional (e.g. downstream rebuild) it is fine, otherwise please double-check to avoid confusion for users parsing this macro.deps/curl/lib/content_encoding.c (3)
68-72: Nice compile-time gating ofDECOMPRESS_BUFFER_SIZEDefining the buffer only when at least one decompressor is enabled avoids unused-macro warnings on minimalist builds. 👍
75-77: Raising the required zlib version — make sure the build scripts agree
#error "requires zlib 1.2.5.2 or newer"stops the compiler early, butconfigure,cmake, etc. must also enforce the same floor or the user will hit a puzzling compile failure instead of an earlier feature test.
218-226:inflateReset2()is safe only with zlib ≥ 1.2.4You already bumped the minimum to 1.2.5.2, so that’s satisfied. Just pointing out that older embedded tool-chains will now hard-fail; if that is outside the supported matrix, no action needed.
deps/curl/lib/krb5.c (1)
174-178: Good use ofCURL_UNCONSTThe new macro makes the const-cast explicit and safer than a plain C-style cast.
deps/curl/lib/md4.c (2)
173-178: Const-correct Windows Crypto call looks goodNice catch switching to
(const BYTE *)dataand usingCURL_UNCONSTfor the
Windows CE special-case. This removes a boat-load of noisy MSVC warnings while
keeping the code path safe for the general Win32 API.
316-325: Minor: retain const-correctness throughout macro expansionThe updated
MD4_SETmacro now dereferences aconst MD4_u32plus *, ensuring
that strict-aliasing and const rules are respected on little-endian targets.
No further action required.deps/curl/lib/multihandle.h (3)
30-32: Verify header include order & conditional guards
cshutdn.handmulti_ev.hare now included unconditionally.
If either header relies on platform–specific prerequisites (e.g.#ifdef USE_*) but is not self-guarded, compilation may break on trimmed builds (e.g.CURL_DISABLE_SOCKETPAIR,CURL_DISABLE_HTTP, etc.). Please double-check that both headers:
- Contain their own
#ifndef HEADER_*guards, and- Protect any platform-specific code with the appropriate
#ifdefs.Failing to do so could cause symbol/typedef leaks in minimal feature builds.
147-148: Member re-ordering may break ABI for out-of-tree codeEven though
struct Curl_multiis an internal header, a few ecosystem
projects (static analysers, fuzzers, unit tests) include it directly.
Movingcpooldown by 1-2 words changes the binary layout and can cause
silent memory corruption if those projects are compiled against the old
header but linked to the new lib. Please highlight this in the changelog
or bump the appropriate internal ABI tag.
104-105:Details
✅ Verification successful
New
admineasy-handle requires explicit life-cycle management
struct Curl_multinow owns an internalCurl_easy *admin.
Make sure that:
- It is initialised to
NULLinCurl_multi_handle()(calloc already helps, but explicit assignment improves readability).- It is freed and removed from all l‐lists in
Curl_multi_cleanup()/error unwind paths; otherwise we risk a single-easy leak per multi handle.If this is already handled in
multi.cfeel free to ignore.
🏁 Script executed:
#!/bin/bash set -e echo "Searching for Curl_multi_handle in deps/curl/lib/multi.c" rg "Curl_multi_handle" -n deps/curl/lib/multi.c || echo "Curl_multi_handle not found" echo echo "Searching for curl_multi_init in deps/curl/lib/multi.c" rg "curl_multi_init" -n deps/curl/lib/multi.c || echo "curl_multi_init not found" echo echo "Searching for 'admin' references in deps/curl/lib/multi.c" rg "admin" -n deps/curl/lib/multi.c || echo "No 'admin' references in multi.c" echo echo "Searching for Curl_multi_cleanup in deps/curl/lib/multi.c" rg "Curl_multi_cleanup" -n deps/curl/lib/multi.c || echo "Curl_multi_cleanup not found" echo echo "Searching for cleanup of admin (free or easy_cleanup calls)" rg "easy_cleanup" -n deps/curl/lib/multi.c || echo "No easy_cleanup calls" rg "free(" -n deps/curl/lib/multi.c | grep admin || echo "No free(admin) calls"Length of output: 2925
🏁 Script executed:
#!/bin/bash set -e echo "Searching for curl_multi_cleanup in deps/curl/lib/multi.c" rg -n "curl_multi_cleanup" deps/curl/lib/multi.c || echo "curl_multi_cleanup not found" if rg -q "curl_multi_cleanup" deps/curl/lib/multi.c; then echo echo "Context around curl_multi_cleanup:" rg -n -B 5 -A 30 "curl_multi_cleanup" deps/curl/lib/multi.c fiLength of output: 3244
🏁 Script executed:
#!/bin/bash set -e echo "Displaying lines 200-260 of deps/curl/lib/multi.c for Curl_multi_handle initialization" sed -n '200,260p' deps/curl/lib/multi.cLength of output: 2018
🏁 Script executed:
#!/bin/bash set -e echo "Excerpt around Curl_multi_handle end (lines 260-340):" sed -n '260,340p' deps/curl/lib/multi.c echo echo "Error handling block in Curl_multi_handle:" rg -n '^error:' -A20 -B5 deps/curl/lib/multi.cLength of output: 3242
Admin handle life-cycle already covered—no changes needed
- The
Curl_multi_handlefunction usescalloc(1, …), which zeroesmulti->admintoNULLbefore it’s next assigned.- In the error unwind path,
multi->adminis checked and cleaned up (multi->admin->multi = NULL; Curl_close(&multi->admin);).- In
curl_multi_cleanup, after processing user handles, the internal admin handle is likewise destroyed and closed (Curl_cshutdn_destroy(..., multi->admin); … Curl_close(&multi->admin);).All suggested init and teardown steps for
multi->adminare already implemented.deps/curl/lib/cf-h2-proxy.c (1)
1092-1110:Details
✅ Verification successful
Call-site update looks good – verify removal of the old 4-arg signature
cf_h2_proxy_connect()now calls
Curl_conn_cf_connect(cf->next, data, done);– excellent.Please grep the codebase for any lingering calls with the obsolete
blockingparameter to prevent build breaks on less-used filters.
🏁 Script executed:
#!/bin/bash # Detect obsolete 4-parameter invocations rg -n $'Curl_conn_cf_connect\\([^,]+,[^,]+,[^,]+,[^,]+\\)' | headLength of output: 64
🏁 Script executed:
#!/bin/bash echo "All occurrences of Curl_conn_cf_connect:" rg -n "Curl_conn_cf_connect(" echo "" echo "Potential 4-argument invocations (3 commas inside parentheses):" rg -n -P 'Curl_conn_cf_connect\([^)]*(?:,[^)]*){3}\)'Length of output: 419
🏁 Script executed:
#!/bin/bash # List all calls using a literal search to avoid regex group errors echo "=== All calls to Curl_conn_cf_connect ===" rg -F -n 'Curl_conn_cf_connect(' || true echo # Detect invocations with four arguments (three commas) using extended regex echo "=== Potential 4-argument invocations (3 commas inside parentheses) ===" grep -R -n -E 'Curl_conn_cf_connect\([^,]*,[^,]*,[^,]*,[^,]*\)' .Length of output: 1594
No obsolete 4-arg
Curl_conn_cf_connectcalls foundI searched the entire codebase for any invocations of
Curl_conn_cf_connectwith four parameters (three commas inside the parentheses) and found none—every call now uses the new three-argument signature. No further changes are needed.deps/curl/lib/cf-h1-proxy.c (4)
47-47: Addedstrparse.hfor improved string parsing functionality.The inclusion of
strparse.hsupports the new string parsing utilities being used in this file.
318-322: Improved Content-Length parsing with better error handling.This change replaces direct numeric parsing with the safer
Curl_str_numblanks()function, adding explicit error handling for invalid Content-Length values. This approach is more robust and consistent with modern string parsing practices in libcurl.
644-644: Simplified function signature by removing the blocking parameter.The removal of the
blockingparameter fromcf_h1_proxy_connect()aligns with the broader refactoring of connection functions across libcurl to use a consistently non-blocking model.
604-604: Updated function call to match the new signature without the blocking parameter.This change correctly updates the call to
Curl_conn_cf_connect()to match its new signature that no longer includes the blocking parameter.deps/curl/lib/curl_range.c (2)
29-29: Replacedstrtoofft.hwithstrparse.hfor modern string parsing.This change adopts the newer string parsing utilities which provide improved safety and error handling.
41-84: Improved HTTP range parsing with better validation and error handling.The refactored code brings several improvements:
- Uses the safer
Curl_str_number()andCurl_str_single()functions for parsing- Maintains a boolean flag to track successful number parsing
- Adds explicit validation for the dash separator
- Rejects invalid ranges like "-0"
- Provides clearer handling of the three range types (X-, -Y, X-Y)
This enhances robustness and readability while maintaining the same functionality.
deps/curl/lib/pop3.c (3)
247-247: Improved const-correctness in function parameter.Changing
char *linetoconst char *lineprevents accidental modification of the input string and better communicates the function's intent.
1583-1584: Removed unnecessary casts when passing string constants.The removal of
(char *const)casts when passingPOP3_EOBto functions improves code clarity without changing functionality. This is consistent with improved const-correctness throughout the codebase.Also applies to: 1587-1588, 1607-1607
1393-1394: Simplified function implementation by removing unused parameter.The function now explicitly marks the
connparameter as unused with(void)connand directly callspop3_init()with only thedataparameter, improving code clarity.deps/curl/lib/Makefile.inc (2)
125-125: Added new source files for improved connection and event management.The addition of these source files indicates significant internal improvements:
cshutdn.c: New connection shutdown management subsystemcw-pause.c: Pause buffering functionalityhash_offt.c: Hash table implementation keyed bycurl_off_tmulti_ev.c: Event-based socket tracking for the multi interfacestrequal.c: String equality functionsThese additions reflect a positive modernization effort in libcurl's internal architecture.
Also applies to: 147-147, 166-166, 201-201, 231-231
261-261: Added corresponding header files for the new subsystems.The header files complement the new source files and provide the necessary interfaces for the new subsystems:
cshutdn.h: Connection shutdown management APIcw-pause.h: Pause buffering APIhash_offt.h: Hash table API keyed bycurl_off_tmulti_ev.h: Event-based socket tracking APIThis demonstrates good software engineering practices by properly encapsulating the new functionality.
Also applies to: 296-296, 315-315, 340-340
deps/curl/lib/multiif.h (2)
69-76: Public rename requires guarding against stale callers
Curl_multi_will_close()replaces the oldCurl_multi_closed().
Any out-of-tree code (or in-tree but missed in this PR) that still calls
the former symbol will now fail at link time. A deprecation shim (macro
or weak alias) for at least one release cycle would smooth the upgrade
path.Example:
+#define Curl_multi_closed(data,s) Curl_multi_will_close((data),(s))Would you like a patch + script to scan the repository for remaining
Curl_multi_closedreferences?
32-34: 🛠️ Refactor suggestionProvide
WARN_UNUSED_RESULT(or similar) forCurl_expire_ex
Curl_expire()is already easy to misuse because callers may ignore its
return value.
The new overloadCurl_expire_ex()is even more critical (timing bugs if
the value is dropped) yet it is not annotated withWARN_UNUSED_RESULT
as other multi-timing helpers are (e.g.Curl_update_timer).
Consider marking it so the compiler can catch silent mistakes.-void Curl_expire_ex(struct Curl_easy *data, - const struct curltime *nowp, - timediff_t milli, expire_id id); +void Curl_expire_ex(struct Curl_easy *data, + const struct curltime *nowp, + timediff_t milli, expire_id id) WARN_UNUSED_RESULT;Likely an incorrect or invalid review comment.
deps/curl/lib/sendf.c (2)
362-365: Potential const-ness mismatch in debug call
Curl_debug()expects aconst char *, andbufis alreadyconst char *, so the explicit cast that was removed is correct and the call now type-checks.
No action required.
514-522: Handy helper 👍
Curl_cwriter_is_content_decoding()is a nice addition; keeps the query out of hot paths and costs O(N) only on rare inspections.deps/curl/lib/curl_fnmatch.c (1)
322-323: Behavioural change:spacenow maps toISBLANK, notISSPACEThe previous implementation matched the full POSIX “space” class (
ISSPACE⇒ space, tab, CR, LF, VT, FF).
Replacing it withISBLANKrestricts the match to only space and tab.
This is visible to users in patterns such as[[:space:]].Please confirm that the reduced character set is intentional and aligned with the fnmatch spec you want to follow; otherwise consider reverting to
ISSPACEor adding an explicit comment explaining the reasoning.deps/curl/lib/curl_trc.h (1)
55-56: Const-correctness upgrade looks goodChanging the
ptrparameter ofCurl_debug()toconst char *removes the need for caller side casts and prevents accidental modification of the buffer that is being logged. Nice catch.deps/curl/lib/cf-https-connect.c (1)
637-646: Helper function is clear and correct
cf_https_alpns_contain()is small, self-explanatory, and allows the caller to remain agnostic of the underlying container type. 👍deps/curl/lib/pingpong.c (3)
52-79: Timeout computation is clearer and saferCaching
nowonce and delegating overall-timeout maths toCurl_timeleft()reduces drift and duplicate code; the additional clamp whendisconnecting == TRUEalso prevents negative waits during graceful shutdown. Nice refactor!
151-158: Re-initialisation guard requires all call-sites toCurl_pp_disconnect()
DEBUGASSERT(!pp->initialised);forces every code path that sets up apingpong
struct to callCurl_pp_disconnect()before attempting a secondCurl_pp_init().
Please double-check that legacy protocols (POP3/IMAP/SMTP) never reuse a
pingponginstance without an intervening disconnect, otherwise the assert will
fire in debug builds.
452-456: Safe cleanup pattern 👍Resetting the struct to zero after freeing the dynamic buffers eliminates use-after-free risks if the caller accidentally reuses the object. Good defensive coding.
deps/curl/lib/curl_setup_once.h (2)
324-357:Details
✅ Verification successful
Renamed
SOCKETIMEDOUTmacro – check downstream usagesThe new symbol drops the historical
E(SOCKETIMEDOUTvsSOCKET_ETIMEDOUT/SOCKETIMEDOUT). If any source file still references the old token compilation will break only on non-Windows builds.Please run a quick grep to confirm every occurrence was renamed:
🏁 Script executed:
grep -R --line-number -I -E 'SOCK(E|ET)_?TIMEDOUT' deps/curl | headLength of output: 453
Confirmed new
SOCKETIMEDOUTusage – no legacy symbol remainsA grep for
SOCK(E|ET)_?TIMEDOUTshows only the newSOCKETIMEDOUTmacro in:
- deps/curl/lib/curl_setup_once.h:337, 354
- deps/curl/lib/connect.c:611, 612, 773, 774
No occurrences of a historical
SOCKET_ETIMEDOUTtoken were found.
72-80:⚠️ Potential issueCURL_UNCONST: risk of pointer-size mismatch on exotic targets
The current implementation silently falls back to a plain
(void *)cast wheneverHAVE_STDINT_His not defined (even though<stdint.h>may have been included because ofUSE_WOLFSSL).
On LLP64 platforms prior to VS2010,ULONG_PTRis 32-bit in 32-bit builds but 64-bit pointers can still appear when compiling with/LARGEADDRESSAWARE, making the double cast lossy.-#elif defined(_WIN32) /* for VS2008 */ -# define CURL_UNCONST(p) ((void *)(ULONG_PTR)(const void *)(p)) +#elif defined(_MSC_VER) /* VS < 2010 */ +# include <BaseTsd.h> +# define CURL_UNCONST(p) \ + ((void *)(UINT_PTR)(const void *)(p)) /* UINT_PTR == size-t on Win */Using
UINT_PTRfromBaseTsd.h(available since VC6) guarantees a pointer-sized integer irrespective of toolset and mitigates accidental truncation.Likely an incorrect or invalid review comment.
deps/curl/lib/cfilters.h (1)
52-55:Curl_cft_connectsignature updated – update comment & callersThe
blockingparameter has been removed, but the large comment block above still mentions “blocking”. Please:
- Drop the outdated wording.
- Verify all filter implementations and helpers (
Curl_conn_connect,cf-*.c) were adjusted to the new prototype – missing changes will only be caught at link time.deps/curl/lib/hostip.c (6)
45-48: Conditional<signal.h>include looks correctThe
#ifndef UNDER_CEguard prevents compilation breakage on WinCE where<signal.h>is unavailable. Nice portability touch.
148-156: Good move to sharedcurlx_inet_ntophelpersSwitching to
curlx_inet_ntopunifies address printing across platforms and removes the duplicated wrapper logic.
654-657: Consistent helper usage acknowledgedBoth IPv4 and IPv6 literal checks now rely on
curlx_inet_pton, removing duplicated platform ifdef trees. 👍
745-757: Potential NULL‐resolver dereference whenCURLRES_SYNCH
data->set.resolver_startcallback receivesdata->state.async.resolveronly
whenCURLRES_ASYNCHis defined.
In purely synchronous builds the parameter list still passesNULL, so legacy
callers must cope with that (they previously always gotNULL).
If any in-tree or third-party callback unconditionally dereferences the
argument this becomes a regression.Please double-check external users or gate the entire callback behind
#ifdef CURLRES_ASYNCHthe same way the resolver pointer is.
1087-1089: Single cleanup call simplifies logicConsolidating HTTPSRR cleanup into one helper makes lifetime management clearer and less error-prone.
1453-1455:Curl_conn_terminatereplaces deprecated disconnect helper – OKThe new shutdown subsystem expects
Curl_conn_terminate; swap looks correct and keeps the failure path intact.deps/curl/lib/memdebug.h (2)
36-58: Clang-aware allocation attributes are welcomeAdding a dedicated
__clang__branch prevents false positives on older GCCs while still enabling
alloc_sizewhere supported. Nice maintenance.
61-65: Extra include-guard alias avoids duplicate extern warningsThe secondary guard (
HEADER_CURL_MEMDEBUG_H_EXTERNS) is a neat trick to allow
multiple inclusions whenMEMDEBUG_NODEFINESdiffers.deps/curl/lib/multi_ev.h (1)
40-48: Thoughtful API: runtime hash-size parameterAllowing the caller to pick the socket-hash size (
Curl_multi_ev_init) is a flexible design choice.
No issues spotted.deps/curl/lib/http.h (1)
77-88:http_negotiationbit-fields: implementation-defined packingUsing
BIT(x)(presumablyunsigned int :1) inside a packed struct that is
shared across translation units can provoke ABI mismatches on some compilers.If binary compatibility matters, replace with an explicit
unsigned char flags
and helper macros, or ensure the struct is never exposed outside the library
boundary.deps/curl/lib/Makefile.in (3)
226-244: Good catch adding the new source files to the distribution listsThe additions of
cshutdn.c,cw-pause.c,hash_offt.c,multi_ev.c, andstrequal.cto every _SOURCES_DIST and _la_OBJECTS stanza look complete and consistent with upstream 8.13.0.
This should prevent missing-symbol or link-order issues across all build flavours (shared, static, unity, Windows/Cygwin).
1234-1237: Conditional debug defines look correctThe migration preserves existing semantics (
-DDEBUGBUILD,-DCURLDEBUG) and cleanly scopes them behind automake conditions.
No further action required.
2700-2727:Details
✅ Verification successful
Left-over references to deleted
strtoofftimplementation?
strtoofft.chas disappeared from the build lists, but the symbol is still used in historic code paths on some platforms.
Please verify no translation unit (or public header) still callsstrtoofft().You can run this quick scan:
If the command returns results, either re-add the implementation or refactor the calls to
curlx_strtoofft()introduced upstream.Also applies to: 3957-3966
🏁 Script executed:
#!/bin/bash # Detect lingering uses of strtoofft after removal of strtoofft.c rg -n --fixed-strings "strtoofft(" deps/curl/lib | headLength of output: 57
🏁 Script executed:
#!/bin/bash # Search the entire repository for lingering calls or references to strtoofft rg -n --fixed-strings "strtoofft(" . rg -n --fixed-strings "strtoofft" .Length of output: 74
No leftover
strtoofft()references
A repository-wide search for “strtoofft” returned no occurrences. The deletedstrtoofft.cimplementation has no remaining callers in any translation unit or public header — no further action required.deps/curl/lib/imap.c (1)
818-853:⚠️ Potential issueFlags-builder always reports OOM and may continue after failure
resultis set toCURLE_OUT_OF_MEMORYbefore any attempt to add
to the dynbuf, so the function will erroneously leave with an error
even when no allocation problems occurred.
imap_sendf()is invoked unconditionally. If any of the previous
Curl_dyn_add*()calls fail, the code jumps tocleanup, keeps the
staleresultvalue, yet still transmits a half-built flags string.- Curl_dyn_init(&flags, 100); - if(data->set.upload_flags) { - int i; + Curl_dyn_init(&flags, 100); + result = CURLE_OK; /* reset */ + if(data->set.upload_flags) { + int i; ... - result = imap_sendf(data, "APPEND %s%s {%" FMT_OFF_T "}", - mailbox, Curl_dyn_ptr(&flags), data->state.infilesize); + if(!result) { + result = imap_sendf(data, "APPEND %s%s {%" FMT_OFF_T "}", + mailbox, Curl_dyn_ptr(&flags), + data->state.infilesize); + }This guarantees:
- A clean success path (result ==
CURLE_OK).- We never talk to the server if we already detected an OOM condition.
Likely an incorrect or invalid review comment.
deps/curl/lib/asyn-ares.c (1)
242-249: Behaviour change:Curl_multi_will_close()vsCurl_multi_closed()
sock_state_cb()now callsCurl_multi_will_close().
Verify that all multi-handle paths expect the early-notification
semantics or connections might be removed twice, leading to dangling
pointers.If unsure, add a defensive NULL-check inside
Curl_multi_will_close()or
re-evaluate the transition sequence.deps/curl/lib/connect.c (1)
164-181:Details
❌ Incorrect review comment
Possible truncation/overflow when storing the shutdown timeout
timeout_msis a (signed)intcoming from the caller, but the value is blind-cast to an unsigned int before being stored.
Iftimeout_msis greater thanUINT_MAX(or negative but still larger thanINT_MIN), the cast silently truncates and the effective timeout will wrap, potentially resulting in an extremely long or immediately expired shutdown timer.- data->conn->shutdown.timeout_ms = (timeout_ms >= 0) ? - (unsigned int)timeout_ms : + /* clamp to UINT_MAX to avoid wrap-around */ + data->conn->shutdown.timeout_ms = (timeout_ms >= 0) ? + (timeout_ms > (int)UINT_MAX ? UINT_MAX : (unsigned int)timeout_ms) :Alternatively store the value in an unsigned, width-agnostic type (
unsigned longoruint32_t) all the way through.
Please double-check the callers – some of them pass atimediff_t, which may be 64-bit and overflow here as well.
No truncation risk in the shutdown timeout assignment
Casting the signed
int timeout_mstounsigned inthere is safe because in C anunsigned inthas the same width and a strictly larger non-negative range than its correspondingint. Moreover, negativetimeout_msvalues take theelsebranch and are never cast, so there is no wrap-around or silent truncation. No changes required.Likely an incorrect or invalid review comment.
🧰 Tools
🪛 Cppcheck (2.10-2)
[warning] 173-173: Uninitialized variable
(uninitvar)
deps/curl/lib/config-win32.h (1)
38-49:HAVE_*macros are suppressed on Windows CE although comments claim they existThe new
#ifndef UNDER_CEguard preventsHAVE_FCNTL_H,HAVE_IO_HandHAVE_LOCALE_Hfrom being defined for Windows CE builds, yet the inline comments still state “exists on MINGW32CE”. If these headers really are available on (some) CE tool-chains, this change silently disables compile-time feature detection and can break downstream#ifdef HAVE_Xblocks.Please double-check the actual CE tool-chains in use and either
a) drop the comments, or
b) move the defines outside the guard / add a dedicated__MINGW32CE__test.deps/curl/lib/http2.c (2)
1418-1426: Return code ofnghttp2_submit_rst_stream()is ignoredIn the invalid-frame callback a RST_STREAM is submitted but the return code is not checked.
Shouldnghttp2_submit_rst_stream()fail (e.g. OOM) the library remains unaware and the connection continues in an undefined state.nghttp2_submit_rst_stream(ctx->h2, NGHTTP2_FLAG_NONE, stream->id, NGHTTP2_STREAM_CLOSED);Add error handling:
- nghttp2_submit_rst_stream(ctx->h2, NGHTTP2_FLAG_NONE, - stream->id, NGHTTP2_STREAM_CLOSED); + rv = nghttp2_submit_rst_stream(ctx->h2, NGHTTP2_FLAG_NONE, + stream->id, NGHTTP2_STREAM_CLOSED); + if(rv) { + failf(data, "nghttp2_submit_rst_stream() failed: %s(%d)", + nghttp2_strerror(rv), rv); + return NGHTTP2_ERR_CALLBACK_FAILURE; + }This prevents silently continuing after a fatal library error.
1306-1315: Potential misuse ofnghttp2_http2_strerror()
nghttp2_http2_strerror()was introduced in nghttp2 v1.45. Building against older (yet still supported) versions will fail.failf(data, "nghttp2 shuts down connection with error %d: %s", frame->goaway.error_code, nghttp2_http2_strerror(frame->goaway.error_code));Consider guarding the call with a version check or fall back to
nghttp2_strerror()to keep backward compatibility.deps/curl/include/curl/curl.h (1)
116-126: Good cross-compiler attribute detectionNice use of the feature-test macro pattern (
CURL_HAS_DECLSPEC_ATTRIBUTE) to avoid relying on__has_declspec_attributebeing universally available. This should prevent warning spam on non-Clang compilers.deps/curl/lib/http.c (1)
861-867:⚠️ Potential issueOut-of-bounds read in
authcmpwhen header is shorter than scheme
authcmp()blindly readsline[n]afterstrncasecompare(), assuming the
header is at leastn+1bytes long. When the header is exactly the same
length as the auth scheme (e.g."Basic"with no trailing space) this
accesses one byte past the end of the buffer and triggers UB.- return strncasecompare(auth, line, n) && !ISALNUM(line[n]); + return strncasecompare(auth, line, n) && + (line[n] == '\0' || !ISALNUM((unsigned char)line[n]));Casting to
unsigned charavoids undefined behaviour for negative values.Likely an incorrect or invalid review comment.
deps/curl/lib/ftp.c (3)
75-75: Header inclusion order looks good
strparse.his added at exactly the right place (immediately after the other curl-internals).
No conflicts or re-defines detected.
590-593: Nice safeguard while shutting downSkipping the update of
data->info.httpcodewhenftpc->shutdownis set
prevents confusing status lines from leaking into higher layers during
connection tear-down.
4097-4101: State reset after graceful QUIT
ftpc->shutdown = TRUEis set right before sendingQUIT, preventing the
control reply from overwriting the last application‐levelhttpcode. Good.But on successful keep-alive reuse the same
struct ftp_connmay be carried
over; make sureshutdownis cleared inftp_connect()(or zero-init via
calloc) so that subsequent transfers are not treated as in-shutdown.deps/curl/lib/multi.c (2)
212-246: Ensure admin handle is fully isolated and cleaned-up
The new internaladmineasy handle gives the event/shutdown subsystems a context – great idea – but it is still a regularCurl_easyobject that inherits defaults such as share-locks, SSL settings, cookies, etc. Because it is never exposed to the API you must guarantee:
- No callbacks (progress, debug, TLS, etc.) can ever be invoked on it – today
verbosemay get enabled viaCURL_DEBUG; any other options inherited later might surprise users.- The handle is removed from all global lists before
Curl_close()(currently done in the error-path but double-check the success path incurl_multi_cleanup()as well).multi->admin->state.internal = TRUEis helpful, but several internal helpers only checkdata->state.internalinside DEBUGBUILD branches. Consider an explicit runtime assert (e.g.DEBUGASSERT(data == multi->admin || !data->state.internal)).Would you like me to scan the tree for places where callbacks are still invoked for internal handles?
700-702: Race window when transfer is removed while socket state changes
Curl_multi_ev_xfer_done()is invoked afterCurl_detach_connection().
If the connection held the last reference to a socket, the event hash might still carry stale interest flags for that FD untilCurl_multi_will_close()runs. Consider callingCurl_multi_ev_socket_done()(or equivalent) right here whendata->connis still valid, so the FD is removed in one deterministic place and never observed again by the app’s poller.deps/curl/lib/conncache.h (1)
67-70:Details
✅ Verification successful
Signature change requires a tree-wide update
Curl_cpool_init()now expectsidata; older call-sites (e.g. tools or unit tests) will fail to compile. Ensure every invocation passes eithermulti->adminorNULL.
🏁 Script executed:
#!/bin/bash # List remaining calls with only 3 arguments ast-grep --pattern $'Curl_cpool_init($_, $_, $_)' -A0 -B0Length of output: 5043
🏁 Script executed:
#!/usr/bin/env bash # Search all C source and header files for Curl_cpool_init occurrences rg -n -t c 'Curl_cpool_init'Length of output: 358
No 3-argument calls to Curl_cpool_init remain
All existing call sites already pass the newidataparameter:
- deps/curl/lib/multi.c:245 —
Curl_cpool_init(&multi->cpool, multi->admin, NULL, chashsize)- deps/curl/lib/share.c:139 —
Curl_cpool_init(&share->cpool, share->admin, share, 103)No further changes are required.
deps/curl/lib/CMakeLists.txt (3)
70-77: InitializeIMPORT_LIB_SUFFIXto prevent name collisions
The new conditional assignment ofIMPORT_LIB_SUFFIX(defaulting to"_imp"on Windows when shared and static suffixes collide) correctly avoids import/static library naming conflicts on MSVC.
83-90: Detect and bail on import/static lib name collisions
The added fatal check ensures that ifIMPORT_LIB_SUFFIXandSTATIC_LIB_SUFFIXproduce identical filenames, the configuration will fail early with a clear message. This protects against silent build errors.
150-151: Expose interface compile defs and link dirs for static target
AddingINTERFACE_COMPILE_DEFINITIONSandINTERFACE_LINK_DIRECTORIESguarantees that downstream consumers of thelibcurlstatic target inherit the proper settings.
| int Curl_easyopts_check(void) | ||
| { | ||
| return (CURLOPT_LASTENTRY % 10000) != (326 + 1); | ||
| return (CURLOPT_LASTENTRY % 10000) != (327 + 1); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify validation check for option count.
The validation check uses (327 + 1) as the expected count, but with the addition of the new option, the total should be 328 (plus the sentinel). Consider updating this to be explicit about the current count.
🏁 Script executed:
#!/bin/bash
# Count total options in the Curl_easyopts array to verify the validation check is accurate
grep -c '^ {' deps/curl/lib/easyoptions.cLength of output: 48
Continuing verification with a precise entry count and inspecting the macro definition:
🏁 Script executed:
#!/bin/bash
set -eux
echo "1) Precise count of entries in Curl_easyopts array:"
awk '/Curl_easyopts\[]/,/};/ { if($0 ~ /^[[:space:]]*{/) c++ } END { print c }' deps/curl/lib/easyoptions.c
echo
echo "2) Context around the return validation (lines 350–400):"
sed -n '350,400p' deps/curl/lib/easyoptions.c
echo
echo "3) Locating and showing CURLOPT_LASTENTRY in headers:"
entry=$(grep -R -n "CURLOPT_LASTENTRY" deps/curl | head -1)
echo "$entry"
file=$(printf '%s' "$entry" | cut -d: -f1)
line=$(printf '%s' "$entry" | cut -d: -f2)
echo "---"
sed -n "$((line-3)),$((line+3))p" "$file"Length of output: 2930
Update CURLOPT_LASTENTRY validation to match current option count
The debug‐only Curl_easyopts_check still compares against (327 + 1), but the most recent option bump moved the last option ID to 328 (so the sentinel is now 329). Please update the literal (or better yet derive it from the table size) to keep this check in sync.
Locations:
- File: deps/curl/lib/easyoptions.c
- Function:
Curl_easyopts_check, around the return on line 381
Suggested diff:
int Curl_easyopts_check(void)
{
- return (CURLOPT_LASTENTRY % 10000) != (327 + 1);
+ /* Updated for new option: last enum is 328, sentinel is 329 */
+ return (CURLOPT_LASTENTRY % 10000) != (328 + 1);
}Optional enhancement (auto‐derive from the table):
#define EASYOPTS_COUNT (sizeof(Curl_easyopts)/sizeof(Curl_easyopts[0]) - 1)
int Curl_easyopts_check(void)
{
return (CURLOPT_LASTENTRY % 10000) != EASYOPTS_COUNT;
}| if(tr) { | ||
| size_t trlen; | ||
| result = Curl_dyn_addn(&ch->trailer, (char *)STRCONST("\x0d\x0a")); | ||
| result = Curl_dyn_addn(&ch->trailer, STRCONST("\x0d\x0a")); |
There was a problem hiding this comment.
Fix macro parameter issue in Curl_dyn_addn call.
The static analysis tool identified a potential issue with the number of parameters in this macro call.
- result = Curl_dyn_addn(&ch->trailer, STRCONST("\x0d\x0a"));
+ result = Curl_dyn_addn(&ch->trailer, "\x0d\x0a", 2);STRCONST is likely an internal macro for passing string literals with their length. Using the direct form with explicit length parameter is safer.
📝 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.
| result = Curl_dyn_addn(&ch->trailer, STRCONST("\x0d\x0a")); | |
| result = Curl_dyn_addn(&ch->trailer, "\x0d\x0a", 2); |
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 260-260: failed to expand 'Curl_dyn_addn', Wrong number of parameters for macro 'Curl_dyn_addn'.
(preprocessorErrorDirective)
| void | ||
| Curl_sec_conn_init(struct connectdata *conn) | ||
| { | ||
| Curl_dyn_init(&conn->in_buffer.buf, CURL_MAX_INPUT_LENGTH); | ||
| conn->in_buffer.index = 0; | ||
| conn->in_buffer.eof_flag = 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialiser lacks OOM feedback
Curl_dyn_init() can fail (returns CURLcode). Silently ignoring that leaves conn->in_buffer.buf unusable and makes the next access crash. Propagate the error so the caller can abort:
- Curl_dyn_init(&conn->in_buffer.buf, CURL_MAX_INPUT_LENGTH);
+ if(Curl_dyn_init(&conn->in_buffer.buf, CURL_MAX_INPUT_LENGTH))
+ /* propagate or log; returning void hides the failure */
+ return;(or change the function to return CURLcode).
Committable suggestion skipped: line range outside the PR's diff.
| const char *p = &mem[parser->item_offset]; | ||
| curl_off_t hlinks; | ||
| mem[parser->item_offset + parser->item_length - 1] = 0; | ||
| hlinks = strtol(mem + parser->item_offset, &p, 10); | ||
| if(p[0] == '\0' && hlinks != LONG_MAX && hlinks != LONG_MIN) { | ||
|
|
||
| if(!Curl_str_number(&p, &hlinks, LONG_MAX)) { | ||
| parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT; | ||
| parser->file_data->info.hardlinks = hlinks; | ||
| parser->file_data->info.hardlinks = (long)hlinks; | ||
| } | ||
| parser->item_length = 0; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid truncation when parsing hard-link counts
Curl_str_number() stores the value in a curl_off_t but it is later cast to
long, and the upper bound passed is LONG_MAX. On LLP64 platforms
(Windows/64-bit) long is 32-bit while curl_off_t is 64-bit, so a legal
value > 2³¹−1 would be parsed correctly but then truncated.
- const char *p = &mem[parser->item_offset];
- curl_off_t hlinks;
+ const char *p = mem + parser->item_offset;
+ curl_off_t hlinks;
…
- if(!Curl_str_number(&p, &hlinks, LONG_MAX)) {
- parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT;
- parser->file_data->info.hardlinks = (long)hlinks;
+ if(!Curl_str_number(&p, &hlinks, CURL_OFF_T_MAX)) {
+ if(hlinks <= CURL_OFF_T_C(0x7fffffff)) /* fits in 32-bit long? */
+ parser->file_data->info.hardlinks = (long)hlinks;
+ else
+ parser->file_data->info.hardlinks = LONG_MAX; /* or handle as error */
}If full 64-bit support is desired long-term, make hardlinks a
curl_off_t in struct curl_fileinfo.
🏁 Script executed:
#!/bin/bash
# Locate struct curl_fileinfo to inspect hardlinks field type
rg "struct curl_fileinfo" -A6 -n
# Confirm occurrences of 'hardlinks' to see its type and context
rg "hardlinks" -n
# Check for CURL_OFF_T_MAX and CURL_OFF_T_C macros in the codebase
rg "CURL_OFF_T_MAX" -n
rg "CURL_OFF_T_C" -nLength of output: 8369
🏁 Script executed:
#!/bin/bash
# Extract Curl_str_number implementation to understand overflow behavior
rg -n "Curl_str_number" -A20 -B5 deps/curl/lib/strparse.cLength of output: 1861
Clamp parsed hard-link counts to LONG_MAX to avoid truncation
On LLP64 platforms (Windows x64) long is 32-bit but curl_off_t is 64-bit. Parsing up to CURL_OFF_T_MAX and then casting to long would overflow for values > 2³¹−1. Instead, continue to parse the full range and then clamp to LONG_MAX.
• File: deps/curl/lib/ftplistparser.c
• Lines: ~547–555
- const char *p = &mem[parser->item_offset];
- curl_off_t hlinks;
+ const char *p = mem + parser->item_offset;
+ curl_off_t hlinks;
mem[parser->item_offset + parser->item_length - 1] = 0;
- if(!Curl_str_number(&p, &hlinks, LONG_MAX)) {
- parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT;
- parser->file_data->info.hardlinks = (long)hlinks;
+ if(!Curl_str_number(&p, &hlinks, CURL_OFF_T_MAX)) {
+ parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT;
+ if(hlinks <= LONG_MAX) {
+ parser->file_data->info.hardlinks = (long)hlinks;
+ }
+ else {
+ /* clamp to avoid truncation */
+ parser->file_data->info.hardlinks = LONG_MAX;
+ }
}
parser->item_length = 0;Long-term, to support true 64-bit hard-link counts you may wish to change curl_fileinfo.hardlinks from long to curl_off_t, noting the API/ABI impact.
📝 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 *p = &mem[parser->item_offset]; | |
| curl_off_t hlinks; | |
| mem[parser->item_offset + parser->item_length - 1] = 0; | |
| hlinks = strtol(mem + parser->item_offset, &p, 10); | |
| if(p[0] == '\0' && hlinks != LONG_MAX && hlinks != LONG_MIN) { | |
| if(!Curl_str_number(&p, &hlinks, LONG_MAX)) { | |
| parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT; | |
| parser->file_data->info.hardlinks = hlinks; | |
| parser->file_data->info.hardlinks = (long)hlinks; | |
| } | |
| parser->item_length = 0; | |
| const char *p = mem + parser->item_offset; | |
| curl_off_t hlinks; | |
| mem[parser->item_offset + parser->item_length - 1] = 0; | |
| if(!Curl_str_number(&p, &hlinks, CURL_OFF_T_MAX)) { | |
| parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT; | |
| if(hlinks <= LONG_MAX) { | |
| parser->file_data->info.hardlinks = (long)hlinks; | |
| } | |
| else { | |
| /* clamp to avoid truncation */ | |
| parser->file_data->info.hardlinks = LONG_MAX; | |
| } | |
| } | |
| parser->item_length = 0; |
| CURLcode result; /* CURLE_OK or error handling response */ | ||
| #ifndef HAVE_CARES_GETADDRINFO | ||
| struct curltime happy_eyeballs_dns_time; /* when this timer started, or 0 */ | ||
| #endif | ||
| #ifdef USE_HTTPSRR | ||
| struct Curl_https_rrinfo hinfo; | ||
| #endif | ||
| char hostname[1]; | ||
| char *hostname; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Memory ownership for the newly introduced hostname copy
thread_data.hostname is now present in both the threaded and ares
variants. Confirm that:
- It is
free()d when the async job is cancelled, timed-out, or completed. Curl_resolver_duphandle()duplicates the string when cloning an easy
handle; otherwise the dup’d handle might dangle.
| timediff_t highscore =- 1; | ||
| timediff_t score; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initial value prevents any timer from being armed
timediff_t next_expire_ms = 0; means the later comparison
if(ms && ms < next_expire_ms)is never true for positive ms, so EXPIRE_SHUTDOWN is never scheduled.
Initialise with -1 (or CURL_OFF_T_MAX) instead.
- timediff_t next_expire_ms = 0, ms;
+ timediff_t next_expire_ms = -1, ms;Remember to guard the final if(next_expire_ms …) with >= 0.
| size_t dest_len = strlen(dest); | ||
|
|
||
| bundle = calloc(1, sizeof(*bundle) + dest_len); | ||
| if(!bundle) | ||
| return NULL; | ||
| Curl_llist_init(&bundle->conns, NULL); | ||
| bundle->dest_len = dest_len; | ||
| memcpy(bundle->dest, dest, dest_len); | ||
| bundle->dest_len = dest_len + 1; | ||
| memcpy(bundle->dest, dest, bundle->dest_len); | ||
| return bundle; |
There was a problem hiding this comment.
Allocation is one byte short and the subsequent memcpy overflows
dest_len is strlen(dest) (no NUL).
calloc(sizeof + dest_len) therefore allocates without space for the
terminator, yet memcpy(bundle->dest, dest, dest_len + 1) copies the NUL too,
writing one byte past the chunk.
Suggested fix (pairs with the previous comment):
- size_t dest_len = strlen(dest);
+ size_t dest_len = strlen(dest) + 1; /* include NUL */
- bundle = calloc(1, sizeof(*bundle) + dest_len);
+ bundle = calloc(1, sizeof(*bundle) + dest_len); /* now large enough */
- bundle->dest_len = dest_len + 1;
- memcpy(bundle->dest, dest, bundle->dest_len);
+ bundle->dest_len = dest_len; /* already includes NUL */
+ memcpy(bundle->dest, dest, dest_len);This removes the overflow and keeps dest_len consistent everywhere.
📝 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.
| size_t dest_len = strlen(dest); | |
| bundle = calloc(1, sizeof(*bundle) + dest_len); | |
| if(!bundle) | |
| return NULL; | |
| Curl_llist_init(&bundle->conns, NULL); | |
| bundle->dest_len = dest_len; | |
| memcpy(bundle->dest, dest, dest_len); | |
| bundle->dest_len = dest_len + 1; | |
| memcpy(bundle->dest, dest, bundle->dest_len); | |
| return bundle; | |
| size_t dest_len = strlen(dest) + 1; /* include NUL */ | |
| bundle = calloc(1, sizeof(*bundle) + dest_len); /* now large enough */ | |
| if(!bundle) | |
| return NULL; | |
| Curl_llist_init(&bundle->conns, NULL); | |
| bundle->dest_len = dest_len; /* already includes NUL */ | |
| memcpy(bundle->dest, dest, dest_len); | |
| return bundle; |
| timediff_t next_expire_ms = 0, ms; | ||
| bool done; | ||
|
|
||
| if(!e) | ||
| return; | ||
|
|
||
| CURL_TRC_M(data, "[SHUTDOWN] perform on %zu connections", | ||
| Curl_llist_count(&cshutdn->list)); | ||
| while(e) { | ||
| enext = Curl_node_next(e); | ||
| conn = Curl_node_elem(e); | ||
| Curl_cshutdn_run_once(data, conn, &done); | ||
| if(done) { | ||
| Curl_node_remove(e); | ||
| Curl_cshutdn_terminate(data, conn, FALSE); | ||
| } | ||
| else { | ||
| /* idata has one timer list, but maybe more than one connection. | ||
| * Set EXPIRE_SHUTDOWN to the smallest time left for all. */ | ||
| if(!nowp) { | ||
| now = Curl_now(); | ||
| nowp = &now; | ||
| } | ||
| ms = Curl_conn_shutdown_timeleft(conn, nowp); | ||
| if(ms && ms < next_expire_ms) | ||
| next_expire_ms = ms; | ||
| } |
There was a problem hiding this comment.
Timer logic never chooses the soonest deadline
next_expire_ms starts at 0, so the ms < next_expire_ms test can never
succeed (all positive deadlines are larger). As a result, Curl_expire_ex
is only called when no shutdown timeout is pending, defeating the purpose of
the incremental shutdown loop.
- timediff_t next_expire_ms = 0, ms;
+ timediff_t next_expire_ms = -1, ms;
…
- if(ms && ms < next_expire_ms)
+ if(ms && (next_expire_ms < 0 || ms < next_expire_ms))
next_expire_ms = ms;Without this, long-running graceful shutdowns may stall indefinitely.
| Curl_cshutdn_setfds(&multi->cshutdn, multi->admin, | ||
| read_fd_set, write_fd_set, &this_max_fd); | ||
|
|
There was a problem hiding this comment.
FD_SETSIZE overflow guard
Curl_cshutdn_setfds() adds descriptors to the same fd_set after the loop that already filled it. If the shutdown manager ever tracks more than FD_SETSIZE sockets the FD_SET() macro will overflow silently and clobber memory.
Please assert the return value of Curl_cshutdn_setfds() or add:
DEBUGASSERT(this_max_fd < FD_SETSIZE);inside that helper so the issue is caught earlier.
| /* The admin handle only ever has default timeouts set. To improve the | ||
| state somewhat we clone the timeouts from each added handle so that the | ||
| admin handle always has the same timeouts as the most recently added | ||
| easy handle. */ | ||
| multi->admin->set.timeout = data->set.timeout; | ||
| multi->admin->set.server_response_timeout = | ||
| data->set.server_response_timeout; | ||
| multi->admin->set.no_signal = data->set.no_signal; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Copy all timeout-related options to the admin handle
Only three fields (timeout, server_response_timeout, no_signal) are cloned. connecttimeout, expect_100_timeout and per-transfer rate limit settings influence the installation of timers in Curl_expire_ex, therefore the admin handle may end up with stale / default values, producing wrong wake-ups or starved shutdown processing.
multi->admin->set.timeout = data->set.timeout;
+multi->admin->set.connecttimeout = data->set.connecttimeout;
+multi->admin->set.expect_100_timeout = data->set.expect_100_timeout;
multi->admin->set.server_response_timeout =
data->set.server_response_timeout;📝 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.
| /* The admin handle only ever has default timeouts set. To improve the | |
| state somewhat we clone the timeouts from each added handle so that the | |
| admin handle always has the same timeouts as the most recently added | |
| easy handle. */ | |
| multi->admin->set.timeout = data->set.timeout; | |
| multi->admin->set.server_response_timeout = | |
| data->set.server_response_timeout; | |
| multi->admin->set.no_signal = data->set.no_signal; | |
| /* The admin handle only ever has default timeouts set. To improve the | |
| state somewhat we clone the timeouts from each added handle so that the | |
| admin handle always has the same timeouts as the most recently added | |
| easy handle. */ | |
| multi->admin->set.timeout = data->set.timeout; | |
| multi->admin->set.connecttimeout = data->set.connecttimeout; | |
| multi->admin->set.expect_100_timeout = data->set.expect_100_timeout; | |
| multi->admin->set.server_response_timeout = | |
| data->set.server_response_timeout; | |
| multi->admin->set.no_signal = data->set.no_signal; |
PR-URL: #307 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
|
Landed in 0d88dde |
PR-URL: #307 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores
Documentation