Skip to content

http: clear the proxy credentials as well on port or scheme change#21304

Closed
bagder wants to merge 3 commits into
masterfrom
bagder/test2009-proxy-creds
Closed

http: clear the proxy credentials as well on port or scheme change#21304
bagder wants to merge 3 commits into
masterfrom
bagder/test2009-proxy-creds

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 13, 2026

Add test 2009 to verify switching between proxies with credentials when the switch is driven by a redirect

Reported-by: Dwij Mehta

@github-actions github-actions Bot added the tests label Apr 13, 2026
@bagder bagder marked this pull request as ready for review April 13, 2026 21:16
@testclutch

This comment was marked as resolved.

@bagder bagder force-pushed the bagder/test2009-proxy-creds branch 2 times, most recently from 938b90b to ab91040 Compare April 14, 2026 07:54
@bagder bagder requested a review from Copilot April 14, 2026 08:59
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 fixes credential handling across redirects where a scheme/port change causes libcurl to switch proxies, ensuring proxy credentials from the previous proxy do not “stick” to the next proxy. It also adds new regression tests to cover proxy switching with and without explicitly provided proxy credentials.

Changes:

  • Add Curl_reset_userpwd() helper and use it during pretransfer setup and redirect-follow auth clearing.
  • Update redirect-follow logic to reset proxy credentials when clearing auth on scheme/port changes.
  • Add new tests test2009/test2010 plus a small tweak in test795, and register new tests in tests/data/Makefile.am.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
lib/transfer.c Introduces Curl_reset_userpwd() and reuses it in Curl_pretransfer().
lib/transfer.h Exposes Curl_reset_userpwd() for internal callers (e.g., HTTP redirect handling).
lib/http.c Calls Curl_reset_userpwd() when clearing auth on redirect port/scheme changes.
tests/data/test2009 New test: ensures env-provided proxy creds don’t leak when redirect switches from http_proxy to https_proxy.
tests/data/test2010 New test: ensures --proxy-user creds still apply across a proxy switch driven by redirect.
tests/data/test795 Replaces a hard-coded base64 blob with %b64[...]b64% equivalent for IMAP AUTH PLAIN.
tests/data/Makefile.am Registers the new tests in the test case list.

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

Comment thread tests/data/test2009
Comment thread tests/data/test2010
Comment thread lib/http.c
Comment thread lib/http.c
bagder added 3 commits April 14, 2026 15:32
Add test 2009 and 2010 to verify switching between proxies with
credentials when the switch is driven by a redirect

Reported-by: Dwij Mehta

Closes #21304
@bagder bagder force-pushed the bagder/test2009-proxy-creds branch from e6589de to d6464f9 Compare April 14, 2026 13:51
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Apr 14, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 14, 2026

🤖 Augment PR Summary

Summary: This PR hardens HTTP redirect handling so proxy credentials do not leak when the effective proxy changes due to a redirect.

Changes:

  • Introduces `Curl_reset_userpwd()` and `Curl_reset_proxypwd()` helpers to restore credentials from option strings.
  • Refactors `Curl_pretransfer()` to use the new helpers instead of duplicating credential-reset logic.
  • Updates `Curl_http_follow()` to reset proxy credentials during follow processing so stale proxy auth is not reused across scheme/port changes.
  • Adds new testcases (2009–2011) covering redirects that switch between `http_proxy` and `https_proxy`, with and without explicit proxy-user options.
  • Updates the tests Makefile to include the new testcases in the suite.

Technical notes: The new tests validate that Proxy-Authorization is not sent to a different proxy after a cross-scheme redirect, while still being sent when explicitly configured via --proxy-user.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

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

Review completed. 3 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread lib/http.c
Comment thread tests/data/test2011
Comment thread lib/transfer.c
@bagder bagder closed this in 188c2f1 Apr 14, 2026
@bagder bagder deleted the bagder/test2009-proxy-creds branch April 14, 2026 14:15
dkarpov1970 pushed a commit to dkarpov1970/curl that referenced this pull request Apr 20, 2026
Add tests 2009-2011 to verify switching between proxies with credentials
when the switch is driven by a redirect

Reported-by: Dwij Mehta

Closes curl#21304
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