[HttpClient] ntlm regression on authPersistNonNTLM=false connections with reset()#64349
Merged
Merged
[HttpClient] ntlm regression on authPersistNonNTLM=false connections with reset()#64349
Conversation
…where reset no longer discards the connection.
383367b to
17b3acd
Compare
Member
|
Thank you @Dooij, and congrats for your very first PR! |
This was referenced May 27, 2026
Merged
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes a regression introduced by #64046.
#64046 moved the connection cache off the curl share handle. That makes max_host_connections actually work, which was the goal, but it also breaks auth_ntlm against servers that don't persist NTLM authentication across requests on the same TCP connection. I ran into the issue calling a service configured as authPersistNonNTLM=false.
The first request succeeds: libcurl does the Type 1/2/3 handshake and gets a 201.
The second request reuses that TCP connection, libcurl's NTLM state machine considers it already authenticated and skips the handshake, IIS treats each request independently and returns 401 with a fresh WWW-Authenticate: NTLM challenge, which isnt picked up as a new handshare by libcurl anymore.
My example:
The underlying NTLM-on-reused-connection problem predates #64046: any consumer making two or more NTLM requests against an authPersistNonNTLM=false server has been silently broken since well before v7.4.9. The way to make them work was by calling
$connection->reset()before a request. This broke due to reset() unset the share handle and the share owned the connection cache via CURL_LOCK_DATA_CONNECT. #64046 correctly removed that share to honour max_host_connections, but as a side-effect it removed the resetting of the connection needed for the NTLM requests.Fix: in CurlResponse::perform(), detect a 401 with an NTLM WWW-Authenticate arriving on a reused connection (CURLINFO_NUM_CONNECTS == 0), close the deauthenticated socket, retry on a fresh connection, and record the origin. Subsequent requests to a recorded origin set CURLOPT_FRESH_CONNECT and CURLOPT_FORBID_REUSE at setup time so they skip the pool.
Tests for the setup-time and negative cases. Cannot cover the full reused-connection because the cli-server forces connection: close, which doesnt allow me to recreate the issue in the test.
Considered alternative: making CurlClientState::reset() close NTLM-authenticated connections, which would restore the pre-#64046 side-effect for consumers that call reset() between requests, but this seemed like a sturdier fix.
My current local fix to get it working again in 7.4.9 (which I can remove again after this PR) is adding the CURLOPT_FORBID_REUSE to my framework.yml config.