Fix a bug in query tunneling that affects URIs containing keys with e…

…ncoded reserved Rest.Li protocol 2.0.0 characters. RB=1885019 BUG=SI-13742 G=si-core-reviewers R=nmankula,fcapponi,evwillia,cxu,kbalasub A=evwillia,nmankula,kbalasub,ssheng
linkedin · Nov 27, 2019 · 3ecf9c4 · 3ecf9c4
1 parent 8913433
commit 3ecf9c4
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 14 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,10 @@
+28.0.10
+-------
+
 28.0.9
 ------
+(RB=1885019)
+Fix a bug in query tunneling affecting URIs containing keys with encoded reserved Rest.li protocol 2.0.0 characters
 
 28.0.8
 ------

diff --git a/gradle.properties b/gradle.properties
@@ -1,4 +1,4 @@
-version=28.0.8
+version=28.0.9
 sonatypeUsername=please_set_in_home_dir_if_uploading_to_maven_central
 sonatypePassword=please_set_in_home_dir_if_uploading_to_maven_central
 org.gradle.configureondemand=true

diff --git a/r2-core/src/main/java/com/linkedin/r2/message/QueryTunnelUtil.java b/r2-core/src/main/java/com/linkedin/r2/message/QueryTunnelUtil.java
@@ -145,15 +145,17 @@ private static RestRequest doEncode(final RestRequest request)
   {
 
     RestRequestBuilder requestBuilder = new RestRequestBuilder(request);
+
+    // Reconstruct URI without query. Use the URI(String) constructor to preserve any Rest.li specific encoding of the
+    // URI path keys.
     URI uri = request.getURI();
-    // reconstruct URI without query
-    URI newUri = new URI(uri.getScheme(),
-        uri.getUserInfo(),
-        uri.getHost(),
-        uri.getPort(),
-        uri.getPath(),
-        null,
-        uri.getFragment());
+    String uriString = uri.toString();
+    int queryIndex = uriString.indexOf('?');
+    if (queryIndex > 0)
+    {
+      uriString = uriString.substring(0, queryIndex);
+    }
+    URI newUri = new URI(uriString);
 
     // If there's no existing body, just pass the request as x-www-form-urlencoded
     ByteString entity = request.getEntity();

diff --git a/r2-core/src/test/java/test/r2/message/TestQueryTunnel.java b/r2-core/src/test/java/test/r2/message/TestQueryTunnel.java
@@ -480,6 +480,39 @@ public void testCaseInsensitiveHeaders(String header) throws Exception
     Assert.assertNull(decoded.getHeader(header), "Mixed case 'content-type' header failed to be decoded");
   }
 
+  @Test
+  public void testTunneledLongQueryWithRestLiSpecialEncodedCharactersInPath() throws Exception
+  {
+    // Make sure the URI path includes Rest.li special encoded characters, and create a true long query
+    StringBuilder query = new StringBuilder("q=queryString");
+    for (int i = 0; i < 10000; i++) {
+      query.append("&a=");
+      query.append(i);
+    }
+
+    // Special characters with their encoding:
+    // ',' - %2C
+    // '(' - %28
+    // ')' - %29
+    String uriPathKeyString = "/foo/(bar:biz%3Aabc%28%29,baz:xyz%3A%2C)";
+
+    RestRequest request = new RestRequestBuilder(new URI(("http://localhost:7279/" + uriPathKeyString + "?" + query.toString())))
+                                                .setMethod("GET").build();
+    RestRequest encoded = encode(request, 1);  // Set threshold to 1, so we force query tunneling
+
+    Assert.assertEquals(encoded.getMethod(), "POST");
+    Assert.assertEquals(request.getURI().getRawPath(), encoded.getURI().getRawPath());
+    Assert.assertTrue(encoded.getEntity().length() == query.length());
+    Assert.assertEquals(encoded.getHeader("Content-Type"), "application/x-www-form-urlencoded");
+    Assert.assertEquals(encoded.getHeader("Content-Length"), Integer.toString(query.length()));
+
+    RequestContext requestContext = new RequestContext();
+    RestRequest decoded = decode(encoded, requestContext);
+    Assert.assertEquals(decoded.getURI(), request.getURI());
+    Assert.assertEquals(decoded.getMethod(), "GET");
+    Assert.assertTrue((Boolean) requestContext.getLocalAttr(R2Constants.IS_QUERY_TUNNELED));
+  }
+
   private RestRequest encode(RestRequest request, int threshold) throws Exception
   {
     return encode(request, new RequestContext(), threshold);
@@ -574,8 +607,3 @@ public RestRequest getRestRequest() throws Exception
     }
   }
 }
-
-
-
-
-