Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch FileNotFoundException when sending file body #471

Merged
merged 6 commits into from
Aug 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 77 additions & 50 deletions src/aleph/http/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@
[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"
Expand Down Expand Up @@ -266,6 +270,17 @@
(defn chunked-writer-enabled? [^Channel ch]
(some? (-> ch netty/channel .pipeline (.get ChunkedWriteHandler))))

(def default-error-response
{:status 500
:headers {"content-type" "text/plain"}
:body "Internal server error"})

;; Logs exception and returns default 500 response
;; not to expose internal logic to the client
(defn error-response [^Throwable e]
(log/error e "error in HTTP handler")
default-error-response)

(defn send-streaming-body [ch ^HttpMessage msg body]

(HttpUtil/setTransferEncodingChunked msg (boolean (not (has-content-length? msg))))
Expand Down Expand Up @@ -432,6 +447,33 @@
(.close raf)
(.close fc)))))

(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]
(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 (-> default-error-response
(update :headers merge headers))
msg' (ring-response->netty-response resp)]
(send-contiguous-body ch msg' (:body resp))))

(defn send-chunked-file [ch ^HttpMessage msg ^HttpFile file]
(let [raf (RandomAccessFile. ^File (.-fd file) "r")
cf (ChunkedFile. raf
Expand All @@ -458,34 +500,17 @@
(defn send-file-body [ch ssl? ^HttpMessage msg ^HttpFile file]
(cond
ssl?
(send-streaming-body ch msg
(-> file
(bs/to-byte-buffers {:chunk-size (.-chunk-size file)})
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)

: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
Expand All @@ -502,35 +527,37 @@
(defn send-message
[ch keep-alive? ssl? ^HttpMessage msg body]

(let [f (cond

(or
(nil? body)
(identical? :aleph/omitted body)
(instance? String body)
(instance? ary-class body)
(instance? ByteBuffer body)
(instance? ByteBuf body))
(send-contiguous-body ch msg body)

(instance? ChunkedInput body)
(send-chunked-body ch msg body)

(instance? File body)
(send-file-body ch ssl? msg (http-file body))

(instance? Path body)
(send-file-body ch ssl? msg (http-file body))

(instance? HttpFile body)
(send-file-body ch ssl? msg body)

:else
(let [class-name (.getName (class body))]
(try
(send-streaming-body ch msg body)
(catch Throwable e
(log/error e "error sending body of type " class-name)))))]
(let [f (try
(cond
(or
(nil? body)
(identical? :aleph/omitted body)
(instance? String body)
(instance? ary-class body)
(instance? ByteBuffer body)
(instance? ByteBuf body))
(send-contiguous-body ch msg body)

(instance? ChunkedInput body)
(send-chunked-body ch msg body)

(instance? File body)
(send-file-body ch ssl? msg (http-file body))

(instance? Path body)
(send-file-body ch ssl? msg (http-file body))

(instance? HttpFile body)
(send-file-body ch ssl? msg body)

:else
(send-streaming-body ch msg body))
(catch Throwable e
(let [class-name (.getName (class 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))
Expand Down
33 changes: 11 additions & 22 deletions src/aleph/http/server.clj
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,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]
Expand All @@ -118,27 +107,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)
Expand Down Expand Up @@ -169,7 +158,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"})))
Expand All @@ -178,15 +167,15 @@
(try
(handler req')
(catch Throwable e
(error-response e))))]
(http/error-response e))))]

(-> previous-response
(d/chain'
netty/wrap-future
(fn [_]
(netty/release req)
(-> rsp
(d/catch' error-response)
(d/catch' http/error-response)
(d/chain'
(fn [rsp]
(when (not (-> req' ^AtomicBoolean (.websocket?) .get))
Expand Down
20 changes: 20 additions & 0 deletions test/aleph/http_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,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 []
Expand Down