Michael Blow has submitted this change and it was merged. Change subject: Send Bad-request response on failure of decoding query string ......................................................................
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 <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Michael Blow <mb...@apache.org> --- M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java M hyracks-fullstack/hyracks/hyracks-http/src/test/java/org/apache/hyracks/http/test/HttpServerTest.java 3 files changed, 77 insertions(+), 13 deletions(-) Approvals: Michael Blow: Looks good to me, approved Jenkins: Verified; No violations found 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(); -- To view, visit https://asterix-gerrit.ics.uci.edu/1602 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7aa000e469392a5e4b079b331472edd0dc4f65a4 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>