Surprising param encoding fix jooby-project#900

jknack · jknack · commit cb72e7350fdf · 2017-10-20T09:55:39.000-03:00
diff --git a/jooby/src/main/java/org/jooby/Router.java b/jooby/src/main/java/org/jooby/Router.java
@@ -203,13 +203,17 @@
  */
 package org.jooby;
 
+import java.net.URLDecoder;
 import java.nio.file.Path;
 import java.util.Optional;
 import java.util.concurrent.Executor;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 
+import com.google.common.escape.Escaper;
+import com.google.common.net.PercentEscaper;
 import org.jooby.Route.Mapper;
+import org.jooby.funzy.Try;
 import org.jooby.handlers.AssetHandler;
 
 import javax.annotation.Nonnull;
@@ -222,6 +226,16 @@
  */
 public interface Router {
 
+  /**
+   * Decode a path by delegating to {@link URLDecoder#decode(String, String)}. This function keeps
+   * the <code>+</code> character.
+   *
+   * @param path Path to decoded.
+   */
+  static String decode(String path) {
+    return Try.apply(() -> URLDecoder.decode(path.replace("+", "%2b"), "UTF-8")).get();
+  }
+
   /**
    * Import content from provide application (routes, parsers/renderers, start/stop callbacks, ...
    * etc.).
diff --git a/modules/coverage-report/src/test/java/org/jooby/PathVarMustBeDecodedFeature.java b/modules/coverage-report/src/test/java/org/jooby/PathVarMustBeDecodedFeature.java
@@ -17,6 +17,6 @@ public void pathVarShouldBeDecoded() throws Exception {
 
     request()
         .get("/plus+plus")
-        .expect("plus plus");
+        .expect("plus+plus");
   }
 }
diff --git a/modules/coverage-report/src/test/java/org/jooby/issues/Issue900.java b/modules/coverage-report/src/test/java/org/jooby/issues/Issue900.java
@@ -0,0 +1,24 @@
+package org.jooby.issues;
+
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+import java.net.URLEncoder;
+
+public class Issue900 extends ServerFeature {
+
+  {
+    caseSensitiveRouting(false);
+
+    get("/900/:code", req -> req.param("code").value());
+  }
+
+  @Test
+  public void shouldNotEncodingPlusSign() throws Exception {
+    int ch = '+';
+    System.out.println(Integer.toHexString(ch));
+    request().get("/900/a+b")
+        .expect("a+b");
+  }
+
+}
diff --git a/modules/jooby-netty/src/main/java/org/jooby/internal/netty/NettyRequest.java b/modules/jooby-netty/src/main/java/org/jooby/internal/netty/NettyRequest.java
@@ -203,6 +203,7 @@
  */
 package org.jooby.internal.netty;
 
+import com.google.common.escape.Escapers;
 import static io.netty.channel.ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE;
 
 import java.io.IOException;
@@ -217,6 +218,7 @@
 import java.util.stream.Collectors;
 
 import org.jooby.MediaType;
+import org.jooby.Router;
 import org.jooby.Sse;
 import org.jooby.spi.NativePushPromise;
 import org.jooby.spi.NativeRequest;
@@ -290,7 +292,7 @@ public NettyRequest(final ChannelHandlerContext ctx,
     this.req = req;
     this.tmpdir = tmpdir;
     this.query = new QueryStringDecoder(req.uri());
-    this.path = URLDecoder.decode(query.path(), "UTF-8");
+    this.path =  Router.decode(query.path());
     this.wsMaxMessageSize = wsMaxMessageSize;
     Channel channel = ctx.channel();
     channel.attr(ASYNC).set(false);
diff --git a/modules/jooby-servlet/src/main/java/org/jooby/servlet/ServletServletRequest.java b/modules/jooby-servlet/src/main/java/org/jooby/servlet/ServletServletRequest.java
@@ -223,6 +223,7 @@
 
 import org.jooby.Cookie;
 import org.jooby.MediaType;
+import org.jooby.Router;
 import org.jooby.spi.NativeRequest;
 import org.jooby.spi.NativeUpload;
 
@@ -251,7 +252,7 @@ public ServletServletRequest(final HttpServletRequest req, final String tmpdir,
     if (pathInfo == null) {
       pathInfo = "/";
     }
-    this.path = req.getContextPath() + URLDecoder.decode(pathInfo, "UTF-8");
+    this.path = req.getContextPath() + Router.decode(pathInfo);
   }
 
   public HttpServletRequest servletRequest() {
diff --git a/modules/jooby-undertow/src/main/java/org/jooby/internal/undertow/UndertowRequest.java b/modules/jooby-undertow/src/main/java/org/jooby/internal/undertow/UndertowRequest.java
@@ -217,6 +217,7 @@
 
 import org.jooby.Cookie;
 import org.jooby.MediaType;
+import org.jooby.Router;
 import org.jooby.Sse;
 import org.jooby.spi.NativePushPromise;
 import org.jooby.spi.NativeRequest;
@@ -260,7 +261,7 @@ public UndertowRequest(final HttpServerExchange exchange, final Config conf) thr
     this.exchange = exchange;
     this.blocking = Suppliers.memoize(() -> this.exchange.startBlocking());
     this.conf = conf;
-    this.path = URLDecoder.decode(exchange.getRequestPath(), "UTF-8");
+    this.path = Router.decode(exchange.getRequestPath());
   }
 
   @Override

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,6 @@ public void pathVarShouldBeDecoded() throws Exception {`
`17`	`17`
`18`	`18`	`request()`
`19`	`19`	`.get("/plus+plus")`
`20`		`- .expect("plus plus");`
	`20`	`+ .expect("plus+plus");`
`21`	`21`	`}`
`22`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -223,6 +223,7 @@`
`223`	`223`
`224`	`224`	`import org.jooby.Cookie;`
`225`	`225`	`import org.jooby.MediaType;`
	`226`	`+import org.jooby.Router;`
`226`	`227`	`import org.jooby.spi.NativeRequest;`
`227`	`228`	`import org.jooby.spi.NativeUpload;`
`228`	`229`
`@@ -251,7 +252,7 @@ public ServletServletRequest(final HttpServletRequest req, final String tmpdir,`
`251`	`252`	`if (pathInfo == null) {`
`252`	`253`	`pathInfo = "/";`
`253`	`254`	`}`
`254`		`- this.path = req.getContextPath() + URLDecoder.decode(pathInfo, "UTF-8");`
	`255`	`+ this.path = req.getContextPath() + Router.decode(pathInfo);`
`255`	`256`	`}`
`256`	`257`
`257`	`258`	`public HttpServletRequest servletRequest() {`