Skip to content

curl_get_line: error out on read errors#20958

Closed
bagder wants to merge 1 commit intomasterfrom
bagder/getline-error
Closed

curl_get_line: error out on read errors#20958
bagder wants to merge 1 commit intomasterfrom
bagder/getline-error

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 17, 2026

Missing ferror handling in Curl_get_line causes infinite loops on I/O errors, leading to denial-of-service hangs for config/cache file loads.

Follow-up to 769ccb4

Pointed out by Codex Security

Missing ferror handling in Curl_get_line causes infinite loops on I/O
errors, leading to denial-of-service hangs for config/cache file loads.

Follow-up to 769ccb4

Pointed out by Codex Security
@bagder bagder requested a review from Copilot March 17, 2026 12:57
@bagder bagder marked this pull request as ready for review March 17, 2026 12:58
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

Fixes a read-error hang in Curl_get_line() by returning an error when fgets() fails due to an underlying I/O error, preventing infinite loops during config/cache file loading.

Changes:

  • Detect fgets() returning NULL with ferror() set and return CURLE_READ_ERROR.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread lib/curl_get_line.c
@vszakats
Copy link
Copy Markdown
Member

vszakats commented Mar 17, 2026

Seems to be a reboot of #20826 and may make #20873 unnecessary,
if this approach works as expected.

Would be interesting to revert #20873 and see if tests keep passing.

@testclutch
Copy link
Copy Markdown

Analysis of PR #20958 at e9f3f889:

Test ../../tests/http/test_05_errors.py::TestErrors::test_05_01_partial_1[h3] failed, but it has been 1.8% 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).

Generated by Testclutch

@bagder
Copy link
Copy Markdown
Member Author

bagder commented Mar 17, 2026

Would be interesting to revert #20873 and see if tests keep passing.

The previous problem was related to opening and reading a directory instead of a file, which probably is caught by this on most platforms but as noted before not on (Net)BSD - so we still need something like that.

@bagder bagder closed this in ae09e5b Mar 17, 2026
@bagder bagder deleted the bagder/getline-error branch March 17, 2026 15:14
dkarpov1970 pushed a commit to dkarpov1970/curl that referenced this pull request Mar 25, 2026
Missing ferror handling in Curl_get_line causes infinite loops on I/O
errors, leading to denial-of-service hangs for config/cache file loads.

Follow-up to 769ccb4

Pointed out by Codex Security

Closes curl#20958
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.

4 participants