From 9806c3e66a6cf02bbf8ea878b753fc4907603bf8 Mon Sep 17 00:00:00 2001 From: Alexey Kachayev Date: Wed, 30 Jan 2019 22:49:27 +0200 Subject: [PATCH 1/5] Added test cases to show that non existing file just hangs --- test/aleph/http_test.clj | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/aleph/http_test.clj b/test/aleph/http_test.clj index 11097492..2016e09f 100644 --- a/test/aleph/http_test.clj +++ b/test/aleph/http_test.clj @@ -353,6 +353,26 @@ :body bs/to-string))))) +(defn non-existing-file-handler [req] + {:status 200 + :body (File. "this-file-does-not-exist.sure")}) + +(defn- assert-file-error [] + (is (= 500 (-> (http/get (str "http://localhost:" port) + {:throw-exceptions? false}) + (d/timeout! 1e3) + deref + :status)))) + +(deftest test-sending-non-existing-file + (testing "sending FileRegion" + (with-handler non-existing-file-handler + (assert-file-error))) + + (testing "sending chunked body" + (with-compressed-handler non-existing-file-handler + (assert-file-error)))) + ;;; (defn get-netty-client-event-threads [] From c9cb652a9c3d976a2fc306119c4f4881705ca9de Mon Sep 17 00:00:00 2001 From: Alexey Kachayev Date: Wed, 30 Jan 2019 23:50:19 +0200 Subject: [PATCH 2/5] Move error response processing to http/core, wrap FileNotFoundException --- src/aleph/http/core.clj | 86 +++++++++++++++++++++++++++------------ src/aleph/http/server.clj | 33 +++++---------- 2 files changed, 70 insertions(+), 49 deletions(-) diff --git a/src/aleph/http/core.clj b/src/aleph/http/core.clj index 3c25be84..1848a6c4 100644 --- a/src/aleph/http/core.clj +++ b/src/aleph/http/core.clj @@ -39,13 +39,20 @@ [java.io File RandomAccessFile - Closeable] + Closeable + FileNotFoundException + StringWriter + PrintWriter] [java.util.concurrent ConcurrentHashMap TimeUnit] [java.util.concurrent.atomic AtomicBoolean])) +(def ^CharSequence server-name (HttpHeaders/newEntity "Server")) +(def ^CharSequence connection-name (HttpHeaders/newEntity "Connection")) +(def ^CharSequence date-name (HttpHeaders/newEntity "Date")) + (def non-standard-keys (let [ks ["Content-MD5" "ETag" @@ -250,6 +257,14 @@ (defn chunked-writer-enabled? [^Channel ch] (some? (-> ch netty/channel .pipeline (.get ChunkedWriteHandler)))) +(defn error-response [^Throwable e] + (log/error e "error in HTTP handler") + {:status 500 + :headers {"content-type" "text/plain"} + :body (let [w (StringWriter.)] + (.printStackTrace e (PrintWriter. w)) + (str w))}) + (defn send-streaming-body [ch ^HttpMessage msg body] (HttpUtil/setTransferEncodingChunked msg (boolean (not (has-content-length? msg)))) @@ -323,11 +338,41 @@ (netty/write-and-flush ch empty-last-content))) +(defn send-contiguous-body [ch ^HttpMessage msg body] + (let [omitted? (identical? :aleph/omitted body) + body (if (or (nil? body) omitted?) + empty-last-content + (DefaultLastHttpContent. (netty/to-byte-buf ch body))) + length (-> ^HttpContent body .content .readableBytes)] + + (when-not omitted? + (if (instance? HttpResponse msg) + (let [code (-> ^HttpResponse msg .status .code)] + (when-not (or (<= 100 code 199) (= 204 code)) + (try-set-content-length! msg length))) + (try-set-content-length! msg length))) + + (netty/write ch msg) + (netty/write-and-flush ch body))) + +(defn send-internal-error [ch ^HttpResponse msg ^Throwable e] + (let [raw-headers (.headers msg) + headers {:server (.get raw-headers server-name) + :connection (.get raw-headers connection-name) + :date (.get raw-headers date-name)} + resp (-> (error-response e) + (update :headers merge headers)) + msg' (ring-response->netty-response resp)] + (send-contiguous-body ch msg' (:body resp)))) + (defn send-chunked-file [ch ^HttpMessage msg ^File file] - (let [raf (RandomAccessFile. file "r") - len (.length raf) - ci (HttpChunkedInput. (ChunkedFile. raf))] - (try-set-content-length! msg len) + (when-let [ci (try + (let [raf (RandomAccessFile. file "r")] + (try-set-content-length! msg (.length raf)) + (HttpChunkedInput. (ChunkedFile. raf))) + (catch FileNotFoundException e + (send-internal-error ch msg e) + nil))] (netty/write ch msg) (netty/write-and-flush ch ci))) @@ -336,11 +381,15 @@ (netty/write-and-flush ch body)) (defn send-file-region [ch ^HttpMessage msg ^File file] - (let [raf (RandomAccessFile. file "r") - len (.length raf) - fc (.getChannel raf) - fr (DefaultFileRegion. fc 0 len)] - (try-set-content-length! msg len) + (when-let [fr (try + (let [raf (RandomAccessFile. file "r") + len (.length raf) + fc (.getChannel raf)] + (try-set-content-length! msg len) + (DefaultFileRegion. fc 0 len)) + (catch FileNotFoundException e + (send-internal-error ch msg e) + nil))] (netty/write ch msg) (netty/write ch fr) (netty/write-and-flush ch empty-last-content))) @@ -359,23 +408,6 @@ :else (send-file-region ch msg file))) -(defn send-contiguous-body [ch ^HttpMessage msg body] - (let [omitted? (identical? :aleph/omitted body) - body (if (or (nil? body) omitted?) - empty-last-content - (DefaultLastHttpContent. (netty/to-byte-buf ch body))) - length (-> ^HttpContent body .content .readableBytes)] - - (when-not omitted? - (if (instance? HttpResponse msg) - (let [code (-> ^HttpResponse msg .status .code)] - (when-not (or (<= 100 code 199) (= 204 code)) - (try-set-content-length! msg length))) - (try-set-content-length! msg length))) - - (netty/write ch msg) - (netty/write-and-flush ch body))) - (let [ary-class (class (byte-array 0)) ;; extracted to make `send-message` more inlineable diff --git a/src/aleph/http/server.clj b/src/aleph/http/server.clj index 4175ff56..c05eac61 100644 --- a/src/aleph/http/server.clj +++ b/src/aleph/http/server.clj @@ -94,18 +94,7 @@ TimeUnit/MILLISECONDS) (.get ref)))) -(defn error-response [^Throwable e] - (log/error e "error in HTTP handler") - {:status 500 - :headers {"content-type" "text/plain"} - :body (let [w (java.io.StringWriter.)] - (.printStackTrace e (java.io.PrintWriter. w)) - (str w))}) - -(let [[server-name connection-name date-name] - (map #(HttpHeaders/newEntity %) ["Server" "Connection" "Date"]) - - [server-value keep-alive-value close-value] +(let [[server-value keep-alive-value close-value] (map #(HttpHeaders/newEntity %) ["Aleph/0.4.6" "Keep-Alive" "Close"])] (defn send-response [^ChannelHandlerContext ctx keep-alive? ssl? rsp] @@ -114,27 +103,27 @@ [(http/ring-response->netty-response rsp) (get rsp :body)] (catch Throwable e - (let [rsp (error-response e)] + (let [rsp (http/error-response e)] [(http/ring-response->netty-response rsp) (get rsp :body)])))] (netty/safe-execute ctx (let [headers (.headers rsp)] - (when-not (.contains headers ^CharSequence server-name) - (.set headers ^CharSequence server-name server-value)) + (when-not (.contains headers ^CharSequence http/server-name) + (.set headers ^CharSequence http/server-name server-value)) - (when-not (.contains headers ^CharSequence date-name) - (.set headers ^CharSequence date-name (date-header-value ctx))) + (when-not (.contains headers ^CharSequence http/date-name) + (.set headers ^CharSequence http/date-name (date-header-value ctx))) - (.set headers ^CharSequence connection-name (if keep-alive? keep-alive-value close-value)) + (.set headers ^CharSequence http/connection-name (if keep-alive? keep-alive-value close-value)) (http/send-message ctx keep-alive? ssl? rsp body)))))) ;;; (defn invalid-value-response [req x] - (error-response + (http/error-response (IllegalArgumentException. (str "cannot treat " (pr-str x) @@ -165,7 +154,7 @@ (try (rejected-handler req') (catch Throwable e - (error-response e))) + (http/error-response e))) {:status 503 :headers {"content-type" "text/plain"} :body "503 Service Unavailable"}))) @@ -174,7 +163,7 @@ (try (handler req') (catch Throwable e - (error-response e))))] + (http/error-response e))))] (-> previous-response (d/chain' @@ -183,7 +172,7 @@ (netty/release req) (netty/release body) (-> rsp - (d/catch' error-response) + (d/catch' http/error-response) (d/chain' (fn [rsp] (when (not (-> req' ^AtomicBoolean (.websocket?) .get)) From 2e2d58794d360bb82d5cddba4b1f27326270b7e9 Mon Sep 17 00:00:00 2001 From: Alexey Kachayev Date: Wed, 30 Jan 2019 23:58:56 +0200 Subject: [PATCH 3/5] Catch FileNotFoundException when sending chunked body --- src/aleph/http/core.clj | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/aleph/http/core.clj b/src/aleph/http/core.clj index 1848a6c4..548bd74f 100644 --- a/src/aleph/http/core.clj +++ b/src/aleph/http/core.clj @@ -366,13 +366,10 @@ (send-contiguous-body ch msg' (:body resp)))) (defn send-chunked-file [ch ^HttpMessage msg ^File file] - (when-let [ci (try - (let [raf (RandomAccessFile. file "r")] - (try-set-content-length! msg (.length raf)) - (HttpChunkedInput. (ChunkedFile. raf))) - (catch FileNotFoundException e - (send-internal-error ch msg e) - nil))] + (let [raf (RandomAccessFile. file "r") + len (.length raf) + ci (HttpChunkedInput. (ChunkedFile. raf))] + (try-set-content-length! msg len) (netty/write ch msg) (netty/write-and-flush ch ci))) @@ -381,15 +378,11 @@ (netty/write-and-flush ch body)) (defn send-file-region [ch ^HttpMessage msg ^File file] - (when-let [fr (try - (let [raf (RandomAccessFile. file "r") - len (.length raf) - fc (.getChannel raf)] - (try-set-content-length! msg len) - (DefaultFileRegion. fc 0 len)) - (catch FileNotFoundException e - (send-internal-error ch msg e) - nil))] + (let [raf (RandomAccessFile. file "r") + len (.length raf) + fc (.getChannel raf) + fr (DefaultFileRegion. fc 0 len)] + (try-set-content-length! msg len) (netty/write ch msg) (netty/write ch fr) (netty/write-and-flush ch empty-last-content))) @@ -397,10 +390,10 @@ (defn send-file-body [ch ssl? ^HttpMessage msg ^File file] (cond ssl? - (send-streaming-body ch msg - (-> file - (bs/to-byte-buffers {:chunk-size 1e6}) - s/->source)) + (let [source (-> file + (bs/to-byte-buffers {:chunk-size 1e6}) + s/->source)] + (send-streaming-body ch msg source)) (chunked-writer-enabled? ch) (send-chunked-file ch msg file) @@ -408,6 +401,12 @@ :else (send-file-region ch msg file))) +(defn safe-send-file-body [ch ssl? ^HttpMessage msg ^File file] + (try + (send-file-body ch ssl? msg file) + (catch FileNotFoundException e + (send-internal-error ch msg e)))) + (let [ary-class (class (byte-array 0)) ;; extracted to make `send-message` more inlineable @@ -439,7 +438,7 @@ (send-chunked-body ch msg body) (instance? File body) - (send-file-body ch ssl? msg body) + (safe-send-file-body ch ssl? msg body) :else (let [class-name (.getName (class body))] From 4031c95ef7b305e303e36a34ffe8568a1978bf36 Mon Sep 17 00:00:00 2001 From: Oleksii Kachaiev Date: Sun, 21 Apr 2019 22:15:05 -0700 Subject: [PATCH 4/5] Fix error with sending internal error --- src/aleph/http/core.clj | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/aleph/http/core.clj b/src/aleph/http/core.clj index 320abf20..64c6a360 100644 --- a/src/aleph/http/core.clj +++ b/src/aleph/http/core.clj @@ -558,8 +558,10 @@ (send-streaming-body ch msg body)) (catch Throwable e (let [class-name (.getName (class body))] - (log/error e "error sending body of type " class-name)) - (send-internal-error ch msg body)))] + (log/errorf "error sending body of type %s: %s" + class-name + (.getMessage ^Throwable e))) + (send-internal-error ch msg)))] (when-not keep-alive? (handle-cleanup ch f)) From 9ccb973069d62506e120825d9d3d8ff49892aa70 Mon Sep 17 00:00:00 2001 From: Oleksii Kachaiev Date: Sun, 21 Apr 2019 22:18:19 -0700 Subject: [PATCH 5/5] Remove unused imports --- src/aleph/http/core.clj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/aleph/http/core.clj b/src/aleph/http/core.clj index 64c6a360..6739b0b9 100644 --- a/src/aleph/http/core.clj +++ b/src/aleph/http/core.clj @@ -50,10 +50,6 @@ [java.io File RandomAccessFile - Closeable - FileNotFoundException - StringWriter - PrintWriter Closeable] [java.nio.file Path] [java.nio.channels