On Thu, 12 Jun 2025 14:20:24 GMT, Darragh Clarke <dcla...@openjdk.org> wrote:
>>> 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 Agreed. And it's very good to have a test now to verify the expected behavior. So I would change the test to test the behaviour we want. Then double check that it fails with the current proposed changes, and pass if we revert those changes. Then I would clarify the documentation of the system property, in order to remove the confusion. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25490#discussion_r2142975052
