From 0a31fca8fef6cf1b09ad137eae18890bf4bf6247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Prante?= Date: Thu, 21 Dec 2023 15:48:37 +0100 Subject: [PATCH] fix parameter/multimap handling with duplicates, add test --- gradle.properties | 2 +- net-http-server-netty/build.gradle | 1 + .../net/http/server/netty/HttpRequest.java | 4 +- .../http/server/netty/HttpRequestBuilder.java | 19 +---- .../net/http/netty/test/HttpRequestTest.java | 70 ++++++++++++++++ .../xbib/net/http/server/BaseHttpRequest.java | 84 +++++++++++-------- .../http/server/BaseHttpRequestBuilder.java | 15 ++-- 7 files changed, 135 insertions(+), 60 deletions(-) create mode 100644 net-http-server-netty/src/test/java/org/xbib/net/http/netty/test/HttpRequestTest.java diff --git a/gradle.properties b/gradle.properties index ee6b1a3..52208db 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ group = org.xbib name = net-http -version = 4.0.8 +version = 4.0.9 diff --git a/net-http-server-netty/build.gradle b/net-http-server-netty/build.gradle index 9c572c2..9baa493 100644 --- a/net-http-server-netty/build.gradle +++ b/net-http-server-netty/build.gradle @@ -3,6 +3,7 @@ dependencies { api libs.netty.codec.http2 testImplementation project(':net-http-client-netty') testImplementation project(':net-http-template-groovy') + testImplementation libs.datastructures.json.tiny } test { diff --git a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequest.java b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequest.java index 54eab1a..e643a01 100644 --- a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequest.java +++ b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequest.java @@ -35,7 +35,7 @@ public class HttpRequest extends BaseHttpRequest { @Override public InputStream getInputStream() { - return builder.byteBuffer != null ? new ByteBufferInputStream(builder.byteBuffer) : null; + return builder.getBody() != null ? new ByteBufferInputStream(builder.getBody()) : null; } @Override @@ -49,7 +49,7 @@ public class HttpRequest extends BaseHttpRequest { return "HttpRequest[method=" + builder.getMethod() + ",version=" + builder.getVersion() + ",parameter=" + builder.getParameter() + - ",body=" + (builder.byteBuffer != null) + + ",body=" + (builder.getBody() != null) + "]"; } } diff --git a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequestBuilder.java b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequestBuilder.java index 47bda31..7d609cb 100644 --- a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequestBuilder.java +++ b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/HttpRequestBuilder.java @@ -27,10 +27,6 @@ public class HttpRequestBuilder extends BaseHttpRequestBuilder { private FullHttpRequest httpRequest; - protected ByteBuffer byteBuffer; - - protected CharBuffer charBuffer; - protected HttpRequestBuilder() { } @@ -59,25 +55,12 @@ public class HttpRequestBuilder extends BaseHttpRequestBuilder { if (fullHttpRequest.content() != null) { ByteBuf byteBuf = fullHttpRequest.content(); byte[] bytes = ByteBufUtil.getBytes(byteBuf); - byteBuffer = ByteBuffer.wrap(bytes); + this.byteBuffer = ByteBuffer.wrap(bytes); } } return this; } - @Override - public ByteBuffer getBody() { - return byteBuffer; - } - - @Override - public CharBuffer getBodyAsChars(Charset charset) { - if (charBuffer == null) { - charBuffer = byteBuffer != null ? charset.decode(byteBuffer) : null; - } - return charBuffer; - } - @Override public HttpRequestBuilder setAddress(HttpAddress httpAddress) { super.setAddress(httpAddress); diff --git a/net-http-server-netty/src/test/java/org/xbib/net/http/netty/test/HttpRequestTest.java b/net-http-server-netty/src/test/java/org/xbib/net/http/netty/test/HttpRequestTest.java new file mode 100644 index 0000000..a4234d3 --- /dev/null +++ b/net-http-server-netty/src/test/java/org/xbib/net/http/netty/test/HttpRequestTest.java @@ -0,0 +1,70 @@ +package org.xbib.net.http.netty.test; + +import org.junit.jupiter.api.Test; +import org.xbib.datastructures.api.Builder; +import org.xbib.datastructures.json.tiny.JsonBuilder; +import org.xbib.net.Parameter; +import org.xbib.net.http.HttpHeaders; +import org.xbib.net.http.HttpMethod; +import org.xbib.net.http.server.HttpRequest; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Objects; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class HttpRequestTest { + + @Test + public void testFormAsMultiMap() { + Parameter parameter = Parameter.builder().domain(Parameter.Domain.FORM) + .enableDuplicates() + .add("key", "key1") + .add("key", "key2") + .add("key", "key3") + .add("value", "value1") + .add("value", "") + .add("value", "") + .add("op", "and") + .add("op", "") + .add("op", "") + .build(); + HttpHeaders httpHeaders = new HttpHeaders(); + HttpRequest httpRequest = org.xbib.net.http.server.netty.HttpRequest.builder() + .setMethod(HttpMethod.POST) + .setParameter(parameter) + .setHeaders(httpHeaders) + .build(); + assertEquals("{key=[key1, key2, key3], value=[value1, , ], op=[and, , ]}", + httpRequest.asMultiMap().toString()); + } + + @Test + public void testJsonBodyAsMultiMap() throws IOException { + // JSON keys overwrite each other + Builder builder = JsonBuilder.builder() + .beginMap() + .field("key", "key1") + .field("key", "key2") + .field("key", "key3") + .field("value", "value1") + .field("value", "") + .field("value", "") + .field("op", "and") + .field("op", "") + .field("op", "") + .endMap(); + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.add("content-type", "application/json"); + HttpRequest httpRequest = org.xbib.net.http.server.netty.HttpRequest.builder() + .setMethod(HttpMethod.POST) + .setHeaders(httpHeaders) + .setBody(ByteBuffer.wrap(builder.build().getBytes(StandardCharsets.UTF_8))) + .build(); + Objects.requireNonNull(httpRequest.getBody()); + assertEquals("{key=[key3], value=[], op=[]}", + httpRequest.asMultiMap().toString()); + } +} diff --git a/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequest.java b/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequest.java index 7269ca1..c503cb1 100644 --- a/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequest.java +++ b/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequest.java @@ -2,6 +2,7 @@ package org.xbib.net.http.server; import java.io.IOException; import java.net.InetSocketAddress; +import java.nio.CharBuffer; import java.nio.charset.MalformedInputException; import java.nio.charset.StandardCharsets; import java.nio.charset.UnmappableCharacterException; @@ -184,34 +185,45 @@ public abstract class BaseHttpRequest implements HttpRequest { @SuppressWarnings("unchecked") @Override public MultiMap asMultiMap() { - PercentDecoder percentDecoder = new PercentDecoder(); MultiMap multiMap = new ParameterMap(); - String contentType = getHeaders().get(HttpHeaderNames.CONTENT_TYPE); - if (getMethod() == HttpMethod.POST && - contentType != null && contentType.contains(HttpHeaderValues.APPLICATION_JSON)) { - String bodyAsChars = getBodyAsChars(StandardCharsets.UTF_8).toString(); - Map map = Json.toMap(bodyAsChars); - for (Map.Entry entry : map.entrySet()) { - if (entry.getValue() instanceof Iterable) { - multiMap.putAll(entry.getKey(), (Iterable) entry.getValue()); + Parameter parameter = getParameter(); + try { + PercentDecoder percentDecoder = new PercentDecoder(); + String contentType = getHeaders().get(HttpHeaderNames.CONTENT_TYPE); + if (getMethod() == HttpMethod.POST && + contentType != null && contentType.contains(HttpHeaderValues.APPLICATION_JSON)) { + CharBuffer charBuffer = builder.getBodyAsChars(StandardCharsets.UTF_8); + if (charBuffer != null) { + String bodyAsChars = charBuffer.toString(); + Map map = Json.toMap(bodyAsChars); + for (Map.Entry entry : map.entrySet()) { + if (entry.getValue() instanceof Iterable) { + multiMap.putAll(entry.getKey(), (Iterable) entry.getValue()); + } else { + multiMap.put(entry.getKey(), entry.getValue()); + } + } } else { - multiMap.put(entry.getKey(), entry.getValue()); + logger.log(Level.WARNING, "body is null in POST request where json is declared"); + } + } else { + if (parameter != null) { + toMultiMapEntry(parameter.get(Parameter.Domain.FORM), + percentDecoder, + HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED.equals(contentType), + multiMap); } } - } - try { - toMultiMapEntry(getParameter().get(Parameter.Domain.PATH), - percentDecoder, - false, - multiMap); - toMultiMapEntry(getParameter().get(Parameter.Domain.FORM), - percentDecoder, - HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED.equals(contentType), - multiMap); - toMultiMapEntry(getParameter().get(Parameter.Domain.QUERY), - percentDecoder, - HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED.equals(contentType), - multiMap); + if (parameter != null) { + toMultiMapEntry(parameter.get(Parameter.Domain.PATH), + percentDecoder, + false, + multiMap); + toMultiMapEntry(parameter.get(Parameter.Domain.QUERY), + percentDecoder, + HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED.equals(contentType), + multiMap); + } } catch (ParameterException e) { logger.log(Level.WARNING, e.getMessage(), ExceptionFormatter.format(e)); } @@ -223,23 +235,29 @@ public abstract class BaseHttpRequest implements HttpRequest { PercentDecoder percentDecoder, boolean isFormEncoded, MultiMap multiMap) { + if (parameter == null) { + return; + } for (Pair entry : parameter) { try { List list; - Object value = entry.getValue(); - if (value instanceof List) { - list = (List) value; - } else if (value != null) { - list = List.of(value); + Object object = entry.getValue(); + if (object instanceof List) { + list = (List) object; + } else if (object != null) { + list = List.of(object); } else { list = List.of(); } - for (Object object : list) { - String string = object.toString(); + for (Object o : list) { + String string = o.toString(); if (isFormEncoded) { string = string.replace('+', ' '); } - multiMap.put(entry.getKey(), percentDecoder.decode(string)); + String name = entry.getKey(); + String value = percentDecoder.decode(string); + logger.log(Level.INFO, "name = " + name + " value = " + value); + multiMap.put(name, value); } } catch (MalformedInputException | UnmappableCharacterException e) { logger.log(Level.WARNING, "unable to percent decode parameter: " + @@ -255,7 +273,7 @@ public abstract class BaseHttpRequest implements HttpRequest { @Override protected Collection newValues() { - // keep values with multiple occurences + // keep values with multiple occurrences return TinyList.builder(); } } diff --git a/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequestBuilder.java b/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequestBuilder.java index 1944d82..e4a9fed 100644 --- a/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequestBuilder.java +++ b/net-http-server/src/main/java/org/xbib/net/http/server/BaseHttpRequestBuilder.java @@ -1,6 +1,5 @@ package org.xbib.net.http.server; -import java.net.InetAddress; import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.nio.CharBuffer; @@ -57,6 +56,8 @@ public abstract class BaseHttpRequestBuilder implements HttpRequestBuilder { protected ByteBuffer byteBuffer; + protected CharBuffer charBuffer; + protected boolean done; protected List messages; @@ -194,6 +195,13 @@ public abstract class BaseHttpRequestBuilder implements HttpRequestBuilder { return byteBuffer; } + public CharBuffer getBodyAsChars(Charset charset) { + if (charBuffer == null) { + charBuffer = byteBuffer != null ? charset.decode(byteBuffer) : null; + } + return charBuffer; + } + @Override public BaseHttpRequestBuilder setBaseURL(URL baseURL) { if (done) { @@ -259,11 +267,6 @@ public abstract class BaseHttpRequestBuilder implements HttpRequestBuilder { return requestPath; } - @Override - public CharBuffer getBodyAsChars(Charset charset) { - return byteBuffer != null ? charset.decode(byteBuffer) : null; - } - @Override public BaseHttpRequestBuilder setParameter(Parameter parameter) { if (done) {