From 32d4dbac8abbcf416d8cd1d269e3f1fa58bec127 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Mon, 30 Jan 2012 16:10:01 -0800 Subject: [PATCH] Issue 821:retry on close_notify SSLException --- .../BaseHttpCommandExecutorService.java | 47 ++++++++----------- .../http/IntegrationTestClientExpectTest.java | 33 +++++++++++++ 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java b/core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java index 445366da13..7a60a37219 100644 --- a/core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java +++ b/core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java @@ -18,23 +18,7 @@ */ package org.jclouds.http.internal; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.io.ByteStreams.copy; -import static org.jclouds.http.HttpUtils.checkRequestHasContentLengthOrChunkedEncoding; -import static org.jclouds.http.HttpUtils.wirePayloadIfEnabled; - -import java.io.FilterInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Future; - -import javax.annotation.Resource; -import javax.inject.Inject; -import javax.inject.Named; -import javax.net.ssl.SSLException; - +import com.google.common.io.NullOutputStream; import org.jclouds.Constants; import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpCommandExecutorService; @@ -47,10 +31,22 @@ import org.jclouds.http.IOExceptionRetryHandler; import org.jclouds.http.handlers.DelegatingErrorHandler; import org.jclouds.http.handlers.DelegatingRetryHandler; import org.jclouds.logging.Logger; -import org.jclouds.rest.AuthorizationException; import org.jclouds.util.Throwables2; -import com.google.common.io.NullOutputStream; +import javax.annotation.Resource; +import javax.inject.Inject; +import javax.inject.Named; +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.io.ByteStreams.copy; +import static org.jclouds.http.HttpUtils.checkRequestHasContentLengthOrChunkedEncoding; +import static org.jclouds.http.HttpUtils.wirePayloadIfEnabled; /** * @@ -171,18 +167,13 @@ public abstract class BaseHttpCommandExecutorService implements HttpCommandEx } } catch (Exception e) { IOException ioe = Throwables2.getFirstThrowableOfType(e, IOException.class); - if (ioe != null) { - if (ioe instanceof SSLException) { - command.setException(new AuthorizationException(e.getMessage() + " connecting to " - + command.getCurrentRequest().getRequestLine(), e)); - break; - } else if (ioRetryHandler.shouldRetryRequest(command, ioe)) { - continue; - } + if (ioe != null && ioRetryHandler.shouldRetryRequest(command, ioe)) { + continue; } command.setException(new HttpResponseException(e.getMessage() + " connecting to " - + command.getCurrentRequest().getRequestLine(), command, null, e)); + + command.getCurrentRequest().getRequestLine(), command, null, e)); break; + } finally { cleanup(nativeRequest); } diff --git a/core/src/test/java/org/jclouds/http/IntegrationTestClientExpectTest.java b/core/src/test/java/org/jclouds/http/IntegrationTestClientExpectTest.java index d584ef7f34..3e59269162 100644 --- a/core/src/test/java/org/jclouds/http/IntegrationTestClientExpectTest.java +++ b/core/src/test/java/org/jclouds/http/IntegrationTestClientExpectTest.java @@ -18,14 +18,20 @@ */ package org.jclouds.http; +import static com.google.common.base.Throwables.propagate; import static org.testng.Assert.assertEquals; import java.net.URI; +import java.util.concurrent.atomic.AtomicInteger; + +import javax.net.ssl.SSLException; import org.jclouds.rest.BaseRestClientExpectTest; import org.jclouds.rest.BaseRestClientExpectTest.RegisterContext; import org.testng.annotations.Test; +import com.google.common.base.Function; + /** * * Allows us to test a client via its side effects. @@ -36,7 +42,34 @@ import org.testng.annotations.Test; // only needed as IntegrationTestClient is not registered in rest.properties @RegisterContext(sync = IntegrationTestClient.class, async = IntegrationTestAsyncClient.class) public class IntegrationTestClientExpectTest extends BaseRestClientExpectTest { + + public void testRetryOnSSLExceptionClose() { + // keeps track of request count + final AtomicInteger counter = new AtomicInteger(0); + IntegrationTestClient client = createClient(new Function() { + @Override + public HttpResponse apply(HttpRequest input) { + // on first request, throw an SSL close_notify exception + if (counter.getAndIncrement() == 0) + throw propagate(new SSLException("Received close_notify during handshake")); + + // on other requests, just validate and return 200 + assertEquals(renderRequest(input), renderRequest(HttpRequest.builder().method("HEAD").endpoint( + URI.create("http://mock/objects/rabbit")).build())); + return HttpResponse.builder().statusCode(200).build(); + } + }); + + // try three times, first should fail quietly due to retry logic + for (int i = 0; i < 3; i++) + assertEquals(client.exists("rabbit"), true); + + // should be an extra request relating to the retry + assertEquals(counter.get(), 4); + + } + public void testWhenResponseIs2xxExistsReturnsTrue() { IntegrationTestClient client = requestSendsResponse(HttpRequest.builder().method("HEAD").endpoint(