Skip to content

pingpong: add a max loop counter for Curl_pp_readresp - #22932

Closed
bagder wants to merge 3 commits into
masterfrom
bagder/pingpong-maxloop
Closed

bagder wants to merge 3 commits into
masterfrom
bagder/pingpong-maxloop

Conversation

@bagder

@bagder bagder commented Sep 13, 2026

Copy link
Copy Markdown
Member

To make sure it cannot get stuck in this function.

Reported-by: Joshua Rogers

@bagder
bagder marked this pull request as ready for review September 13, 2026 14:29
@bagder
bagder requested a lite review from Copilot September 13, 2026 14:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The loop-limit check can discard the final chunk and incorrectly convert read errors into success.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a ten-iteration read-loop limit to Curl_pp_readresp to prevent excessive looping while reading responses.

Changes:

  • Adds a bounded response-read loop.
  • Applies the limit during response processing.
File summaries
File Summary
lib/pingpong.c Adds the response-read loop counter.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/pingpong.c Outdated
To make sure it cannot get stuck in this function.

Reported-by: Joshua Rogers
@bagder
bagder force-pushed the bagder/pingpong-maxloop branch from a90dce0 to a0d4bd6 Compare September 14, 2026 07:25
@testclutch

Copy link
Copy Markdown

Analysis of PR #22932 at a0d4bd60:

Test 3232 failed, but it has been 94.4% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (3, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The loop guard can skip the tenth read and return before the response is fully processed.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread lib/pingpong.c Outdated
ok, post increment

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@bagder bagder closed this in 753735a Sep 14, 2026
@bagder
bagder deleted the bagder/pingpong-maxloop branch September 25, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants