Repository navigation
tool_easysrc: reset the handle between --next operations - #22890
xhon-pelushi wants to merge 1 commit into
Conversation
The curl tool creates a fresh easy handle for every operation, but the --libcurl output reuses a single handle for all of them and never resets it, so options set for one operation silently apply to the following ones and the generated program does not do what the command line did. 19d0f4a fixed this for SSL verification by re-emitting those two options after --next. Everything else still leaks. For curl -H "X-First: yes" --compressed http://host/one \ --next http://host/two the generated program sends X-First and Accept-Encoding on the second transfer as well, while the command line sends them only on the first. Confirmed by compiling the generated program and comparing what both actually put on the wire against a local server: operation 1 operation 2 curl X-First, Accept-Encoding neither generated program X-First, Accept-Encoding X-First, Accept-Encoding Emit curl_easy_reset() between operations instead, which covers the general case: every option the next operation needs is already re-emitted after the reset, because the tool writes out its options per operation. The call is deferred rather than written straight after curl_easy_perform() so that it only appears when another operation actually follows: a single-operation command generates no reset at all, and none is emitted before the trailing curl_easy_cleanup(). Existing --libcurl coverage (tests 1400-1407, 1420, 1465, 1481) is all single-operation, and 1679 checks only the SSL options, so add test 1687 for the general case. It fails without this change. Test 1679's expected output gains the same curl_easy_reset() line, since it is a --next test too. Ref: curl#22701
ab94f13 to
369e1b2
Compare
|
Pushed a fix for the CI failures: I also widened Re-ran with TLS this time, in three configurations:
|
There was a problem hiding this comment.
🟢 Approval recommended
The deferred reset correctly prevents cross-operation option leakage and is covered by focused regression tests.
Pull request overview
This PR makes multi-operation --libcurl output match curl’s per-operation handle behavior by resetting the reused easy handle between --next operations.
Changes:
- Defers
curl_easy_reset()until code for a subsequent operation is emitted. - Adds a regression test covering option isolation across
--next. - Updates the existing SSL verification fixture to include the reset.
File summaries
| File | Description |
|---|---|
src/tool_easysrc.c |
Tracks and emits a pending easy-handle reset between operations. |
tests/data/test1687 |
Adds multi-operation --libcurl regression coverage. |
tests/data/data1687.c |
Defines the expected generated source for the new test. |
tests/data/data1679.c |
Updates existing multi-operation expected output. |
tests/data/Makefile.am |
Registers and distributes the new test files. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Thanks! |
Follow-up to #22745 / 19d0f4a, which fixed the
--libcurlcross---nextleak for SSL verification by re-emittingCURLOPT_SSL_VERIFYPEERandCURLOPT_SSL_VERIFYHOSTon later operations. The handle is still never reset, so every other option still carries over.The tool creates a fresh easy handle per operation; the
--libcurlgenerator reuses one for all of them. So forthe generated program sets
CURLOPT_HTTPHEADERandCURLOPT_ACCEPT_ENCODINGfor the first operation and never clears them, and the second transfer sends both — which the command line does not.I compiled the generated program and compared what it and the real command line actually put on the wire, against a local HTTP server:
curlX-First,Accept-EncodingX-First,Accept-EncodingX-First,Accept-EncodingX-First,Accept-EncodingWith the patch the generated program's requests are identical to curl's own for both operations.
The change
Emit
curl_easy_reset()between operations rather than force-emitting more individual options. Everything the next operation needs is re-emitted after the reset already, because the generator writes its options out per operation — visible in the new test's expected output, where operation 2 re-setsCURLOPT_URL,CURLOPT_USERAGENT,CURLOPT_BUFFERSIZE,CURLOPT_MAXREDIRS,CURLOPT_TCP_KEEPALIVEandCURLOPT_VERBOSEafter the reset.The call is deferred rather than written straight after
curl_easy_perform():easysrc_perform()sets a pending flag andeasysrc_add()flushes it the next time something is appended. That way a single-operation command generates no reset at all, and none appears before the trailingcurl_easy_cleanup().CURLOPT_USERAGENTno longer leaks on master, incidentally — curl sets a user agent for every operation, so operation 2 re-emits the default. Headers,--compressedand the rest do.Testing
New
test1687for the multi-operation case: the existing--libcurltests (1400-1407, 1420, 1465, 1481) are all single-operation, and 1679 covers only the SSL options.test1687passes with the patch and fails without it.--libcurltests: 11/11 pass (1481 and 1679 skipped, "curl lacks SSL support" — my build has no TLS, unrelated to the change).tests 1 to 200: 171/171 OK, 29 skipped for features my build lacks (NTLM, crypto, IDN, SSL, TrackMemory).scripts/checksrc.pl src/tool_easysrc.cclean.Ref: #22701