Updates to Http: separate md5 from etag, switch from URL to URI

git-svn-id: http://jclouds.googlecode.com/svn/trunk@1909 3d8758e0-26b5-11de-8745-db77d3ebf521

core/src/main/java/org/jclouds/http/HttpRequest.java

@@ -23,6 +23,7 @@
  */
 package org.jclouds.http;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.net.URI;
@@ -57,6 +58,8 @@ public class HttpRequest extends HttpMessage implements Request<URI> {
    public HttpRequest(String method, URI endPoint) {
       this.method = checkNotNull(method, "method");
       this.endpoint = checkNotNull(endPoint, "endPoint");
+      checkArgument(endPoint.getHost() != null, String.format("endPoint.getHost() is null for %s",
+               endPoint));
    }
 
    /**
@@ -82,7 +85,7 @@ public class HttpRequest extends HttpMessage implements Request<URI> {
             @Nullable Object entity) {
       this(method, endPoint);
       setHeaders(checkNotNull(headers, "headers"));
-      setEntity("entity");
+      setEntity(entity);
    }
 
    @Override
@@ -92,6 +95,7 @@ public class HttpRequest extends HttpMessage implements Request<URI> {
       sb.append("{endPoint='").append(endpoint).append('\'');
       sb.append(", method='").append(method).append('\'');
       sb.append(", headers=").append(headers);
+      sb.append(", filters=").append(requestFilters);
       if (entity != null && entity instanceof String) {
          sb.append(", entity=").append(entity);
       } else {
@@ -102,7 +106,9 @@ public class HttpRequest extends HttpMessage implements Request<URI> {
    }
 
    /**
-    * We cannot return an enum, as per specification custom methods are allowed.  Enums are not extensible.
+    * We cannot return an enum, as per specification custom methods are allowed. Enums are not
+    * extensible.
+    * 
     * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.1
     */
    public String getMethod() {

core/src/main/java/org/jclouds/http/HttpUtils.java

@@ -101,19 +101,19 @@ public class HttpUtils {
       return hmacBase64(toEncode, key, digest);
    }
 
-   public static String eTagHex(byte[] toEncode) throws NoSuchAlgorithmException,
+   public static String md5Hex(byte[] toEncode) throws NoSuchAlgorithmException,
             NoSuchProviderException, InvalidKeyException, UnsupportedEncodingException {
-      byte[] resBuf = eTag(toEncode);
+      byte[] resBuf = md5(toEncode);
       return toHexString(resBuf);
    }
 
-   public static String eTagBase64(byte[] toEncode) throws NoSuchAlgorithmException,
+   public static String md5Base64(byte[] toEncode) throws NoSuchAlgorithmException,
             NoSuchProviderException, InvalidKeyException {
-      byte[] resBuf = eTag(toEncode);
+      byte[] resBuf = md5(toEncode);
       return toBase64String(resBuf);
    }
 
-   public static byte[] eTag(byte[] plainBytes) {
+   public static byte[] md5(byte[] plainBytes) {
       MD5Digest eTag = new MD5Digest();
       byte[] resBuf = new byte[eTag.getDigestSize()];
       eTag.update(plainBytes, 0, plainBytes.length);
@@ -121,7 +121,7 @@ public class HttpUtils {
       return resBuf;
    }
 
-   public static byte[] eTag(File toEncode) throws IOException {
+   public static byte[] md5(File toEncode) throws IOException {
       MD5Digest eTag = new MD5Digest();
       byte[] resBuf = new byte[eTag.getDigestSize()];
       byte[] buffer = new byte[1024];
@@ -160,24 +160,24 @@ public class HttpUtils {
    /**
     * @throws IOException
     */
-   public static byte[] eTag(Object data) throws IOException {
+   public static byte[] md5(Object data) throws IOException {
       checkNotNull(data, "data must be set before calling generateETag()");
-      byte[] eTag = null;
+      byte[] md5 = null;
       if (data == null) {
       } else if (data instanceof byte[]) {
-         eTag = eTag((byte[]) data);
+         md5 = md5((byte[]) data);
       } else if (data instanceof String) {
-         eTag = eTag(((String) data).getBytes());
+         md5 = md5(((String) data).getBytes());
       } else if (data instanceof File) {
-         eTag = eTag(((File) data));
+         md5 = md5(((File) data));
       } else {
          throw new UnsupportedOperationException("Content not supported " + data.getClass());
       }
-      return eTag;
+      return md5;
 
    }
 
-   public static ETagInputStreamResult generateETagResult(InputStream toEncode) throws IOException {
+   public static MD5InputStreamResult generateMD5Result(InputStream toEncode) throws IOException {
       MD5Digest eTag = new MD5Digest();
       byte[] resBuf = new byte[eTag.getDigestSize()];
       byte[] buffer = new byte[1024];
@@ -198,15 +198,15 @@ public class HttpUtils {
          IOUtils.closeQuietly(toEncode);
       }
       eTag.doFinal(resBuf, 0);
-      return new ETagInputStreamResult(out.toByteArray(), resBuf, length);
+      return new MD5InputStreamResult(out.toByteArray(), resBuf, length);
    }
 
-   public static class ETagInputStreamResult {
+   public static class MD5InputStreamResult {
       public final byte[] data;
       public final byte[] eTag;
       public final long length;
 
-      ETagInputStreamResult(byte[] data, byte[] eTag, long length) {
+      MD5InputStreamResult(byte[] data, byte[] eTag, long length) {
          this.data = checkNotNull(data, "data");
          this.eTag = checkNotNull(eTag, "eTag");
          checkArgument(length >= 0, "length cannot me negative");

core/src/main/java/org/jclouds/http/functions/ReturnTrueOn404.java

@@ -0,0 +1,42 @@
+/**
+ *
+ * Copyright (C) 2009 Global Cloud Specialists, Inc. <info@globalcloudspecialists.com>
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+package org.jclouds.http.functions;
+
+import org.jclouds.http.HttpResponseException;
+
+import com.google.common.base.Function;
+
+public class ReturnTrueOn404 implements Function<Exception, Boolean> {
+
+   public Boolean apply(Exception from) {
+      if (from instanceof HttpResponseException) {
+         HttpResponseException responseException = (HttpResponseException) from;
+         if (responseException.getResponse().getStatusCode() == 404) {
+            return true;
+         }
+      }
+      return null;
+   }
+
+}

core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java

@@ -73,10 +73,11 @@ public abstract class BaseHttpCommandExecutorService<Q> implements HttpCommandEx
             HttpRequest request = command.getRequest();
             Q nativeRequest = null;
             try {
-               logger.trace("%s - converting request %s", request.getEndpoint(), request);
+               logger.trace("%s - filtering request %s", request.getEndpoint(), request);
                for (HttpRequestFilter filter : request.getFilters()) {
                   filter.filter(request);
                }
+               logger.trace("%s - request now %s", request.getEndpoint(), request);            
                nativeRequest = convert(request);
                response = invoke(nativeRequest);
                int statusCode = response.getStatusCode();

core/src/main/java/org/jclouds/http/internal/JavaUrlHttpCommandExecutorService.java

@@ -30,6 +30,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.net.HttpURLConnection;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
@@ -90,7 +91,12 @@ public class JavaUrlHttpCommandExecutorService extends
    protected HttpResponse invoke(HttpURLConnection connection) throws IOException {
       logger.trace("%s - submitting request %s; %s", connection.getURL().getHost(), connection
                .getURL(), connection.getHeaderFields().toString());
-      HttpResponse response = new HttpResponse(connection.getURL());
+      HttpResponse response;
+      try {
+         response = new HttpResponse(connection.getURL().toURI());
+      } catch (URISyntaxException e1) {
+         throw new RuntimeException(e1);
+      }
       InputStream in;
       try {
          in = connection.getInputStream();
@@ -128,12 +134,12 @@ public class JavaUrlHttpCommandExecutorService extends
       for (String header : request.getHeaders().keySet()) {
          for (String value : request.getHeaders().get(header)) {
             connection.setRequestProperty(header, value);
-            
+
             if ("Transfer-Encoding".equals(header) && "chunked".equals(value)) {
-               connection.setChunkedStreamingMode(8192);               
+               connection.setChunkedStreamingMode(8192);
             }
          }
-      }      
+      }
       connection.setRequestProperty(HttpHeaders.HOST, request.getEndpoint().getHost());
       if (request.getEntity() != null) {
          OutputStream out = connection.getOutputStream();

core/src/main/java/org/jclouds/http/pool/ConnectionPoolTransformingHttpCommandExecutorService.java

@@ -23,6 +23,8 @@
  */
 package org.jclouds.http.pool;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import java.net.URI;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Callable;
@@ -67,6 +69,8 @@ public class ConnectionPoolTransformingHttpCommandExecutorService<C> extends Bas
       // TODO inject this.
       poolMap = new MapMaker().makeComputingMap(new Function<URI, HttpCommandConnectionPool<C>>() {
          public HttpCommandConnectionPool<C> apply(URI endPoint) {
+            checkArgument(endPoint.getHost() != null, String.format(
+                     "endPoint.getHost() is null for %s", endPoint));
             try {
                HttpCommandConnectionPool<C> pool = poolFactory.create(endPoint);
                addDependency(pool);
@@ -159,10 +163,10 @@ public class ConnectionPoolTransformingHttpCommandExecutorService<C> extends Bas
     */
    protected void invoke(HttpCommandRendezvous<?> command) {
       exceptionIfNotActive();
-      URI endpoint = command.getCommand().getRequest().getEndpoint();
+
-      URI specificEndpoint = URI.create(endpoint.getScheme() + "://" + endpoint.getHost() + ":"
+      URI endpoint = createBaseEndpointFor(command);
-               + endpoint.getPort());
+
-      HttpCommandConnectionPool<C> pool = poolMap.get(specificEndpoint);
+      HttpCommandConnectionPool<C> pool = poolMap.get(endpoint);
       if (pool == null) {
          // TODO limit;
          logger.warn("pool not available for command %s; retrying", command);
@@ -182,6 +186,12 @@ public class ConnectionPoolTransformingHttpCommandExecutorService<C> extends Bas
                   command, pool);
          commandQueue.add(command);
          return;
+      } catch (RuntimeException e) {
+         logger.warn(e, "Error getting a connection for command %s on pool %s; retrying", command,
+                  pool);
+         discardPool(endpoint, pool);
+         commandQueue.add(command);
+         return;
       }
 
       if (connectionHandle == null) {
@@ -192,6 +202,26 @@ public class ConnectionPoolTransformingHttpCommandExecutorService<C> extends Bas
       connectionHandle.startConnection();
    }
 
+   private void discardPool(URI endpoint, HttpCommandConnectionPool<C> pool) {
+      poolMap.remove(endpoint, pool);
+      pool.shutdown();
+      this.dependencies.remove(pool);
+   }
+
+   /**
+    * keys to the map are only used for socket information, not path. In this case, you should
+    * remove any path or query details from the URI.
+    */
+   private URI createBaseEndpointFor(HttpCommandRendezvous<?> command) {
+      URI endpoint = command.getCommand().getRequest().getEndpoint();
+      if (endpoint.getPort() == -1) {
+         return URI.create(String.format("%s://%s", endpoint.getScheme(), endpoint.getHost()));
+      } else {
+         return URI.create(String.format("%s://%s:%d", endpoint.getScheme(), endpoint.getHost(),
+                  endpoint.getPort()));
+      }
+   }
+
    @Override
    public String toString() {
       final StringBuilder sb = new StringBuilder();

core/src/main/java/org/jclouds/http/pool/HttpCommandConnectionPool.java

@@ -159,7 +159,7 @@ public abstract class HttpCommandConnectionPool<C> extends BaseLifeCycle {
 
    protected abstract boolean isReplayable(HttpCommandRendezvous<?> rendezvous);
 
-   HttpCommandRendezvous<?> getCommandFromConnection(C connection) {
+   protected HttpCommandRendezvous<?> getCommandFromConnection(C connection) {
       HttpCommandConnectionHandle<C> handle = getHandleFromConnection(connection);
       if (handle != null && handle.getCommandRendezvous() != null) {
          return handle.getCommandRendezvous();
@@ -174,7 +174,7 @@ public abstract class HttpCommandConnectionPool<C> extends BaseLifeCycle {
       }
    }
 
-   private void setExceptionOnCommand(Exception e, HttpCommandRendezvous<?> rendezvous) {
+   protected void setExceptionOnCommand(Exception e, HttpCommandRendezvous<?> rendezvous) {
       logger.warn(e, "exception in rendezvous: %s", rendezvous);
       try {
          rendezvous.setException(e);

core/src/test/java/org/jclouds/concurrent/FutureExceptionParserTest.java

@@ -1,94 +0,0 @@
-/**
- *
- * Copyright (C) 2009 Global Cloud Specialists, Inc. <info@globalcloudspecialists.com>
- *
- * ====================================================================
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- * ====================================================================
- */
-package org.jclouds.concurrent;
-
-import static org.testng.Assert.assertEquals;
-
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
-
-import org.testng.annotations.Test;
-
-import com.google.common.base.Function;
-
-/**
- * Tests behavior of FutureExceptionParser
- * 
- * @author Adrian Cole
- */
-@Test(groups = "unit", testName = "concurrent.FutureExceptionParserTest")
-public class FutureExceptionParserTest {
-   ExecutorService executorService = new WithinThreadExecutorService();
-
-   @Test
-   public void testGet() throws InterruptedException, ExecutionException {
-      Future<?> future = createFuture(new RuntimeException("foo"));
-      assertEquals(future.get(), "foo");
-   }
-
-   @Test(expectedExceptions = ExecutionException.class)
-   public void testGetUnmatched() throws InterruptedException, ExecutionException {
-      Future<?> future = createFuture(new Exception("foo"));
-      assertEquals(future.get(), "foo");
-   }
-
-   @Test
-   public void testGetLongTimeUnit() throws InterruptedException, ExecutionException,
-            TimeoutException {
-      Future<?> future = createFuture(new RuntimeException("foo"));
-      assertEquals(future.get(1, TimeUnit.SECONDS), "foo");
-   }
-
-   @Test(expectedExceptions = ExecutionException.class)
-   public void testGetLongTimeUnitUnmatched() throws InterruptedException, ExecutionException,
-            TimeoutException {
-      Future<?> future = createFuture(new Exception("foo"));
-      assertEquals(future.get(1, TimeUnit.SECONDS), "foo");
-   }
-
-   @SuppressWarnings("unchecked")
-   private Future<?> createFuture(final Exception exception) {
-      Future<?> future = executorService.submit(new Callable<String>() {
-
-         public String call() throws Exception {
-            throw exception;
-         }
-
-      });
-
-      future = new FutureExceptionParser(future, new Function<Exception, String>() {
-
-         public String apply(Exception from) {
-            return (from instanceof RuntimeException) ? from.getMessage() : null;
-         }
-
-      });
-      return future;
-   }
-
-}

core/src/test/java/org/jclouds/http/BaseHttpCommandExecutorServiceTest.java

@@ -32,6 +32,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 import org.jclouds.http.options.GetOptions;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 /**
@@ -50,6 +51,8 @@ public abstract class BaseHttpCommandExecutorServiceTest extends BaseJettyTest {
       assertEquals(get.get(10, TimeUnit.SECONDS).trim(), "test");
    }
 
+   // TODO: filtering redirect test
+
    @Test(invocationCount = 50, timeOut = 5000)
    public void testGetStringWithHeader() throws MalformedURLException, ExecutionException,
             InterruptedException, TimeoutException {
@@ -64,11 +67,17 @@ public abstract class BaseHttpCommandExecutorServiceTest extends BaseJettyTest {
       assertEquals(get.get(10, TimeUnit.SECONDS).trim(), XML);
    }
 
-   @Test(invocationCount = 50, timeOut = 5000)
+   @DataProvider(name = "gets")
-   public void testGetStringSynch() throws MalformedURLException, ExecutionException,
+   public Object[][] createData() {
+      return new Object[][] { { "object" }, { "/path" }, { "sp ace" }, { "unic¿de" },
+               { "qu?stion" } };
+   }
+
+   @Test(invocationCount = 50, timeOut = 5000, dataProvider = "gets")
+   public void testGetStringSynch(String uri) throws MalformedURLException, ExecutionException,
             InterruptedException, TimeoutException {
       // TODO why need trim?
-      assertEquals(client.synch("").trim(), XML);
+      assertEquals(client.synch(uri).trim(), XML);
    }
 
    @Test(invocationCount = 50, timeOut = 5000)

core/src/test/java/org/jclouds/http/HttpRequestTest.java

@@ -0,0 +1,44 @@
+/**
+ *
+ * Copyright (C) 2009 Global Cloud Specialists, Inc. <info@globalcloudspecialists.com>
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+package org.jclouds.http;
+
+import java.net.URI;
+
+import org.testng.annotations.Test;
+
+/**
+ * Tests parsing of a request
+ * 
+ * @author Adrian Cole
+ */
+@Test(testName = "httpnio.HttpRequestTest")
+public class HttpRequestTest {
+   @Test(expectedExceptions = IllegalArgumentException.class)
+   public void testConstructorHostNull() throws Exception {
+      URI uri = URI.create("http://adriancole.blobstore1138eu.s3-external-3.amazonaws.com:-1");
+      assert uri.getHost() == null : "test requires something to produce a uri with a null hostname";
+
+      new HttpRequest("GET", uri);
+   }
+}

core/src/test/java/org/jclouds/http/IntegrationTestClient.java

@@ -61,8 +61,8 @@ public interface IntegrationTestClient {
    Future<String> download(@PathParam("id") String id);
 
    @GET
-   @Path("objects/{id}")
+   @Path("{path}")
-   String synch(@PathParam("id") String id);
+   String synch(@PathParam("path") String id);
 
    @GET
    @Path("objects/{id}")

core/src/test/java/org/jclouds/http/handlers/BackoffLimitedRetryHandlerTest.java

@@ -29,7 +29,6 @@ import static org.testng.Assert.assertTrue;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
-import java.net.URL;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
@@ -100,7 +99,7 @@ public class BackoffLimitedRetryHandlerTest {
    void testClosesInputStream() throws InterruptedException, IOException {
       HttpCommand command = createCommand();
 
-      HttpResponse response = new HttpResponse(new URL("file:///unused"));
+      HttpResponse response = new HttpResponse(URI.create("http://localhost"));
       InputStream inputStream = new InputStream() {
          boolean isOpen = true;
 
@@ -151,7 +150,7 @@ public class BackoffLimitedRetryHandlerTest {
    @Test
    void testIncrementsFailureCount() throws InterruptedException, IOException {
       HttpCommand command = createCommand();
-      HttpResponse response = new HttpResponse(new URL("file:///unused"));
+      HttpResponse response = new HttpResponse(URI.create("http://localhost"));
 
       handler.shouldRetryRequest(command, response);
       assertEquals(command.getFailureCount(), 1);
@@ -166,7 +165,7 @@ public class BackoffLimitedRetryHandlerTest {
    @Test
    void testDisallowsExcessiveRetries() throws InterruptedException, IOException {
       HttpCommand command = createCommand();
-      HttpResponse response = new HttpResponse(new URL("file:///unused"));
+      HttpResponse response = new HttpResponse(URI.create("http://localhost"));
 
       assertEquals(handler.shouldRetryRequest(command, response), true); // Failure 1
 

core/src/test/java/org/jclouds/util/HttpUtilsTest.java

@@ -117,7 +117,7 @@ public class HttpUtilsTest extends PerformanceTest {
    public void testBouncyCastleMD5Digest(String message, String base64Digest)
             throws NoSuchProviderException, NoSuchAlgorithmException, InvalidKeyException,
             UnsupportedEncodingException {
-      String b64 = HttpUtils.eTagHex(message.getBytes());
+      String b64 = HttpUtils.md5Hex(message.getBytes());
       assertEquals(base64Digest, b64);
    }
 

extensions/gae/src/main/java/org/jclouds/gae/GaeHttpCommandExecutorService.java

@@ -30,6 +30,8 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
@@ -106,8 +108,8 @@ public class GaeHttpCommandExecutorService extends BaseHttpCommandExecutorServic
    }
 
    @VisibleForTesting
-   protected HttpResponse convert(URL requestURL, HTTPResponse gaeResponse) {
+   protected HttpResponse convert(URI uri, HTTPResponse gaeResponse) {
-      HttpResponse response = new HttpResponse(requestURL);
+      HttpResponse response = new HttpResponse(uri);
       response.setStatusCode(gaeResponse.getResponseCode());
       for (HTTPHeader header : gaeResponse.getHeaders()) {
          response.getHeaders().put(header.getName(), header.getValue());
@@ -169,12 +171,16 @@ public class GaeHttpCommandExecutorService extends BaseHttpCommandExecutorServic
 
    @Override
    protected HttpResponse invoke(HTTPRequest request) throws IOException {
-      logger.trace("%1$s - submitting request %2$s, headers: %3$s", request.getURL().getHost(),
+      logger.trace("%s - submitting request %s, headers: %s", request.getURL().getHost(), request
-               request.getURL(), headersAsString(request.getHeaders()));
+               .getURL(), headersAsString(request.getHeaders()));
       HTTPResponse response = urlFetchService.fetch(request);
-      logger.info("%1$s - received response code %2$s, headers: %3$s", request.getURL().getHost(),
+      logger.trace("%s - received response code %s, headers: %s", request.getURL().getHost(),
                response.getResponseCode(), headersAsString(response.getHeaders()));
-      return convert(request.getURL(), response);
+      try {
+         return convert(request.getURL().toURI(), response);
+      } catch (URISyntaxException e) {
+         throw new RuntimeException(e);
+      }
    }
 
    String headersAsString(List<HTTPHeader> headers) {

extensions/gae/src/test/java/org/jclouds/gae/GaeHttpCommandExecutorServiceIntegrationTest.java

@@ -26,6 +26,7 @@ package org.jclouds.gae;
 import java.io.File;
 import java.net.MalformedURLException;
 import java.util.Arrays;
+import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
@@ -37,6 +38,7 @@ import org.jclouds.http.BaseHttpCommandExecutorServiceTest;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.BeforeTest;
 import org.testng.annotations.Test;
+import org.testng.v6.Maps;
 
 import com.google.appengine.tools.development.ApiProxyLocalImpl;
 import com.google.apphosting.api.ApiProxy;
@@ -93,11 +95,11 @@ public class GaeHttpCommandExecutorServiceIntegrationTest extends
    }
 
    @Override
-   @Test(invocationCount = 50, timeOut = 3000)
+   @Test(invocationCount = 50, timeOut = 3000, dataProvider = "gets")
-   public void testGetStringSynch() throws MalformedURLException, ExecutionException,
+   public void testGetStringSynch(String path) throws MalformedURLException, ExecutionException,
             InterruptedException, TimeoutException {
       setupApiProxy();
-      super.testGetStringSynch();
+      super.testGetStringSynch(path);
    }
 
    @Override
@@ -214,6 +216,10 @@ public class GaeHttpCommandExecutorServiceIntegrationTest extends
       public boolean isAdmin() {
          return false;
       }
+
+      public Map<String, Object> getAttributes() {
+         return Maps.newHashMap();
+      }
    }
 
    protected Module createClientModule() {

extensions/gae/src/test/java/org/jclouds/gae/GaeHttpCommandExecutorServiceTest.java

@@ -34,7 +34,6 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URI;
-import java.net.URL;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
@@ -88,7 +87,7 @@ public class GaeHttpCommandExecutorServiceTest {
       expect(gaeResponse.getHeaders()).andReturn(headers);
       expect(gaeResponse.getContent()).andReturn(null).atLeastOnce();
       replay(gaeResponse);
-      HttpResponse response = client.convert(new URL("file:///unused"), gaeResponse);
+      HttpResponse response = client.convert(URI.create("http://localhost"), gaeResponse);
       assertEquals(response.getStatusCode(), 200);
       assertEquals(response.getContent(), null);
       assertEquals(response.getHeaders().size(), 1);
@@ -104,7 +103,7 @@ public class GaeHttpCommandExecutorServiceTest {
       expect(gaeResponse.getHeaders()).andReturn(headers);
       expect(gaeResponse.getContent()).andReturn("hello".getBytes()).atLeastOnce();
       replay(gaeResponse);
-      HttpResponse response = client.convert(new URL("file:///unused"), gaeResponse);
+      HttpResponse response = client.convert(URI.create("http://localhost"), gaeResponse);
       assertEquals(response.getStatusCode(), 200);
       assertEquals(IOUtils.toString(response.getContent()), "hello");
       assertEquals(response.getHeaders().size(), 1);

extensions/httpnio/pom.xml

@@ -55,7 +55,7 @@
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpcore-nio</artifactId>
-            <version>4.0.1</version>
+            <version>4.1-alpha1</version>
         </dependency>
     </dependencies>
 </project>

extensions/httpnio/src/main/java/org/jclouds/http/httpnio/pool/NioHttpCommandConnectionPool.java

@@ -23,9 +23,13 @@
  */
 package org.jclouds.http.httpnio.pool;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.URI;
+import java.nio.charset.UnmappableCharacterException;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Semaphore;
@@ -53,6 +57,7 @@ import org.jclouds.http.pool.HttpCommandConnectionHandle;
 import org.jclouds.http.pool.HttpCommandConnectionPool;
 import org.jclouds.http.pool.PoolConstants;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 import com.google.inject.name.Named;
@@ -84,12 +89,24 @@ public class NioHttpCommandConnectionPool extends HttpCommandConnectionPool<NHtt
             @Named(PoolConstants.PROPERTY_POOL_MAX_SESSION_FAILURES) int maxSessionFailures,
             @Assisted URI endPoint) throws Exception {
       super(executor, allConnections, commandQueue, maxConnectionReuse, available, endPoint);
+      String host = checkNotNull(checkNotNull(endPoint, "endPoint").getHost(), String.format(
+               "Host null for endpoint %s", endPoint));
+      int port = endPoint.getPort();
+      if (endPoint.getScheme().equals("https")) {
+         this.dispatch = provideSSLClientEventDispatch(clientHandler, params);
+         if (port == -1)
+            port = 443;
+      } else {
+         this.dispatch = provideClientEventDispatch(clientHandler, params);
+         if (port == -1)
+            port = 80;
+      }
+      checkArgument(port > 0, String.format("Port %d not in range for endpoint %s", endPoint
+               .getPort(), endPoint));
       this.ioReactor = ioReactor;
-      this.dispatch = endPoint.getScheme().equals("https") ? provideSSLClientEventDispatch(
-               clientHandler, params) : provideClientEventDispatch(clientHandler, params);
       this.maxSessionFailures = maxSessionFailures;
       this.sessionCallback = new NHttpClientConnectionPoolSessionRequestCallback();
-      this.target = new InetSocketAddress(endPoint.getHost(), endPoint.getPort());
+      this.target = new InetSocketAddress(host, port);
       clientHandler.setEventListener(this);
    }
 
@@ -171,8 +188,8 @@ public class NioHttpCommandConnectionPool extends HttpCommandConnectionPool<NHtt
       boolean acquired = allConnections.tryAcquire(1, TimeUnit.SECONDS);
       if (acquired) {
          if (shouldDoWork()) {
-            logger.debug("%1$s - opening new connection", target);
+            logger.debug("%1$s - opening new connection", getTarget());
-            ioReactor.connect(target, null, null, sessionCallback);
+            ioReactor.connect(getTarget(), null, null, sessionCallback);
          } else {
             allConnections.release();
          }
@@ -187,8 +204,8 @@ public class NioHttpCommandConnectionPool extends HttpCommandConnectionPool<NHtt
 
    @Override
    protected NioHttpCommandConnectionHandle getHandleFromConnection(NHttpConnection connection) {
-      return (NioHttpCommandConnectionHandle) connection.getContext().getAttribute(
+      return (NioHttpCommandConnectionHandle) connection.getContext()
-               "command-handle");
+               .getAttribute("command-handle");
    }
 
    class NHttpClientConnectionPoolSessionRequestCallback implements SessionRequestCallback {
@@ -234,7 +251,7 @@ public class NioHttpCommandConnectionPool extends HttpCommandConnectionPool<NHtt
             logger.error(request.getException(),
                      "%1$s->%2$s[%3$s] - SessionRequest failures: %4$s, Disabling pool for %5$s",
                      request.getLocalAddress(), request.getRemoteAddress(), maxSessionFailures,
-                     target);
+                     getTarget());
             exception.set(request.getException());
          }
 
@@ -266,12 +283,23 @@ public class NioHttpCommandConnectionPool extends HttpCommandConnectionPool<NHtt
    }
 
    public void fatalIOException(IOException ex, NHttpConnection conn) {
-      logger.error(ex, "%3$s-%1$s{%2$d} - io error", conn, conn.hashCode(), target);
+      logger.error(ex, "%3$s-%1$s{%2$d} - io error", conn, conn.hashCode(), getTarget());
-      resubmitIfRequestIsReplayable(conn, ex);
+      HttpCommandRendezvous<?> rendezvous = getCommandFromConnection(conn);
+      if (rendezvous != null) {
+         /**
+          * these exceptions, while technically i/o are unresolvable. set the error on the command
+          * itself so that it doesn't replay.
+          */
+         if (ex instanceof UnmappableCharacterException) {
+            setExceptionOnCommand(ex, rendezvous);
+         } else {
+            resubmitIfRequestIsReplayable(conn, ex);
+         }
+      }
    }
 
    public void fatalProtocolException(HttpException ex, NHttpConnection conn) {
-      logger.error(ex, "%3$s-%1$s{%2$d} - http error", conn, conn.hashCode(), target);
+      logger.error(ex, "%3$s-%1$s{%2$d} - http error", conn, conn.hashCode(), getTarget());
       setExceptionOnCommand(conn, ex);
    }
 
@@ -279,8 +307,8 @@ public class NioHttpCommandConnectionPool extends HttpCommandConnectionPool<NHtt
    protected NioHttpCommandConnectionHandle createHandle(HttpCommandRendezvous<?> command,
             NHttpConnection conn) {
       try {
-         return new NioHttpCommandConnectionHandle(allConnections, available, endPoint,
+         return new NioHttpCommandConnectionHandle(allConnections, available, endPoint, command,
-                  command, conn);
+                  conn);
       } catch (InterruptedException e) {
          throw new RuntimeException("Interrupted creating a handle to " + conn, e);
       }
@@ -291,4 +319,9 @@ public class NioHttpCommandConnectionPool extends HttpCommandConnectionPool<NHtt
       return rendezvous.getCommand().isReplayable();
    }
 
+   @VisibleForTesting
+   InetSocketAddress getTarget() {
+      return target;
+   }
+
 }

extensions/httpnio/src/main/java/org/jclouds/http/httpnio/pool/NioHttpCommandExecutionHandler.java

@@ -107,7 +107,7 @@ public class NioHttpCommandExecutionHandler implements NHttpRequestExecutionHand
             HttpCommandRendezvous<?> rendezvous = handle.getCommandRendezvous();
             HttpCommand command = rendezvous.getCommand();
             org.jclouds.http.HttpResponse response = NioHttpUtils.convertToJavaCloudsResponse(
-                     command.getRequest().getEndpoint().toURL(), apacheResponse);
+                     command.getRequest().getEndpoint(), apacheResponse);
             int statusCode = response.getStatusCode();
             // TODO determine how to get the original request here so we don't need to build each
             // time

extensions/httpnio/src/main/java/org/jclouds/http/httpnio/util/NioHttpUtils.java

@@ -27,7 +27,7 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
-import java.net.URL;
+import java.net.URI;
 
 import javax.ws.rs.core.HttpHeaders;
 
@@ -45,9 +45,11 @@ import org.jclouds.http.HttpResponse;
 public class NioHttpUtils {
    public static HttpEntityEnclosingRequest convertToApacheRequest(HttpRequest request) {
 
-      String path = request.getEndpoint().getPath() + request.getEndpoint().getQuery();
+      String path = request.getEndpoint().getRawPath();
+      if (request.getEndpoint().getQuery() != null)
+         path += "?" + request.getEndpoint().getQuery();
       BasicHttpEntityEnclosingRequest apacheRequest = new BasicHttpEntityEnclosingRequest(request
-               .getMethod().toString(), path, HttpVersion.HTTP_1_1);
+               .getMethod(), path, HttpVersion.HTTP_1_1);
 
       Object content = request.getEntity();
 
@@ -103,9 +105,9 @@ public class NioHttpUtils {
       }
    }
 
-   public static HttpResponse convertToJavaCloudsResponse(URL requestURL,
+   public static HttpResponse convertToJavaCloudsResponse(URI uri,
             org.apache.http.HttpResponse apacheResponse) throws IOException {
-      HttpResponse response = new HttpResponse(requestURL);
+      HttpResponse response = new HttpResponse(uri);
       if (apacheResponse.getEntity() != null) {
          response.setContent(apacheResponse.getEntity().getContent());
       }

extensions/httpnio/src/test/java/org/jclouds/http/httpnio/pool/NioHttpCommandConnectionPoolTest.java

@@ -0,0 +1,88 @@
+/**
+ *
+ * Copyright (C) 2009 Global Cloud Specialists, Inc. <info@globalcloudspecialists.com>
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+package org.jclouds.http.httpnio.pool;
+
+import static org.easymock.classextension.EasyMock.createNiceMock;
+import static org.testng.Assert.assertEquals;
+
+import java.net.InetSocketAddress;
+import java.net.URI;
+
+import org.apache.http.nio.protocol.AsyncNHttpClientHandler;
+import org.apache.http.params.HttpParams;
+import org.testng.annotations.Test;
+
+/**
+ * Tests parsing of nio
+ * 
+ * @author Adrian Cole
+ */
+@Test(testName = "httpnio.NioHttpCommandConnectionPool")
+public class NioHttpCommandConnectionPoolTest {
+
+   public void testConstructorGoodPort() throws Exception {
+      NioHttpCommandConnectionPool pool = new NioHttpCommandConnectionPool(null, null, null, null,
+               createNiceMock(AsyncNHttpClientHandler.class), null,
+               createNiceMock(HttpParams.class), 0, 0, URI.create("http://localhost:80"));
+      assertEquals(pool.getTarget(), new InetSocketAddress("localhost", 80));
+   }
+
+   public void testConstructorGoodSSLPort() throws Exception {
+      NioHttpCommandConnectionPool pool = new NioHttpCommandConnectionPool(null, null, null, null,
+               createNiceMock(AsyncNHttpClientHandler.class), null,
+               createNiceMock(HttpParams.class), 0, 0, URI.create("https://localhost:443"));
+      assertEquals(pool.getTarget(), new InetSocketAddress("localhost", 443));
+   }
+
+   public void testConstructorUnspecifiedPort() throws Exception {
+      NioHttpCommandConnectionPool pool = new NioHttpCommandConnectionPool(null, null, null, null,
+               createNiceMock(AsyncNHttpClientHandler.class), null,
+               createNiceMock(HttpParams.class), 0, 0, URI.create("http://localhost"));
+      assertEquals(pool.getTarget(), new InetSocketAddress("localhost", 80));
+   }
+
+   public void testConstructorUnspecifiedSSLPort() throws Exception {
+      NioHttpCommandConnectionPool pool = new NioHttpCommandConnectionPool(null, null, null, null,
+               createNiceMock(AsyncNHttpClientHandler.class), null,
+               createNiceMock(HttpParams.class), 0, 0, URI.create("https://localhost"));
+      assertEquals(pool.getTarget(), new InetSocketAddress("localhost", 443));
+   }
+
+   @Test(expectedExceptions = NullPointerException.class)
+   public void testConstructorNullURI() throws Exception {
+      new NioHttpCommandConnectionPool(null, null, null, null,
+               createNiceMock(AsyncNHttpClientHandler.class), null,
+               createNiceMock(HttpParams.class), 0, 0, null);
+   }
+
+   public void testConstructorWeirdName() throws Exception {
+      NioHttpCommandConnectionPool pool = new NioHttpCommandConnectionPool(null, null, null, null,
+               createNiceMock(AsyncNHttpClientHandler.class), null,
+               createNiceMock(HttpParams.class), 0, 0, URI
+                        .create("http://adriancole.blobstore1138eu.s3-external-3.amazonaws.com"));
+      assertEquals(pool.getTarget(), new InetSocketAddress(
+               "adriancole.blobstore1138eu.s3-external-3.amazonaws.com", 80));
+   }
+
+}

extensions/httpnio/src/test/java/org/jclouds/http/httpnio/util/NioHttpUtilsTest.java

@@ -0,0 +1,71 @@
+/**
+ *
+ * Copyright (C) 2009 Global Cloud Specialists, Inc. <info@globalcloudspecialists.com>
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+package org.jclouds.http.httpnio.util;
+
+import static org.testng.Assert.assertEquals;
+
+import java.net.URI;
+
+import org.apache.http.HttpEntityEnclosingRequest;
+import org.jclouds.http.HttpRequest;
+import org.mortbay.jetty.HttpMethods;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableMultimap;
+
+/**
+ * Tests parsing of nio
+ * 
+ * @author Adrian Cole
+ */
+@Test(testName = "httpnio.NioHttpUtilsTest")
+public class NioHttpUtilsTest {
+   @DataProvider(name = "gets")
+   public Object[][] createData() {
+      return new Object[][] { { "object" }, { "/path" }, { "sp%20ace" }, { "unic¿de" },
+               { "qu?stion" } };
+   }
+
+   @Test(dataProvider = "gets")
+   public void testConvert(String uri) {
+      HttpEntityEnclosingRequest apacheRequest = NioHttpUtils
+               .convertToApacheRequest(new HttpRequest(HttpMethods.GET, URI
+                        .create("https://s3.amazonaws.com:443/" + uri), ImmutableMultimap.of(
+                        "Host", "s3.amazonaws.com")));
+      assertEquals(apacheRequest.getHeaders("Host")[0].getValue(), "s3.amazonaws.com");
+      assertEquals(apacheRequest.getRequestLine().getMethod(), "GET");
+      assertEquals(apacheRequest.getRequestLine().getUri(), "/" + uri);
+   }
+
+   public void testConvertWithQuery() {
+      HttpEntityEnclosingRequest apacheRequest = NioHttpUtils
+               .convertToApacheRequest(new HttpRequest(HttpMethods.GET, URI
+                        .create("https://s3.amazonaws.com:443/?max-keys=0"), ImmutableMultimap.of(
+                        "Host", "s3.amazonaws.com")));
+      assertEquals(apacheRequest.getHeaders("Host")[0].getValue(), "s3.amazonaws.com");
+      assertEquals(apacheRequest.getRequestLine().getMethod(), "GET");
+      assertEquals(apacheRequest.getRequestLine().getUri(), "/?max-keys=0");
+   }
+}