HBASE-2906 [rest/stargate] URI decoding in RowResource

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1005175 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -567,6 +567,7 @@ Release 0.21.0 - Unreleased
    HBASE-2753  Remove sorted() methods from Result now that Gets are Scans
    HBASE-3059  TestReadWriteConsistencyControl occasionally hangs (Hairong 
    	       via Ryan)
+   HBASE-2906  [rest/stargate] URI decoding in RowResource
 
   IMPROVEMENTS
    HBASE-1760  Cleanup TODOs in HTable

src/main/java/org/apache/hadoop/hbase/rest/RowResource.java

@@ -21,7 +21,6 @@
 package org.apache.hadoop.hbase.rest;
 
 import java.io.IOException;
-import java.net.URLDecoder;
 import java.util.List;
 
 import javax.ws.rs.Consumes;
@@ -70,8 +69,11 @@ public class RowResource extends ResourceBase {
       throws IOException {
     super();
     this.tableName = table;
-    this.rowspec = new RowSpec(URLDecoder.decode(rowspec,
+    this.rowspec = new RowSpec(rowspec);
-      HConstants.UTF8_ENCODING));
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("new RowResource: table=" + this.tableName + "rowspec=" +
+        this.rowspec);
+    }
     if (versions != null) {
       this.rowspec.setMaxVersions(Integer.valueOf(versions));
     }

src/main/java/org/apache/hadoop/hbase/rest/RowSpec.java

@@ -20,6 +20,8 @@
 
 package org.apache.hadoop.hbase.rest;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
 import java.util.Collection;
 import java.util.TreeSet;
 
@@ -59,32 +61,29 @@ public class RowSpec {
 
   private int parseRowKeys(final String path, int i)
       throws IllegalArgumentException {
-    StringBuilder startRow = new StringBuilder();
+    String startRow = null, endRow = null;
-    StringBuilder endRow = null;
     try {
+      StringBuilder sb = new StringBuilder();
       char c;
-      boolean doEndRow = false;
       while (i < path.length() && (c = path.charAt(i)) != '/') {
-        if (c == ',') {
+        sb.append(c);
-          doEndRow = true;
-          i++;
-          break;
-        }
-        startRow.append(c);
         i++;
       }
       i++;
-      this.row = Bytes.toBytes(startRow.toString());
+      startRow = sb.toString();
-      if (doEndRow) {
+      int idx = startRow.indexOf(',');
-        endRow = new StringBuilder();
+      if (idx != -1) {
-        while ((c = path.charAt(i)) != '/') {
+        startRow = URLDecoder.decode(startRow.substring(0, idx),
-          endRow.append(c);
+          HConstants.UTF8_ENCODING);
-          i++;
+        endRow = URLDecoder.decode(startRow.substring(idx + 1),
-        }
+          HConstants.UTF8_ENCODING);
-        i++;
+      } else {
+        startRow = URLDecoder.decode(startRow, HConstants.UTF8_ENCODING);
       }
     } catch (IndexOutOfBoundsException e) {
       throw new IllegalArgumentException(e);
+    } catch (UnsupportedEncodingException e) {
+      throw new RuntimeException(e);
     }
     // HBase does not support wildcards on row keys so we will emulate a
     // suffix glob by synthesizing appropriate start and end row keys for
@@ -94,7 +93,7 @@ public class RowSpec {
         throw new IllegalArgumentException("invalid path: start row "+
           "specified with wildcard");
       this.row = Bytes.toBytes(startRow.substring(0, 
-                   startRow.lastIndexOf("*")));
+        startRow.lastIndexOf("*")));
       this.endRow = new byte[this.row.length + 1];
       System.arraycopy(this.row, 0, this.endRow, 0, this.row.length);
       this.endRow[this.row.length] = (byte)255;
@@ -115,37 +114,41 @@ public class RowSpec {
     try {
       char c;
       StringBuilder column = new StringBuilder();
-      boolean hasColon = false;
       while (i < path.length() && (c = path.charAt(i)) != '/') {
         if (c == ',') {
           if (column.length() < 1) {
             throw new IllegalArgumentException("invalid path");
           }
-          if (!hasColon) {
+          String s = URLDecoder.decode(column.toString(),
-            column.append(':');
+            HConstants.UTF8_ENCODING);
+          if (!s.contains(":")) {
+            this.columns.add(Bytes.toBytes(s + ":"));
+          } else {
+            this.columns.add(Bytes.toBytes(s));
           }
-          this.columns.add(Bytes.toBytes(column.toString()));
+          column.setLength(0);
-          column = new StringBuilder();
-          hasColon = false;
           i++;
           continue;
         }
-        if (c == ':') {
-          hasColon = true;
-        }
         column.append(c);
         i++;
       }
       i++;
       // trailing list entry
       if (column.length() > 1) {
-        if (!hasColon) {
+        String s = URLDecoder.decode(column.toString(),
-          column.append(':');
+          HConstants.UTF8_ENCODING);
+        if (!s.contains(":")) {
+          this.columns.add(Bytes.toBytes(s + ":"));
+        } else {
+          this.columns.add(Bytes.toBytes(s));
         }
-        this.columns.add(Bytes.toBytes(column.toString()));
       }
     } catch (IndexOutOfBoundsException e) {
       throw new IllegalArgumentException(e);
+    } catch (UnsupportedEncodingException e) {
+      // shouldn't happen
+      throw new RuntimeException(e);
     }
     return i;
   }
@@ -168,7 +171,8 @@ public class RowSpec {
         i++;
       }
       try {
-        time0 = Long.valueOf(stamp.toString());
+        time0 = Long.valueOf(URLDecoder.decode(stamp.toString(),
+          HConstants.UTF8_ENCODING));
       } catch (NumberFormatException e) {
         throw new IllegalArgumentException(e);
       }
@@ -180,7 +184,8 @@ public class RowSpec {
           i++;
         }
         try {
-          time1 = Long.valueOf(stamp.toString());
+          time1 = Long.valueOf(URLDecoder.decode(stamp.toString(),
+            HConstants.UTF8_ENCODING));
         } catch (NumberFormatException e) {
           throw new IllegalArgumentException(e);
         }
@@ -190,6 +195,9 @@ public class RowSpec {
       }
     } catch (IndexOutOfBoundsException e) {
       throw new IllegalArgumentException(e);
+    } catch (UnsupportedEncodingException e) {
+      // shouldn't happen
+      throw new RuntimeException(e);
     }
     if (time1 != 0) {
       startTime = time0;
@@ -201,32 +209,45 @@ public class RowSpec {
   }
 
   private int parseQueryParams(final String path, int i) {
-    while (i < path.length()) {
+    if (i >= path.length()) {
-      char c = path.charAt(i);
+      return i;
+    }
+    StringBuilder query = new StringBuilder();
+    try {
+      query.append(URLDecoder.decode(path.substring(i), 
+        HConstants.UTF8_ENCODING));
+    } catch (UnsupportedEncodingException e) {
+      // should not happen
+      throw new RuntimeException(e);
+    }
+    i += query.length();
+    int j = 0;
+    while (j < query.length()) {
+      char c = query.charAt(j);
       if (c != '?' && c != '&') {
         break;
       }
-      if (++i > path.length()) {
+      if (++j > query.length()) {
+        throw new IllegalArgumentException("malformed query parameter");
+      }
+      char what = query.charAt(j);
+      if (++j > query.length()) {
         break;
       }
-      char what = path.charAt(i);
+      c = query.charAt(j);
-      if (++i > path.length()) {
-        break;
-      }
-      c = path.charAt(i);
       if (c != '=') {
         throw new IllegalArgumentException("malformed query parameter");
       }
-      if (++i > path.length()) {
+      if (++j > query.length()) {
         break;
       }
       switch (what) {
       case 'm': {
         StringBuilder sb = new StringBuilder();
-        while (i <= path.length()) {
+        while (j <= query.length()) {
-          c = path.charAt(i);
+          c = query.charAt(i);
           if (c < '0' || c > '9') {
-            i--;
+            j--;
             break;
           }
           sb.append(c);
@@ -235,10 +256,10 @@ public class RowSpec {
       } break;
       case 'n': {
         StringBuilder sb = new StringBuilder();
-        while (i <= path.length()) {
+        while (j <= query.length()) {
-          c = path.charAt(i);
+          c = query.charAt(i);
           if (c < '0' || c > '9') {
-            i--;
+            j--;
             break;
           }
           sb.append(c);

src/main/java/org/apache/hadoop/hbase/rest/TableResource.java

@@ -22,6 +22,7 @@ package org.apache.hadoop.hbase.rest;
 
 import java.io.IOException;
 
+import javax.ws.rs.Encoded;
 import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.QueryParam;
@@ -62,7 +63,9 @@ public class TableResource extends ResourceBase {
 
   @Path("{rowspec: .+}")
   public RowResource getRowResource(
-      final @PathParam("rowspec") String rowspec,
+      // We need the @Encoded decorator so Jersey won't urldecode before
+      // the RowSpec constructor has a chance to parse
+      final @PathParam("rowspec") @Encoded String rowspec,
       final @QueryParam("v") String versions) throws IOException {
     return new RowResource(table, rowspec, versions);
   }

src/main/java/org/apache/hadoop/hbase/rest/client/Client.java

@@ -93,11 +93,10 @@ public class Client {
    * @param cluster the cluster definition
    * @param method the transaction method
    * @param headers HTTP header values to send
-   * @param path the path
+   * @param path the properly urlencoded path
    * @return the HTTP response code
    * @throws IOException
    */
-  @SuppressWarnings("deprecation")
   public int executePathOnly(Cluster cluster, HttpMethod method,
       Header[] headers, String path) throws IOException {
     IOException lastException;
@@ -113,7 +112,7 @@ public class Client {
         sb.append("http://");
         sb.append(cluster.lastHost);
         sb.append(path);
-        URI uri = new URI(sb.toString());
+        URI uri = new URI(sb.toString(), true);
         return executeURI(method, headers, uri.toString());
       } catch (IOException e) {
         lastException = e;
@@ -126,14 +125,13 @@ public class Client {
    * Execute a transaction method given a complete URI.
    * @param method the transaction method
    * @param headers HTTP header values to send
-   * @param uri the URI
+   * @param uri a properly urlencoded URI
    * @return the HTTP response code
    * @throws IOException
    */
-  @SuppressWarnings("deprecation")
   public int executeURI(HttpMethod method, Header[] headers, String uri)
       throws IOException {
-    method.setURI(new URI(uri));
+    method.setURI(new URI(uri, true));
     if (headers != null) {
       for (Header header: headers) {
         method.addRequestHeader(header);
@@ -156,7 +154,7 @@ public class Client {
    * @param cluster the cluster definition
    * @param method the HTTP method
    * @param headers HTTP header values to send
-   * @param path the path or URI
+   * @param path the properly urlencoded path or URI
    * @return the HTTP response code
    * @throws IOException
    */

src/test/java/org/apache/hadoop/hbase/rest/TestRowResource.java

@@ -124,7 +124,11 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     path.append(row);
     path.append('/');
     path.append(column);
-    Response response = client.get(path.toString(), MIMETYPE_XML);
+    return getValueXML(path.toString());
+  }
+
+  Response getValueXML(String url) throws IOException {
+    Response response = client.get(url, MIMETYPE_XML);
     return response;
   }
 
@@ -137,7 +141,11 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     path.append(row);
     path.append('/');
     path.append(column);
-    Response response = client.get(path.toString(), MIMETYPE_PROTOBUF); 
+    return getValuePB(path.toString());
+  }
+
+  Response getValuePB(String url) throws IOException {
+    Response response = client.get(url, MIMETYPE_PROTOBUF); 
     return response;
   }
 
@@ -150,6 +158,11 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     path.append(row);
     path.append('/');
     path.append(column);
+    return putValueXML(path.toString(), table, row, column, value);
+  }
+
+  Response putValueXML(String url, String table, String row, String column,
+      String value) throws IOException, JAXBException {
     RowModel rowModel = new RowModel(row);
     rowModel.addCell(new CellModel(Bytes.toBytes(column),
       Bytes.toBytes(value)));
@@ -157,7 +170,7 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     cellSetModel.addRow(rowModel);
     StringWriter writer = new StringWriter();
     marshaller.marshal(cellSetModel, writer);
-    Response response = client.put(path.toString(), MIMETYPE_XML,
+    Response response = client.put(url, MIMETYPE_XML,
       Bytes.toBytes(writer.toString()));
     Thread.yield();
     return response;
@@ -175,6 +188,18 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     assertEquals(Bytes.toString(cell.getValue()), value);
   }
 
+  void checkValueXML(String url, String table, String row, String column,
+      String value) throws IOException, JAXBException {
+    Response response = getValueXML(url);
+    assertEquals(response.getCode(), 200);
+    CellSetModel cellSet = (CellSetModel)
+      unmarshaller.unmarshal(new ByteArrayInputStream(response.getBody()));
+    RowModel rowModel = cellSet.getRows().get(0);
+    CellModel cell = rowModel.getCells().get(0);
+    assertEquals(Bytes.toString(cell.getColumn()), column);
+    assertEquals(Bytes.toString(cell.getValue()), value);
+  }
+
   Response putValuePB(String table, String row, String column, String value)
       throws IOException {
     StringBuilder path = new StringBuilder();
@@ -184,12 +209,17 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     path.append(row);
     path.append('/');
     path.append(column);
+    return putValuePB(path.toString(), table, row, column, value);
+  }
+
+  Response putValuePB(String url, String table, String row, String column,
+      String value) throws IOException {
     RowModel rowModel = new RowModel(row);
     rowModel.addCell(new CellModel(Bytes.toBytes(column),
       Bytes.toBytes(value)));
     CellSetModel cellSetModel = new CellSetModel();
     cellSetModel.addRow(rowModel);
-    Response response = client.put(path.toString(), MIMETYPE_PROTOBUF,
+    Response response = client.put(url, MIMETYPE_PROTOBUF,
       cellSetModel.createProtobufOutput());
     Thread.yield();
     return response;
@@ -207,6 +237,18 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     assertEquals(Bytes.toString(cell.getValue()), value);
   }
 
+  void checkValuePB(String url, String table, String row, String column,
+      String value) throws IOException {
+    Response response = getValuePB(url);
+    assertEquals(response.getCode(), 200);
+    CellSetModel cellSet = new CellSetModel();
+    cellSet.getObjectFromMessage(response.getBody());
+    RowModel rowModel = cellSet.getRows().get(0);
+    CellModel cell = rowModel.getCells().get(0);
+    assertEquals(Bytes.toString(cell.getColumn()), column);
+    assertEquals(Bytes.toString(cell.getValue()), value);
+  }
+
   void doTestDelete() throws IOException, JAXBException {
     Response response;
     
@@ -300,19 +342,23 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     assertEquals(response.getCode(), 200);
   }
 
-  void doTestURLEncodedKey() throws IOException, JAXBException {
+  public void doTestURLEncodedKey() throws IOException, JAXBException {
-    String encodedKey = URLEncoder.encode("http://www.google.com/", 
+    String urlKey = "http://example.com/foo";
-      HConstants.UTF8_ENCODING);
+    StringBuilder path = new StringBuilder();
+    path.append('/');
+    path.append(TABLE);
+    path.append('/');
+    path.append(URLEncoder.encode(urlKey, HConstants.UTF8_ENCODING));
+    path.append('/');
+    path.append(COLUMN_1);
     Response response;
-    response = putValueXML(TABLE, encodedKey, COLUMN_1, VALUE_1);
+    response = putValueXML(path.toString(), TABLE, urlKey, COLUMN_1,
+      VALUE_1);
     assertEquals(response.getCode(), 200);
-    response = putValuePB(TABLE, encodedKey, COLUMN_2, VALUE_2);
+    checkValueXML(path.toString(), TABLE, urlKey, COLUMN_1, VALUE_1);
-    assertEquals(response.getCode(), 200);
-    checkValuePB(TABLE, encodedKey, COLUMN_1, VALUE_1);
-    checkValueXML(TABLE, encodedKey, COLUMN_2, VALUE_2);
   }
 
-  public void testNoSuchCF() throws IOException, JAXBException {
+  public void doTestNoSuchCF() throws IOException, JAXBException {
     final String goodPath = "/" + TABLE + "/" + ROW_1 + "/" + CFA+":";
     final String badPath = "/" + TABLE + "/" + ROW_1 + "/" + "BAD";
     Response response = client.post(goodPath, MIMETYPE_BINARY,
@@ -404,6 +450,7 @@ public class TestRowResource extends HBaseRESTClusterTestBase {
     doTestSingleCellGetPutBinary();
     doTestSingleCellGetJSON();
     doTestURLEncodedKey();
+    doTestNoSuchCF();
     doTestMultiCellGetPutXML();
     doTestMultiCellGetPutPB();
   }