[grid] Guarding how the server reads parameters

diemol · diemol · commit 48848de6ef3d · 2022-01-13T10:04:15.000+01:00
Some users have security tools that send different requests to the Grid and made it crash. This should guard it from those situations. Fixes SeleniumHQ/docker-selenium#1469
diff --git a/java/src/org/openqa/selenium/netty/server/RequestConverter.java b/java/src/org/openqa/selenium/netty/server/RequestConverter.java
@@ -17,7 +17,17 @@
 
 package org.openqa.selenium.netty.server;
 
+import static io.netty.handler.codec.http.HttpMethod.HEAD;
+import static org.openqa.selenium.remote.http.Contents.memoize;
+
 import com.google.common.io.ByteStreams;
+
+import org.openqa.selenium.internal.Debug;
+import org.openqa.selenium.remote.http.HttpMethod;
+import org.openqa.selenium.remote.http.HttpRequest;
+import org.openqa.selenium.remote.http.HttpResponse;
+import org.openqa.selenium.remote.tracing.AttributeKey;
+
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufInputStream;
 import io.netty.channel.ChannelHandlerContext;
@@ -31,10 +41,6 @@
 import io.netty.handler.codec.http.LastHttpContent;
 import io.netty.handler.codec.http.QueryStringDecoder;
 import io.netty.util.ReferenceCountUtil;
-import org.openqa.selenium.remote.http.HttpMethod;
-import org.openqa.selenium.remote.http.HttpRequest;
-import org.openqa.selenium.remote.http.HttpResponse;
-import org.openqa.selenium.remote.tracing.AttributeKey;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -45,9 +51,6 @@
 import java.util.concurrent.Executors;
 import java.util.logging.Logger;
 
-import static io.netty.handler.codec.http.HttpMethod.HEAD;
-import static org.openqa.selenium.remote.http.Contents.memoize;
-
 class RequestConverter extends SimpleChannelInboundHandler<HttpObject> {
 
   private static final Logger LOG = Logger.getLogger(RequestConverter.class.getName());
@@ -133,23 +136,32 @@ private HttpRequest createRequest(
       try {
         method = HttpMethod.valueOf(nettyRequest.method().name());
       } catch (IllegalArgumentException e) {
-        ctx.writeAndFlush(new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.METHOD_NOT_ALLOWED));
+        ctx.writeAndFlush(
+          new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.METHOD_NOT_ALLOWED));
         return null;
       }
     }
 
-    QueryStringDecoder decoder = new QueryStringDecoder(nettyRequest.uri());
-
-    HttpRequest req = new HttpRequest(
-      method,
-      decoder.path());
-
-    decoder.parameters().forEach((key, values) -> values.forEach(value -> req.addQueryParameter(key, value)));
-
-    nettyRequest.headers().entries().stream()
-      .filter(entry -> entry.getKey() != null)
-      .forEach(entry -> req.addHeader(entry.getKey(), entry.getValue()));
-
-    return req;
+    // Attempt to decode parameters
+    try {
+      QueryStringDecoder decoder = new QueryStringDecoder(nettyRequest.uri());
+
+      HttpRequest req = new HttpRequest(
+        method,
+        decoder.path());
+
+      decoder.parameters()
+        .forEach((key, values) -> values.forEach(value -> req.addQueryParameter(key, value)));
+
+      nettyRequest.headers().entries().stream()
+        .filter(entry -> entry.getKey() != null)
+        .forEach(entry -> req.addHeader(entry.getKey(), entry.getValue()));
+      return req;
+    } catch (Exception e) {
+      ctx.writeAndFlush(
+        new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.BAD_REQUEST));
+      LOG.log(Debug.getDebugLogLevel(), "Not possible to decode parameters.", e);
+      return null;
+    }
   }
 }
