Skip to content

http2: initialize req.no_body in Curl_init_do for push#21194

Closed
bagder wants to merge 3 commits intomasterfrom
bagder/h2-push
Closed

http2: initialize req.no_body in Curl_init_do for push#21194
bagder wants to merge 3 commits intomasterfrom
bagder/h2-push

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 1, 2026

No description provided.

@bagder bagder added the HTTP/2 label Apr 1, 2026
@github-actions github-actions bot added the tests label Apr 1, 2026
@testclutch
Copy link
Copy Markdown

Analysis of PR #21194 at dcdfc6a8:

Test 1620 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 4 different CI jobs (the link just goes to one of them).

Generated by Testclutch

@bagder bagder marked this pull request as ready for review April 2, 2026 08:09
@bagder bagder requested a review from Copilot April 2, 2026 08:09
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ensures that transfers created via HTTP/2 server push correctly inherit and initialize the “no body” request state when CURLOPT_NOBODY is enabled, even when the duplicated easy handle is initialized without a connection pointer.

Changes:

  • Initialize data->req.no_body from data->set.opt_no_body inside Curl_init_do().
  • Ensure HTTPREQ_HEAD is selected when no_body is set during Curl_init_do().
  • Add a unit test to verify duplicated handles preserve no_body/HTTPREQ_HEAD after Curl_init_do(dupe, NULL).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
lib/url.c Initializes req.no_body (and sets HTTPREQ_HEAD) during Curl_init_do(), covering push/dup-handle init paths.
tests/unit/unit1620.c Adds coverage to confirm a duplicated handle retains no_body + uses HTTPREQ_HEAD after init without a connection.
Comments suppressed due to low confidence (1)

lib/url.c:3521

  • req.no_body is now initialized here but it is also set in Curl_req_hard_reset() (lib/request.c:155). To avoid having to keep multiple sites in sync (and risking future drift), consider centralizing this initialization in Curl_req_start()/Curl_req_soft_reset() (or a shared helper) and have callers rely on that single place.
  data->req.no_body = data->set.opt_no_body;
  if(data->req.no_body)
    /* in HTTP lingo, no body means using the HEAD request... */
    data->state.httpreq = HTTPREQ_HEAD;


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bagder added 3 commits April 2, 2026 10:26
req.no_body was only initialized in Curl_connect, while HTTP/2 server
push adds a duplicated handle via Curl_multi_add_perform and calls
Curl_init_do with conn==NULL, never invoking Curl_connect.

Found by Codex Security
@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot bot commented Apr 2, 2026

🔒 Aisle Security Analysis

✅ We scanned this PR and did not find any security vulnerabilities.

Aisle supplements but does not replace security review.


Analyzed PR: #21194 at commit 394cfe4

Last updated on: 2026-04-02T08:39:32Z

@bagder bagder closed this in b27e828 Apr 2, 2026
@bagder bagder deleted the bagder/h2-push branch April 2, 2026 08:50
dkarpov1970 pushed a commit to dkarpov1970/curl that referenced this pull request Apr 7, 2026
req.no_body was only initialized in Curl_connect, while HTTP/2 server
push adds a duplicated handle via Curl_multi_add_perform and calls
Curl_init_do with conn==NULL, never invoking Curl_connect.

Verify it by amending test 1620

Found by Codex Security

Closes curl#21194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants