square / okhttp

Square’s meticulous HTTP client for the JVM, Android, and GraalVM.
https://square.github.io/okhttp/
Apache License 2.0
45.92k stars 9.16k forks source link

Infinite request/response loop when using an Authenticator with invalid credentials #960

Closed Gawdl3y closed 9 years ago

Gawdl3y commented 10 years ago

Executing a request with an authenticator on the client that doesn't correctly authenticate with the server (invalid username/password, for example) causes the execution to go into an infinite request-response-request-response loop. Ideally, an exception (something like AuthenticationException) should be thrown if the response from the request returned by the Authenticator still has the 401 header.

swankjesse commented 10 years ago

Yup. Are you implementing com.squareup.okhttp.Authenticator or java.net.Authenticator ?

Gawdl3y commented 10 years ago

I am implementing com.squareup.okhttp.Authenticator.

swankjesse commented 10 years ago

I think we want an API like this on Response:

  public List<Response> priorResponses() { ... }

Then you can just do this in your authenticator:

  if (response.priorResponses().size() > 5) return null;
swankjesse commented 10 years ago

(You could implement this in user code by looking at priorResponse() in a loop.)

JakeWharton commented 10 years ago

I think we should still enforce the 20 redirect max we impose on 3xx requests for this to ensure we never infinite loop. On Jun 28, 2014 11:44 AM, "Jesse Wilson" notifications@github.com wrote:

(You could implement this in user code by looking at priorResponse() in a loop.

— Reply to this email directly or view it on GitHub https://github.com/square/okhttp/issues/960#issuecomment-47435195.

swankjesse commented 10 years ago

Yup. Counting auth challenges against the 20 attempt limit is a good idea.

smaspe commented 9 years ago

I do that:

        @Override
        public Request authenticate(Proxy arg0, Response response) throws IOException {
            String auth = ...;
            if (auth.equals(arg1.request().header("Authorization"))) {
                return null;
            }
            return response.request().newBuilder().header("Authorization", auth).build();
        }

Because a/ I don't want to call 20 times the server with the same authentication header if it is wrong and b/ the 20 requests limit results in a ProtocolException, instead of a 401 status.

ivnsch commented 4 years ago

Is there a way to change the 20x limit? We'd like to retry only 3 times.

Also, it seems that this is not working (or there's something wrong with my configuration?). When my authorization service returns 401 it goes into the endless loop.

hoangnguyen92dn commented 3 years ago

@JakeWharton Do we have a way to change the 20 limits as mentioned by @i-schuetz above? It seems developers always face this issue in development.

yschimke commented 3 years ago

Since the last update here we've added an explanation of how to use the API to limit requests here

https://github.com/yschimke/okhttp/blob/f9171936cac4cfc607b622951e7e65a085781a6a/okhttp/src/main/kotlin/okhttp3/Authenticator.kt#L97-L111

hoangnguyen92dn commented 3 years ago

@yschimke First of all, thanks for your response. I tried to apply that logic to reduce the maximum attempts on Auththencator, but unluckily, the code stuck in the while loop.

while ((response = response.priorResponse()) != null) {
     result++;
    }

I implemented a workaround solution by using a counter to break the loop:

class ApplicationRequestAuthenticator()  : Authenticator {

    private var retryCount = 0

    override fun authenticate(route: Route?, response: Response): Request? {
        return if (retryCount >= MAX_ATTEMPTS) {
            // Reset retry count once reached max attempts
            retryCount = 0
            null
        } else {
            retryCount++
            // Refresh token
        }
    }
}

I think it's good to have an option to change the config of retries on OkHttp from Builder, same as connectTimeout, readTimeout, writeTimeout....