On Thu, 12 Jun 2025 11:45:46 GMT, Michael McMahon <micha...@openjdk.org> wrote:
>> There is a difference between the number of attempts and the number of >> retries. One attempt means no retry; two attempts mean one retry. Maybe the >> confusion partly comes from this sentence: >> >>> jdk.httpclient.auth.retrylimit (default: 3) >>> The number of attempts the Basic authentication filter will attempt to >>> retry a failed authentication. >> >> 1. what is a failed authentication? Is it when you receive 401/407, or is it >> when you receive 401/407 *after* having provided credential? >> 2. There are two many "attempts" in the sentence above. One of them should >> be removed. > >> Ok - so now we're getting somewhere. This lets me think that the change in >> the AuthenticationFilter is not right. The specification says: >> >> > jdk.httpclient.auth.retrylimit (default: 3) >> > The number of attempts the Basic authentication filter will attempt to >> > retry a failed authentication. >> >> When I read this, I expect that if the limit is 0, no retries, then the >> Authenticator will be called once, and if the authentication fails with >> these credentials, then the request will fail. If the limit is 1, then we >> will retry once, which means the Authenticator should be called twice, and >> so on. >> >> So maybe we should always assert that totalRequestCount == >> `Math.max(RETRY_LIMIT, 0) + 1`? > > Yeah, I think I agree with 1st para above. But, there is potential for > confusion. The number of request retries is not the same as the number of > authentication retries. The first authentication request happens during the > first request retry (the 2nd request). The first authentication retry happens > during the second request retry (the 3rd request). > > The property name seems to be clearly referring to the number of auth retries > (ie the number of times the authenticator is retried) and the number of calls > to the authenticator will be that +1. > > So, maybe, it's not a bug? After this discussion I think this isn't a bug, though the wording on the property may need clarification ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25490#discussion_r2142903528
