Change in asterixdb[master]: Send Bad-request response on failure of decoding query string

[abdullah alamoudi (Code Review)]

abdullah alamoudi has uploaded a new change for review.

  https://asterix-gerrit.ics.uci.edu/1602

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
---
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
2 files changed, 26 insertions(+), 7 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb 
refs/changes/02/1602/1

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..364b597 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,39 @@
 
     @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);
             } 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);
+            DefaultHttpResponse serverError =
+                    new DefaultHttpResponse(request.protocolVersion(), 
HttpResponseStatus.INTERNAL_SERVER_ERROR);
+            ctx.write(serverError).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);
+            DefaultHttpResponse badRequest =
+                    new DefaultHttpResponse(request.protocolVersion(), 
HttpResponseStatus.BAD_REQUEST);
+            ctx.write(badRequest).addListener(ChannelFutureListener.CLOSE);
+            return;
+        }
+        handler = new HttpRequestHandler(ctx, servlet, servletRequest, 
chunkSize);
+        submit();
+    }
+
     private void submit() throws IOException {
         try {
             server.getExecutor().submit(handler);

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1602
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7aa000e469392a5e4b079b331472edd0dc4f65a4
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>