Send Bad-request response on failure of decoding query string
Change-Id: I7aa000e469392a5e4b079b331472edd0dc4f65a4
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1602
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Michael Blow <mblow@apache.org>
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
index d271177..5b354af 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
@@ -22,18 +22,20 @@
import java.util.List;
import java.util.Map;
-import io.netty.handler.codec.http.QueryStringDecoder;
import org.apache.hyracks.http.api.IServletRequest;
import org.apache.hyracks.http.server.utils.HttpUtil;
import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.QueryStringDecoder;
public class BaseRequest implements IServletRequest {
protected final FullHttpRequest request;
protected final Map<String, List<String>> parameters;
public static IServletRequest create(FullHttpRequest request) throws IOException {
- return new BaseRequest(request, new QueryStringDecoder(request.uri()).parameters());
+ QueryStringDecoder decoder = new QueryStringDecoder(request.uri());
+ Map<String, List<String>> param = decoder.parameters();
+ return new BaseRequest(request, param);
}
protected BaseRequest(FullHttpRequest request, Map<String, List<String>> parameters) {
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java
index 743800e..6f7ccba 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java
@@ -24,6 +24,7 @@
import java.util.logging.Logger;
import org.apache.hyracks.http.api.IServlet;
+import org.apache.hyracks.http.api.IServletRequest;
import org.apache.hyracks.http.server.utils.HttpUtil;
import io.netty.channel.ChannelFutureListener;
@@ -60,23 +61,38 @@
@Override
protected void channelRead0(ChannelHandlerContext ctx, Object msg) {
+ FullHttpRequest request = (FullHttpRequest) msg;
try {
- FullHttpRequest request = (FullHttpRequest) msg;
IServlet servlet = server.getServlet(request);
if (servlet == null) {
- DefaultHttpResponse notFound =
- new DefaultHttpResponse(request.protocolVersion(), HttpResponseStatus.NOT_FOUND);
- ctx.write(notFound).addListener(ChannelFutureListener.CLOSE);
+ respond(ctx, request, HttpResponseStatus.NOT_FOUND);
} else {
- handler = new HttpRequestHandler(ctx, servlet, HttpUtil.toServletRequest(request), chunkSize);
- submit();
+ submit(ctx, servlet, request);
}
} catch (Exception e) {
- LOGGER.log(Level.SEVERE, "Failure handling HTTP Request", e);
- ctx.close();
+ LOGGER.log(Level.SEVERE, "Failure Submitting HTTP Request", e);
+ respond(ctx, request, HttpResponseStatus.INTERNAL_SERVER_ERROR);
}
}
+ private void respond(ChannelHandlerContext ctx, FullHttpRequest request, HttpResponseStatus status) {
+ DefaultHttpResponse response = new DefaultHttpResponse(request.protocolVersion(), status);
+ ctx.write(response).addListener(ChannelFutureListener.CLOSE);
+ }
+
+ private void submit(ChannelHandlerContext ctx, IServlet servlet, FullHttpRequest request) throws IOException {
+ IServletRequest servletRequest;
+ try {
+ servletRequest = HttpUtil.toServletRequest(request);
+ } catch (IllegalArgumentException e) {
+ LOGGER.log(Level.WARNING, "Failure Decoding Request", e);
+ respond(ctx, request, HttpResponseStatus.BAD_REQUEST);
+ return;
+ }
+ handler = new HttpRequestHandler(ctx, servlet, servletRequest, chunkSize);
+ submit();
+ }
+
private void submit() throws IOException {
try {
server.getExecutor().submit(handler);
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/test/java/org/apache/hyracks/http/test/HttpServerTest.java b/hyracks-fullstack/hyracks/hyracks-http/src/test/java/org/apache/hyracks/http/test/HttpServerTest.java
index 20e6fe7..2076201 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/test/java/org/apache/hyracks/http/test/HttpServerTest.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/test/java/org/apache/hyracks/http/test/HttpServerTest.java
@@ -18,7 +18,13 @@
*/
package org.apache.hyracks.http.test;
+import java.io.BufferedReader;
import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.PrintWriter;
+import java.lang.reflect.Field;
+import java.net.InetAddress;
+import java.net.Socket;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
@@ -76,11 +82,51 @@
}
}
+ @Test
+ public void testMalformedString() throws Exception {
+ WebManager webMgr = new WebManager();
+ HttpServer server =
+ new HttpServer(webMgr.getBosses(), webMgr.getWorkers(), PORT, NUM_EXECUTOR_THREADS, SERVER_QUEUE_SIZE);
+ SlowServlet servlet = new SlowServlet(server.ctx(), new String[] { PATH });
+ server.addServlet(servlet);
+ webMgr.add(server);
+ webMgr.start();
+ try {
+ StringBuilder response = new StringBuilder();
+ try (Socket s = new Socket(InetAddress.getLocalHost(), PORT)) {
+ PrintWriter pw = new PrintWriter(s.getOutputStream());
+ pw.println("GET /?handle=%7B%22handle%22%3A%5B0%2C%200%5D%7 HTTP/1.1");
+ pw.println("Host: 127.0.0.1");
+ pw.println();
+ pw.flush();
+ BufferedReader br = new BufferedReader(new InputStreamReader(s.getInputStream()));
+ String line;
+ while ((line = br.readLine()) != null) {
+ response.append(line).append('\n');
+ }
+ br.close();
+ }
+ String output = response.toString();
+ Assert.assertTrue(output.contains(HttpResponseStatus.BAD_REQUEST.toString()));
+ } catch (Exception e) {
+ e.printStackTrace();
+ throw e;
+ } finally {
+ webMgr.stop();
+ }
+ }
+
+ public static void setPrivateField(Object obj, String filedName, Object value) throws Exception {
+ Field f = obj.getClass().getDeclaredField(filedName);
+ f.setAccessible(true);
+ f.set(obj, value);
+ }
+
private void request(int count) {
for (int i = 0; i < count; i++) {
Thread next = new Thread(() -> {
try {
- HttpUriRequest request = request();
+ HttpUriRequest request = request(null);
HttpResponse response = executeHttpRequest(request);
if (response.getStatusLine().getStatusCode() == HttpResponseStatus.OK.code()) {
SUCCESS_COUNT.incrementAndGet();
@@ -111,8 +157,8 @@
}
}
- protected HttpUriRequest request() throws URISyntaxException {
- URI uri = new URI(PROTOCOL, null, HOST, PORT, PATH, null, null);
+ protected HttpUriRequest request(String query) throws URISyntaxException {
+ URI uri = new URI(PROTOCOL, null, HOST, PORT, PATH, query, null);
RequestBuilder builder = RequestBuilder.get(uri);
builder.setCharset(StandardCharsets.UTF_8);
return builder.build();