Skip to content

Commit

Permalink
Tweaks after review 6
Browse files Browse the repository at this point in the history
- Only catch InvalidFileNameException.
- Test for InvalidFileNameException instead of Throwable.
- Add CPS style arities to default-invalid-filename-handler
- Add the exception to the request, so it's available for the
  invalid-filename-handler.
- Update docstring.
- Update tests.
  • Loading branch information
expez committed Sep 26, 2018
1 parent 3f988b4 commit 6386ed3
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 15 deletions.
27 changes: 16 additions & 11 deletions ring-core/src/ring/middleware/multipart_params.clj
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,11 @@
{:multipart-params params}
{:params params}))))

(defn default-invalid-filename-handler [request e]
(response/bad-request (.getMessage e)))
(defn default-invalid-filename-handler
([request]
(-> request ::invalid-filename-exception .getMessage response/bad-request))
([request respond raise]
(respond (default-invalid-filename-handler request))))

(defn wrap-multipart-params
"Middleware to parse multipart parameters from a request. Adds the following
Expand Down Expand Up @@ -172,9 +175,9 @@
parameters: request, bytes-read, content-length, and item-count.
:invalid-filename-handler
- A function that gets called when the file being uploaded has an invalid name.
The function should expect two parameters: request and an exception of type
InvalidFileNameException. It should return a ring response."
- A handler that gets called when the file being uploaded has an invalid name.
When this handler receives the request it can expect one additional key,
::invalid-filename-exception, which contains an InvalidFileNameException."
([handler]
(wrap-multipart-params handler {}))
([handler options]
Expand All @@ -183,14 +186,16 @@
(fn ([request]
(let [req-or-ex (try
(multipart-params-request request options)
(catch Exception ex ex))]
(if (instance? Throwable req-or-ex)
(invalid-filename-handler request req-or-ex)
(catch InvalidFileNameException ex ex))]
(if (instance? InvalidFileNameException req-or-ex)
(invalid-filename-handler
(assoc request ::invalid-filename-exception req-or-ex))
(handler req-or-ex))))
([request respond raise]
(let [req-or-ex (try
(multipart-params-request request options)
(catch Exception ex ex))]
(if (instance? Throwable req-or-ex)
(respond (invalid-filename-handler request req-or-ex))
(catch InvalidFileNameException ex ex))]
(if (instance? InvalidFileNameException req-or-ex)
(invalid-filename-handler
(assoc request ::invalid-filename-exception req-or-ex) respond raise)
(handler req-or-ex respond raise))))))))
8 changes: 4 additions & 4 deletions ring-core/test/ring/middleware/test/multipart_params.clj
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(ns ring.middleware.test.multipart-params
(:require [clojure.test :refer :all]
[ring.middleware.multipart-params :refer :all]
[ring.middleware.multipart-params :refer :all :as multipart-params]
[ring.middleware.multipart-params.byte-array :refer :all]
[ring.util.io :refer [string-input-stream]])
(:import org.apache.commons.fileupload.InvalidFileNameException))
Expand Down Expand Up @@ -191,10 +191,10 @@
invalid-filename-exception
(try
(org.apache.commons.fileupload.util.Streams/checkFileName invalid-filename)
(catch Exception e
e))
(catch Exception e e))
err-response (default-invalid-filename-handler
:unused-arg invalid-filename-exception)]
{::multipart-params/invalid-filename-exception
invalid-filename-exception})]
(testing "Synchronous error response"
(let [handler (wrap-multipart-params identity)]
(is (= err-response (handler request)))))
Expand Down

0 comments on commit 6386ed3

Please sign in to comment.