Merge pull request #585 from arnaudgeiser/master

Better handling of REQUEST_URI_TOO_LONG/REQUEST_HEADER_FIELDS_TOO_LARGE

Author: KingMob

src/aleph/http/server.clj

@@ -7,7 +7,7 @@
     [clojure.tools.logging :as log]
     [clojure.string :as str]
     [manifold.deferred :as d]
-    [manifold.stream :as s])
+    [manifold.stream :as s]) 
   (:import
     [java.util
      EnumSet
@@ -30,11 +30,12 @@
      ChannelHandlerContext
      ChannelHandler
      ChannelPipeline]
-    [io.netty.channel.embedded EmbeddedChannel]
     [io.netty.handler.stream ChunkedWriteHandler]
     [io.netty.handler.timeout
      IdleState
      IdleStateEvent]
+    [io.netty.handler.codec
+     TooLongFrameException]
     [io.netty.handler.codec.http
      DefaultFullHttpResponse
      HttpContent HttpHeaders HttpUtil
@@ -52,7 +53,6 @@
      TextWebSocketFrame
      BinaryWebSocketFrame
      CloseWebSocketFrame
-     WebSocketFrame
      WebSocketFrameAggregator]
     [io.netty.handler.codec.http.websocketx.extensions.compression
      WebSocketServerCompressionHandler]
@@ -215,15 +215,31 @@
 (defn invalid-request? [^HttpRequest req]
   (-> req .decoderResult .isFailure))
 
-(defn reject-invalid-request [ctx ^HttpRequest req]
-  (d/chain
-    (netty/write-and-flush ctx
-      (DefaultFullHttpResponse.
-        HttpVersion/HTTP_1_1
+(defn- ^HttpResponseStatus cause->status [^Throwable cause]
+  (if (instance? TooLongFrameException cause)
+    (let [message (.getMessage cause)]
+      (cond
+        (.startsWith message "An HTTP line is larger than")
         HttpResponseStatus/REQUEST_URI_TOO_LONG
-        (-> req .decoderResult .cause .getMessage netty/to-byte-buf)))
-    netty/wrap-future
-    (fn [_] (netty/close ctx))))
+
+        (.startsWith message "HTTP header is larger than")
+        HttpResponseStatus/REQUEST_HEADER_FIELDS_TOO_LARGE
+
+        :else
+        HttpResponseStatus/BAD_REQUEST))
+    HttpResponseStatus/BAD_REQUEST))
+
+(defn reject-invalid-request [ctx ^HttpRequest req]
+  (let [cause (-> req .decoderResult .cause)
+        status (cause->status cause)]
+    (d/chain
+     (netty/write-and-flush ctx
+                            (DefaultFullHttpResponse.
+                             HttpVersion/HTTP_1_1
+                             status
+                             (-> cause .getMessage netty/to-byte-buf)))
+     netty/wrap-future
+     (fn [_] (netty/close ctx)))))
 
 (defn ring-handler
   [ssl? handler rejected-handler executor buffer-capacity]

test/aleph/http_test.clj

@@ -1,11 +1,12 @@
 (ns aleph.http-test
-  (:use
-    [clojure test])
   (:require
+    [clojure.string :as str]
+    [clojure.test :refer [deftest testing is]]
     [aleph
      [http :as http]
      [netty :as netty]
-     [flow :as flow]]
+     [flow :as flow]
+     [tcp :as tcp]]
     [byte-streams :as bs]
     [manifold.deferred :as d]
     [manifold.stream :as s])
@@ -253,9 +254,25 @@
         (apply d/zip)
         deref))))
 
-(deftest test-overly-long-request
+(deftest test-overly-long-url
+  (let [long-url (apply str "http://localhost:" port  "/" (repeat 1e4 "a"))]
+    (with-handler basic-handler
+      (is (= 414 (:status @(http-get long-url)))))))
+
+(deftest test-overly-long-header
+  (let [url (str "http://localhost:" port)
+        long-header-value (apply str (repeat 1e5 "a"))
+        opts {:headers {"X-Long" long-header-value}}]
+    (with-handler basic-handler
+      (is (= 431 (:status @(http-get url opts)))))))
+
+(deftest test-invalid-http-version-format
   (with-handler basic-handler
-    (= 414 @(http-get (apply str "http://localhost:" port  "/" (repeat 1e4 "a"))))))
+    (let [client @(tcp/client {:host "localhost" :port port})
+          _ @(s/put! client "GET / HTTP-1,1\r\n\r\n")
+          response (bs/to-string @(s/take! client))]
+      (is (str/starts-with? response "HTTP/1.1 400"))
+      (s/close! client))))
 
 (deftest test-echo
   (with-handler basic-handler