Skip to content

Commit d6cf8e8

Browse files
jknackjknack
committed
revert: unify absent of HTTP parameter and empty strings
* fix #437 * Empty parameters are treated as missing fix #431
1 parent 9990e61 commit d6cf8e8

16 files changed

Lines changed: 157 additions & 88 deletions

File tree

‎coverage-report/src/test/java/org/jooby/issues/Issue407.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,18 @@ public class Issue407 extends ServerFeature {
1010
}
1111

1212
@Test
13-
public void shouldIgnoreEmptyValues() throws Exception {
13+
public void shouldNotIgnoreEmptyParams() throws Exception {
1414
request()
1515
.get("/407")
1616
.expect("Optional.empty");
1717

18+
request()
19+
.get("/407?foo")
20+
.expect("Optional[]");
21+
1822
request()
1923
.get("/407?foo=")
20-
.expect("Optional.empty");
24+
.expect("Optional[]");
2125
}
2226

2327
}

‎coverage-report/src/test/java/org/jooby/issues/Issue408.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ public void shouldIgnoreEmptyValues() throws Exception {
3333
.get("/408?id=1&title=Title&releaseDate=")
3434
.expect("1:Title:null");
3535

36+
request()
37+
.get("/408?id=&title=Title&releaseDate")
38+
.expect("null:Title:null");
39+
40+
request()
41+
.get("/408?id&title=&releaseDate=")
42+
.expect("null::null");
43+
3644
request()
3745
.get("/408")
3846
.expect("null:null:null");

‎coverage-report/src/test/java/org/jooby/issues/Issue408b.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package org.jooby.issues;
2+
3+
import java.util.Date;
4+
5+
import org.jooby.test.ServerFeature;
6+
import org.junit.Test;
7+
8+
public class Issue408b extends ServerFeature {
9+
10+
public static class Bean408 {
11+
public Integer id;
12+
13+
public String title;
14+
15+
public Date releaseDate;
16+
17+
@Override
18+
public String toString() {
19+
return id + ":" + title + ":" + releaseDate;
20+
}
21+
}
22+
23+
{
24+
get("/408", req -> req.params(Bean408.class).toString());
25+
}
26+
27+
@Test
28+
public void shouldIgnoreEmptyValues() throws Exception {
29+
request()
30+
.get("/408?id=1&title=Title")
31+
.expect("1:Title:null");
32+
33+
request()
34+
.get("/408?title=Title")
35+
.expect("null:Title:null");
36+
37+
request()
38+
.get("/408")
39+
.expect("null:null:null");
40+
}
41+
42+
}

‎coverage-report/src/test/java/org/jooby/issues/Issue408c.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package org.jooby.issues;
2+
3+
import java.util.Date;
4+
5+
import org.jooby.Parser;
6+
import org.jooby.test.ServerFeature;
7+
import org.junit.Test;
8+
9+
public class Issue408c extends ServerFeature {
10+
11+
public static class Bean408 {
12+
public Integer id;
13+
14+
public String title;
15+
16+
public Date releaseDate;
17+
18+
@Override
19+
public String toString() {
20+
return id + ":" + title + ":" + releaseDate;
21+
}
22+
}
23+
24+
{
25+
parser(Parser.bean(true));
26+
27+
get("/408", req -> req.params(Bean408.class).toString());
28+
29+
err((req, rsp, err) -> {
30+
rsp.send(err.getMessage());
31+
});
32+
}
33+
34+
@Test
35+
public void wrongValuesShouldBeReported() throws Exception {
36+
request()
37+
.get("/408?id=1&title=Title&releaseDate=*")
38+
.expect("Server Error(500): Failed to parse parameter 'releaseDate' to 'java.util.Date'");
39+
}
40+
41+
}

‎jooby-netty/src/main/java/org/jooby/internal/netty/NettyRequest.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.util.List;
3030
import java.util.Optional;
3131
import java.util.function.Function;
32-
import java.util.function.Predicate;
3332
import java.util.stream.Collectors;
3433

3534
import org.jooby.MediaType;
@@ -38,7 +37,6 @@
3837
import org.jooby.spi.NativeUpload;
3938
import org.jooby.spi.NativeWebSocket;
4039

41-
import com.google.common.base.Strings;
4240
import com.google.common.collect.ArrayListMultimap;
4341
import com.google.common.collect.ImmutableList;
4442
import com.google.common.collect.Multimap;
@@ -234,10 +232,8 @@ private Multimap<String, String> decodeParams() throws IOException {
234232
params = ArrayListMultimap.create();
235233
files = ArrayListMultimap.create();
236234

237-
Predicate<String> notEmpty = s -> !Strings.isNullOrEmpty(s);
238235
query.parameters()
239-
.forEach((name, values) -> values.stream().filter(notEmpty)
240-
.forEach(value -> params.put(name, value)));
236+
.forEach((name, values) -> values.forEach(value -> params.put(name, value)));
241237

242238
HttpMethod method = req.method();
243239
boolean hasBody = method.equals(HttpMethod.POST) || method.equals(HttpMethod.PUT)
@@ -270,9 +266,7 @@ private Multimap<String, String> decodeParams() throws IOException {
270266
break;
271267
default:
272268
String value = field.getString();
273-
if (notEmpty.test(value)) {
274-
params.put(name, value);
275-
}
269+
params.put(name, value);
276270
break;
277271
}
278272
} finally {

‎jooby-servlet/src/main/java/org/jooby/servlet/ServletServletRequest.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import org.jooby.spi.NativeRequest;
3939
import org.jooby.spi.NativeUpload;
4040

41-
import com.google.common.base.Strings;
4241
import com.google.common.collect.ImmutableList;
4342
import com.google.common.collect.ImmutableList.Builder;
4443

@@ -105,9 +104,7 @@ public List<String> params(final String name) throws Exception {
105104
if (values == null) {
106105
return Collections.emptyList();
107106
}
108-
return Arrays.asList(values).stream()
109-
.filter(s -> !Strings.isNullOrEmpty(s))
110-
.collect(Collectors.toList());
107+
return Arrays.asList(values);
111108
}
112109

113110
@Override

‎jooby-undertow/src/main/java/org/jooby/internal/undertow/UndertowRequest.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import java.util.Deque;
2828
import java.util.List;
2929
import java.util.Optional;
30-
import java.util.function.Predicate;
3130
import java.util.stream.Collectors;
3231

3332
import org.jooby.Cookie;
@@ -37,7 +36,6 @@
3736
import org.jooby.spi.NativeUpload;
3837
import org.jooby.spi.NativeWebSocket;
3938

40-
import com.google.common.base.Strings;
4139
import com.google.common.base.Supplier;
4240
import com.google.common.base.Suppliers;
4341
import com.google.common.collect.ImmutableList;
@@ -107,14 +105,13 @@ public List<String> params(final String name) {
107105
Builder<String> builder = ImmutableList.builder();
108106
// query params
109107
Deque<String> query = exchange.getQueryParameters().get(name);
110-
Predicate<String> notEmpty = s -> !Strings.isNullOrEmpty(s);
111108
if (query != null) {
112-
query.stream().filter(notEmpty).forEach(builder::add);
109+
query.stream().forEach(builder::add);
113110
}
114111
// form params
115112
Optional.ofNullable(form.get(name)).ifPresent(values -> {
116113
values.stream().forEach(value -> {
117-
if (!value.isFile() && notEmpty.test(value.getValue())) {
114+
if (!value.isFile()) {
118115
builder.add(value.getValue());
119116
}
120117
});

‎jooby/src/main/java/org/jooby/Parser.java‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
import java.io.OutputStream;
2323
import java.util.Iterator;
2424
import java.util.Map;
25+
import java.util.NoSuchElementException;
2526
import java.util.Optional;
27+
import java.util.function.Function;
2628

2729
import org.jooby.internal.parser.BeanParser;
2830

@@ -373,6 +375,14 @@ interface Context extends Builder {
373375

374376
}
375377

378+
/** Utility function to handle empty values as {@link NoSuchElementException}. */
379+
static Function<String, String> NOT_EMPTY = v -> {
380+
if (v.length() == 0) {
381+
throw new NoSuchElementException();
382+
}
383+
return v;
384+
};
385+
376386
/**
377387
* <p>
378388
* Parse one or more values to the required type. If the parser doesn't support the required type
@@ -420,9 +430,9 @@ interface Context extends Builder {
420430
Object parse(TypeLiteral<?> type, Context ctx) throws Throwable;
421431

422432
/**
423-
* Overwrite the default bean parser with <code>null</code> supports. The default bean parser
424-
* doesn't allow <code>null</code>, so if a parameter is optional you must declare it as
425-
* {@link Optional} otherwise parsing fails with a <code>404</code> status code.
433+
* Overwrite the default bean parser with <code>empty/null</code> supports. The default bean
434+
* parser doesn't allow <code>null</code>, so if a parameter is optional you must declare it as
435+
* {@link Optional} otherwise parsing fails with a <code>404/500</code> status code.
426436
*
427437
* For example:
428438
* <pre>{@code
@@ -451,15 +461,18 @@ interface Context extends Builder {
451461
* With <code>/?title=Title&amp;releaseDate=</code> prints <code>Title:null</code>.
452462
* </p>
453463
* <p>
454-
* Now, same call with <code>allowNulls=false</code> results in <code>Bad Request: 400</code>
464+
* Now, same call with <code>lenient=false</code> results in <code>Bad Request: 400</code>
455465
* because <code>releaseDate</code> if required and isn't present in the HTTP request.
456466
* </p>
457467
*
458-
* @param allowNulls Enabled null supports while parsing HTTP params as Java Beans.
459-
* @return A new parser.
468+
* <p>
469+
* This feature is useful while submitting forms.
470+
* </p>
471+
*
472+
* @param lenient Enabled null/empty supports while parsing HTTP params as Java Beans.
473+
* @return A new bean parser.
460474
*/
461-
static Parser bean(final boolean allowNulls) {
462-
return new BeanParser(allowNulls);
475+
static Parser bean(final boolean lenient) {
476+
return new BeanParser(lenient);
463477
}
464-
465478
}

‎jooby/src/main/java/org/jooby/internal/BuiltinParser.java‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -49,24 +49,24 @@ public enum BuiltinParser implements Parser {
4949
Basic {
5050
private final Map<Class<?>, Function<String, Object>> parsers =
5151
ImmutableMap.<Class<?>, Function<String, Object>> builder()
52-
.put(BigDecimal.class, BigDecimal::new)
53-
.put(BigInteger.class, BigInteger::new)
54-
.put(Byte.class, Byte::valueOf)
55-
.put(byte.class, Byte::valueOf)
56-
.put(Double.class, Double::valueOf)
57-
.put(double.class, Double::valueOf)
58-
.put(Float.class, Float::valueOf)
59-
.put(float.class, Float::valueOf)
60-
.put(Integer.class, Integer::valueOf)
61-
.put(int.class, Integer::valueOf)
62-
.put(Long.class, this::toLong)
63-
.put(long.class, this::toLong)
64-
.put(Short.class, Short::valueOf)
65-
.put(short.class, Short::valueOf)
66-
.put(Boolean.class, this::toBoolean)
67-
.put(boolean.class, this::toBoolean)
68-
.put(Character.class, this::toCharacter)
69-
.put(char.class, this::toCharacter)
52+
.put(BigDecimal.class, NOT_EMPTY.andThen(BigDecimal::new))
53+
.put(BigInteger.class, NOT_EMPTY.andThen(BigInteger::new))
54+
.put(Byte.class, NOT_EMPTY.andThen(Byte::valueOf))
55+
.put(byte.class, NOT_EMPTY.andThen(Byte::valueOf))
56+
.put(Double.class, NOT_EMPTY.andThen(Double::valueOf))
57+
.put(double.class, NOT_EMPTY.andThen(Double::valueOf))
58+
.put(Float.class, NOT_EMPTY.andThen(Float::valueOf))
59+
.put(float.class, NOT_EMPTY.andThen(Float::valueOf))
60+
.put(Integer.class, NOT_EMPTY.andThen(Integer::valueOf))
61+
.put(int.class, NOT_EMPTY.andThen(Integer::valueOf))
62+
.put(Long.class, NOT_EMPTY.andThen(this::toLong))
63+
.put(long.class, NOT_EMPTY.andThen(this::toLong))
64+
.put(Short.class, NOT_EMPTY.andThen(Short::valueOf))
65+
.put(short.class, NOT_EMPTY.andThen(Short::valueOf))
66+
.put(Boolean.class, NOT_EMPTY.andThen(this::toBoolean))
67+
.put(boolean.class, NOT_EMPTY.andThen(this::toBoolean))
68+
.put(Character.class, NOT_EMPTY.andThen(this::toCharacter))
69+
.put(char.class, NOT_EMPTY.andThen(this::toCharacter))
7070
.put(String.class, this::toString)
7171
.build();
7272

‎jooby/src/main/java/org/jooby/internal/MutantImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@
2525

2626
import java.util.HashMap;
2727
import java.util.Map;
28+
import java.util.NoSuchElementException;
2829

2930
import org.jooby.Err;
3031
import org.jooby.MediaType;
3132
import org.jooby.Mutant;
3233
import org.jooby.Status;
33-
import org.jooby.internal.parser.ParamNotFound;
3434
import org.jooby.internal.parser.ParserExecutor;
3535

3636
import com.google.common.collect.ImmutableMap;
@@ -87,7 +87,7 @@ public <T> T to(final TypeLiteral<T> type, final MediaType mtype) {
8787
throw new Err(md._3, String.format(FAILURE, md._2, type));
8888
}
8989
results.put(type, result);
90-
} catch (ParamNotFound ex) {
90+
} catch (NoSuchElementException ex) {
9191
Tuple3<String, String, Status> md = md();
9292
throw new Err.Missing(String.format(REQUIRED, md._2));
9393
} catch (Err ex) {

0 commit comments

Comments
 (0)