[KNOX-709] - HBase request URLs must not be URL encoded

CHANGES

@@ -63,6 +63,7 @@ Release Notes - Apache Knox - Version 0.9.0
     * [KNOX-706] - KnoxSSO Default IDP must not require specific URL
     * [KNOX-707] - Enter Key within KnoxSSO Default IDP Form does not Submit
     * [KNOX-708] - Wrong CSS link in KnoxAuth Application's redirecting.html
+    * [KNOX-709] - HBase request URLs must not be URL encoded
 
 ------------------------------------------------------------------------------
 Release Notes - Apache Knox - Version 0.8.0

gateway-provider-ha/src/main/java/org/apache/hadoop/gateway/ha/dispatch/DefaultHaDispatch.java

@@ -127,13 +127,4 @@ private void failoverRequest(HttpUriRequest outboundRequest, HttpServletRequest
     }
   }
 
-  private static URI getDispatchUrl(HttpServletRequest request) {
-    StringBuffer str = request.getRequestURL();
-    String query = request.getQueryString();
-    if ( query != null ) {
-      str.append('?');
-      str.append(query);
-    }
-    return URI.create(str.toString());
-  }
 }

gateway-service-hbase/pom.xml

@@ -36,6 +36,11 @@
             <artifactId>gateway-test-utils</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.easymock</groupId>
+            <artifactId>easymock</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>

gateway-service-hbase/src/main/java/org/apache/hadoop/gateway/hbase/HBaseDispatch.java

@@ -17,6 +17,11 @@
  */
 package org.apache.hadoop.gateway.hbase;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URI;
+import java.net.URLDecoder;
+import javax.servlet.http.HttpServletRequest;
+
 import org.apache.hadoop.gateway.dispatch.DefaultDispatch;
 
 /**
@@ -26,5 +31,22 @@
 @Deprecated
 public class HBaseDispatch extends DefaultDispatch {
 
+  // KNOX-709: HBase can't handle URL encoded paths.
+  public URI getDispatchUrl( HttpServletRequest request) {
+    String base = request.getRequestURI();
+    StringBuffer str = new StringBuffer();
+    try {
+      str.append( URLDecoder.decode( base, "UTF-8" ) );
+    } catch( UnsupportedEncodingException e ) {
+      str.append( base );
+    } String query = request.getQueryString();
+    if ( query != null ) {
+      str.append( '?' );
+      str.append( query );
+    }
+    URI uri = URI.create( str.toString() );
+    return uri;
+  }
+
 }
 

gateway-service-hbase/src/test/java/org/apache/hadoop/gateway/hbase/HBaseDispatchTest.java

@@ -0,0 +1,88 @@
+/**
+ * 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.apache.hadoop.gateway.hbase;
+
+import java.net.URI;
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.hadoop.gateway.dispatch.Dispatch;
+import org.apache.hadoop.test.TestUtils;
+import org.apache.hadoop.test.category.FastTests;
+import org.apache.hadoop.test.category.UnitTests;
+import org.easymock.EasyMock;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.*;
+
+@Category( { UnitTests.class, FastTests.class } )
+public class HBaseDispatchTest {
+
+  @Test( timeout = TestUtils.SHORT_TIMEOUT )
+  public void testGetDispatchUrl() throws Exception {
+    HttpServletRequest request;
+    Dispatch dispatch;
+    String path;
+    String query;
+    URI uri;
+
+    dispatch = new HBaseDispatch();
+
+    path = "http://test-host:42/test-path";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( null ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test-path" ) );
+
+    path = "http://test-host:42/test,path";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( null ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test,path" ) );
+
+    // KNOX-709: HBase request URLs must not be URL encoded
+    path = "http://test-host:42/test%2Cpath";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( null ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test,path" ) );
+
+    // KNOX-709: HBase request URLs must not be URL encoded
+    path = "http://test-host:42/test%2Cpath";
+    query = "test%26name=test%3Dvalue";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( query ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test,path?test%26name=test%3Dvalue" ) );
+
+  }
+
+}

gateway-service-webhdfs/src/main/java/org/apache/hadoop/gateway/hdfs/dispatch/WebHdfsHaDispatch.java

@@ -181,15 +181,4 @@ private void retryRequest(HttpUriRequest outboundRequest, HttpServletRequest inb
       }
    }
 
-  private static URI getDispatchUrl(HttpServletRequest request) {
-    StringBuffer str = request.getRequestURL();
-    String query = request.getQueryString();
-    if ( query != null ) {
-      str.append('?');
-      str.append(query);
-    }
-    URI url = URI.create(str.toString());
-    return url;
-  }
-
 }

gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/AbstractGatewayDispatch.java

@@ -77,6 +77,18 @@ public void setHttpClient(HttpClient client) {
     this.client = client;
   }
 
+  @Override
+  public URI getDispatchUrl(HttpServletRequest request) {
+    StringBuffer str = request.getRequestURL();
+    String query = request.getQueryString();
+    if ( query != null ) {
+      str.append('?');
+      str.append(query);
+    }
+    URI url = URI.create(str.toString());
+    return url;
+  }
+
   public void doGet( URI url, HttpServletRequest request, HttpServletResponse response )
       throws IOException, URISyntaxException {
     response.sendError( HttpServletResponse.SC_METHOD_NOT_ALLOWED );

gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/Dispatch.java

@@ -36,6 +36,8 @@ public interface Dispatch {
 
   void setHttpClient(HttpClient httpClient);
 
+  URI getDispatchUrl( HttpServletRequest request );
+
   void doGet( URI url, HttpServletRequest request, HttpServletResponse response )
       throws IOException, ServletException, URISyntaxException;
 

gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/GatewayDispatchFilter.java

@@ -112,17 +112,6 @@ protected void doFilter(HttpServletRequest request, HttpServletResponse response
     }
   }
 
-  protected static URI getDispatchUrl(HttpServletRequest request) {
-    StringBuffer str = request.getRequestURL();
-    String query = request.getQueryString();
-    if ( query != null ) {
-      str.append('?');
-      str.append(query);
-    }
-    URI url = URI.create(str.toString());
-    return url;
-  }
-
   private interface Adapter {
     public void doMethod(Dispatch dispatch, HttpServletRequest request, HttpServletResponse response)
         throws IOException, ServletException, URISyntaxException;
@@ -131,35 +120,35 @@ public void doMethod(Dispatch dispatch, HttpServletRequest request, HttpServletR
   private static class GetAdapter implements Adapter {
     public void doMethod(Dispatch dispatch, HttpServletRequest request, HttpServletResponse response)
         throws IOException, ServletException, URISyntaxException {
-      dispatch.doGet(getDispatchUrl(request), request, response);
+      dispatch.doGet( dispatch.getDispatchUrl(request), request, response);
     }
   }
 
   private static class PostAdapter implements Adapter {
     public void doMethod(Dispatch dispatch, HttpServletRequest request, HttpServletResponse response)
         throws IOException, ServletException, URISyntaxException {
-      dispatch.doPost(getDispatchUrl(request), request, response);
+      dispatch.doPost( dispatch.getDispatchUrl(request), request, response);
     }
   }
 
   private static class PutAdapter implements Adapter {
     public void doMethod(Dispatch dispatch, HttpServletRequest request, HttpServletResponse response)
         throws IOException, ServletException, URISyntaxException {
-      dispatch.doPut(getDispatchUrl(request), request, response);
+      dispatch.doPut( dispatch.getDispatchUrl(request), request, response);
     }
   }
 
   private static class DeleteAdapter implements Adapter {
     public void doMethod(Dispatch dispatch, HttpServletRequest request, HttpServletResponse response)
         throws IOException, ServletException, URISyntaxException {
-      dispatch.doDelete(getDispatchUrl(request), request, response);
+      dispatch.doDelete( dispatch.getDispatchUrl(request), request, response);
     }
   }
 
   private static class OptionsAdapter implements Adapter {
     public void doMethod(Dispatch dispatch, HttpServletRequest request, HttpServletResponse response)
         throws IOException, ServletException, URISyntaxException {
-      dispatch.doOptions(getDispatchUrl(request), request, response);
+      dispatch.doOptions( dispatch.getDispatchUrl(request), request, response);
     }
   }
 

gateway-spi/src/test/java/org/apache/hadoop/gateway/dispatch/DefaultDispatchTest.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.gateway.dispatch;
 
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.instanceOf;
@@ -39,14 +40,19 @@
 
 import org.apache.hadoop.gateway.config.GatewayConfig;
 import org.apache.hadoop.gateway.servlet.SynchronousServletOutputStreamAdapter;
+import org.apache.hadoop.test.TestUtils;
+import org.apache.hadoop.test.category.FastTests;
+import org.apache.hadoop.test.category.UnitTests;
 import org.apache.http.HttpEntity;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.impl.client.DefaultHttpClient;
 import org.apache.http.params.BasicHttpParams;
 import org.easymock.EasyMock;
 import org.easymock.IAnswer;
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 
+@Category( { UnitTests.class, FastTests.class } )
 public class DefaultDispatchTest {
 
   // Make sure Hadoop cluster topology isn't exposed to client when there is a connectivity issue.
@@ -165,4 +171,53 @@ public void testUsingDefaultBufferSize() throws URISyntaxException, IOException
     assertEquals(defaultDispatch.getReplayBufferSize(), 16);
   }
 
+  @Test( timeout = TestUtils.SHORT_TIMEOUT )
+  public void testGetDispatchUrl() throws Exception {
+    HttpServletRequest request;
+    Dispatch dispatch;
+    String path;
+    String query;
+    URI uri;
+
+    dispatch = new DefaultDispatch();
+
+    path = "http://test-host:42/test-path";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( null ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test-path" ) );
+
+    path = "http://test-host:42/test,path";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( null ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test,path" ) );
+
+    path = "http://test-host:42/test%2Cpath";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( null ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test%2Cpath" ) );
+
+    path = "http://test-host:42/test%2Cpath";
+    query = "test%26name=test%3Dvalue";
+    request = EasyMock.createNiceMock( HttpServletRequest.class );
+    EasyMock.expect( request.getRequestURI() ).andReturn( path ).anyTimes();
+    EasyMock.expect( request.getRequestURL() ).andReturn( new StringBuffer( path ) ).anyTimes();
+    EasyMock.expect( request.getQueryString() ).andReturn( query ).anyTimes();
+    EasyMock.replay( request );
+    uri = dispatch.getDispatchUrl( request );
+    assertThat( uri.toASCIIString(), is( "http://test-host:42/test%2Cpath?test%26name=test%3Dvalue" ) );
+
+  }
+
 }