unify absent of HTTP parameter and empty strings fix #407

jooby-project · Jul 1, 2016 · 86ab10a · 86ab10a
1 parent f3a2876
commit 86ab10a
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 16 deletions.
diff --git a/coverage-report/src/test/java/org/jooby/issues/Issue407.java b/coverage-report/src/test/java/org/jooby/issues/Issue407.java
@@ -0,0 +1,23 @@
+package org.jooby.issues;
+
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+public class Issue407 extends ServerFeature {
+
+  {
+    get("/407", req -> req.param("foo").toOptional());
+  }
+
+  @Test
+  public void shouldIgnoreEmptyValues() throws Exception {
+    request()
+        .get("/407")
+        .expect("Optional.empty");
+
+    request()
+        .get("/407?foo=")
+        .expect("Optional.empty");
+  }
+
+}
diff --git a/jooby-netty/src/main/java/org/jooby/internal/netty/NettyRequest.java b/jooby-netty/src/main/java/org/jooby/internal/netty/NettyRequest.java
@@ -29,6 +29,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import org.jooby.MediaType;
@@ -37,6 +38,7 @@
 import org.jooby.spi.NativeUpload;
 import org.jooby.spi.NativeWebSocket;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multimap;
@@ -232,8 +234,10 @@ private Multimap<String, String> decodeParams() throws IOException {
       params = ArrayListMultimap.create();
       files = ArrayListMultimap.create();
 
+      Predicate<String> notEmpty = s -> !Strings.isNullOrEmpty(s);
       query.parameters()
-          .forEach((name, values) -> values.forEach(value -> params.put(name, value)));
+          .forEach((name, values) -> values.stream().filter(notEmpty)
+              .forEach(value -> params.put(name, value)));
 
       HttpMethod method = req.method();
       boolean hasBody = method.equals(HttpMethod.POST) || method.equals(HttpMethod.PUT)
@@ -258,16 +262,19 @@ private Multimap<String, String> decodeParams() throws IOException {
           while (hasNext.apply(decoder)) {
             HttpData field = (HttpData) decoder.next();
             try {
-            String name = field.getName();
-            switch (field.getHttpDataType()) {
-              case FileUpload:
-                files.put(name, new NettyUpload((FileUpload) field, tmpdir));
-                // excludes upload from param names.
-                break;
-              default:
-                params.put(name, field.getString());
-                break;
-            }
+              String name = field.getName();
+              switch (field.getHttpDataType()) {
+                case FileUpload:
+                  files.put(name, new NettyUpload((FileUpload) field, tmpdir));
+                  // excludes upload from param names.
+                  break;
+                default:
+                  String value = field.getString();
+                  if (notEmpty.test(value)) {
+                    params.put(name, value);
+                  }
+                  break;
+              }
             } finally {
               field.release();
             }

diff --git a/jooby-servlet/src/main/java/org/jooby/servlet/ServletServletRequest.java b/jooby-servlet/src/main/java/org/jooby/servlet/ServletServletRequest.java
@@ -38,6 +38,7 @@
 import org.jooby.spi.NativeRequest;
 import org.jooby.spi.NativeUpload;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
 
@@ -104,7 +105,9 @@ public List<String> params(final String name) throws Exception {
     if (values == null) {
       return Collections.emptyList();
     }
-    return ImmutableList.copyOf(values);
+    return Arrays.asList(values).stream()
+        .filter(s -> !Strings.isNullOrEmpty(s))
+        .collect(Collectors.toList());
   }
 
   @Override

diff --git a/jooby-undertow/src/main/java/org/jooby/internal/undertow/UndertowRequest.java b/jooby-undertow/src/main/java/org/jooby/internal/undertow/UndertowRequest.java
@@ -27,6 +27,7 @@
 import java.util.Deque;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import org.jooby.Cookie;
@@ -36,6 +37,7 @@
 import org.jooby.spi.NativeUpload;
 import org.jooby.spi.NativeWebSocket;
 
+import com.google.common.base.Strings;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
@@ -105,13 +107,14 @@ public List<String> params(final String name) {
     Builder<String> builder = ImmutableList.builder();
     // query params
     Deque<String> query = exchange.getQueryParameters().get(name);
+    Predicate<String> notEmpty = s -> !Strings.isNullOrEmpty(s);
     if (query != null) {
-      query.forEach(builder::add);
+      query.stream().filter(notEmpty).forEach(builder::add);
     }
     // form params
     Optional.ofNullable(form.get(name)).ifPresent(values -> {
-      values.forEach(value -> {
-        if (!value.isFile()) {
+      values.stream().forEach(value -> {
+        if (!value.isFile() && notEmpty.test(value.getValue())) {
           builder.add(value.getValue());
         }
       });

diff --git a/jooby/src/main/java/org/jooby/internal/RequestImpl.java b/jooby/src/main/java/org/jooby/internal/RequestImpl.java
@@ -388,7 +388,7 @@ private List<String> params(final String name) {
     try {
       return req.params(name);
     } catch (Exception ex) {
-      throw new Err(Status.BAD_REQUEST, "Param " + name + " resulted in error", ex);
+      throw new Err(Status.BAD_REQUEST, "Parameter '" + name + "' resulted in error", ex);
     }
   }