Skip to content

http: clear credentials better on redirect#21345

Closed
bagder wants to merge 4 commits into
masterfrom
bagder/2506
Closed

http: clear credentials better on redirect#21345
bagder wants to merge 4 commits into
masterfrom
bagder/2506

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 16, 2026

Verify with test 2506: netrc with redirect using proxy

Updated test 998 which was wrong.

Reported-by: Muhamad Arga Reksapati

Verify with test 2506: netrc with redirect using proxy

Updated test 998 which was wrong.

Reported-by: Muhamad Arga Reksapati
@github-actions github-actions Bot added the tests label Apr 16, 2026
@bagder bagder marked this pull request as ready for review April 16, 2026 15:43
@bagder bagder requested a review from Copilot April 16, 2026 15:43
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

Updates libcurl’s HTTP redirect handling to more reliably clear credentials on cross-origin redirects (including netrc-derived creds), and adds regression tests for proxy + redirect scenarios.

Changes:

  • Adjust Curl_http_follow() credential-reset logic during redirects.
  • Add new libtest + test case 2506 covering netrc credentials with redirects through an HTTP proxy.
  • Correct test998 expectations so redirected request no longer includes the original Authorization header.

Reviewed changes

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

Show a summary per file
File Description
lib/http.c Refactors redirect auth clearing using same-origin detection and resets credentials accordingly.
tests/libtest/lib2506.c Adds a new libcurl-driven test client for the new testcase.
tests/libtest/Makefile.inc Registers lib2506.c in the libtest build.
tests/data/test2506 New testcase verifying netrc auth is not sent after redirect when using a proxy.
tests/data/test998 Fixes incorrect expectation: redirected request should not carry Authorization.
tests/data/Makefile.am Registers test2506 in the test suite manifest.

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

Comment thread lib/http.c
Comment thread lib/http.c
benign otherwise but still
@testclutch
Copy link
Copy Markdown

Analysis of PR #21345 at a4c1a273:

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

Generated by Testclutch

@bagder bagder closed this in b4024bf Apr 16, 2026
@bagder bagder deleted the bagder/2506 branch April 16, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants