diff --git a/ring-core/src/ring/middleware/multipart_params.clj b/ring-core/src/ring/middleware/multipart_params.clj index e2831f59..b65772ad 100644 --- a/ring-core/src/ring/middleware/multipart_params.clj +++ b/ring-core/src/ring/middleware/multipart_params.clj @@ -9,11 +9,13 @@ ring.middleware.multipart-params.temp-file/temp-file-store" (:require [ring.util.codec :refer [assoc-conj]] [ring.util.request :as req] + [ring.util.response :as response] [ring.util.parsing :refer [re-charset]]) (:import [org.apache.commons.fileupload UploadContext FileItemIterator FileItemStream FileUpload + InvalidFileNameException ProgressListener] [org.apache.commons.io IOUtils])) (defn- progress-listener @@ -135,41 +137,73 @@ {:multipart-params params} {:params params})))) +(defn default-invalid-filename-handler + ([request] + (response/bad-request + (str "Invalid filename in multipart upload: " + (::invalid-filename request)))) + ([request respond raise] + (respond (default-invalid-filename-handler request)))) + +(defn- add-invalid-filename-to-req + [request ^InvalidFileNameException invalid-filename-exception] + (assoc request ::invalid-filename (.getName invalid-filename-exception))) + (defn wrap-multipart-params - "Middleware to parse multipart parameters from a request. Adds the - following keys to the request map: + "Middleware to parse multipart parameters from a request. Adds the following + keys to the request map: :multipart-params - a map of multipart parameters :params - a merged map of all types of parameter - The following options are accepted - - :encoding - character encoding to use for multipart parsing. - Overrides the encoding specified in the request. If not - specified, uses the encoding specified in a part named - \"_charset_\", or the content type for each part, or - request character encoding if the part has no encoding, - or \"UTF-8\" if no request character encoding is set. - - :fallback-encoding - specifies the character encoding used in parsing if a - part of the request does not specify encoding in its - content type or no part named \"_charset_\" is present. - Has no effect if :encoding is also set. - - :store - a function that stores a file upload. The function - should expect a map with :filename, :content-type and - :stream keys, and its return value will be used as the - value for the parameter in the multipart parameter map. - The default storage function is the temp-file-store. - - :progress-fn - a function that gets called during uploads. The - function should expect four parameters: request, - bytes-read, content-length, and item-count." + The following options are accepted: + + :encoding + - Character encoding to use for multipart parsing. Overrides the encoding + specified in the request. If not specified,uses the encoding specified in a + part named \"_charset_\", or the content type for each part, or request + character encoding if the part has no encoding, or \"UTF-8\" if no request + character encoding is set. + + :fallback-encoding + - Specifies the character encoding used in parsing if a part of the request + does not specify encoding in its content type or no part named \"_charset_\" + is present. Has no effect if :encoding is also set. + + :store + - A function that stores a file upload. The function should expect a map with + :filename, :content-type and :stream keys, and its return value will be used + as the value for the parameter in the multipart parameter map. The default + storage function is the temp-file-store. + + :progress-fn + - A function that gets called during uploads. The function should expect four + parameters: request, bytes-read, content-length, and item-count. + + :invalid-filename-handler + - 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, which contains the name of the invalid file." ([handler] (wrap-multipart-params handler {})) ([handler options] - (fn - ([request] - (handler (multipart-params-request request options))) - ([request respond raise] - (handler (multipart-params-request request options) respond raise))))) + (let [invalid-filename-handler + (:invalid-filename-handler options default-invalid-filename-handler)] + (fn ([request] + (let [req-or-ex (try + (multipart-params-request request options) + (catch InvalidFileNameException ex ex))] + (if (instance? InvalidFileNameException req-or-ex) + (-> request + (add-invalid-filename-to-req req-or-ex) + invalid-filename-handler) + (handler req-or-ex)))) + ([request respond raise] + (let [req-or-ex (try + (multipart-params-request request options) + (catch InvalidFileNameException ex ex))] + (if (instance? InvalidFileNameException req-or-ex) + (-> request + (add-invalid-filename-to-req req-or-ex) + (invalid-filename-handler respond raise)) + (handler req-or-ex respond raise)))))))) diff --git a/ring-core/test/ring/middleware/test/multipart_params.clj b/ring-core/test/ring/middleware/test/multipart_params.clj index 88b3aac1..a194d5c8 100644 --- a/ring-core/test/ring/middleware/test/multipart_params.clj +++ b/ring-core/test/ring/middleware/test/multipart_params.clj @@ -1,8 +1,9 @@ (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]])) + [ring.util.io :refer [string-input-stream]]) + (:import org.apache.commons.fileupload.InvalidFileNameException)) (defn string-store [item] (-> (select-keys item [:filename :content-type]) @@ -175,3 +176,34 @@ :body (string-input-stream form-body "ISO-8859-15")} request* (multipart-params-request request {:fallback-encoding "ISO-8859-15"})] (is (= (get-in request* [:multipart-params "foo"]) "äÄÖöÅå€")))) + +(deftest wrap-multipart-params-error-test + (let [invalid-filename (str "foo" \u0000 ".bar") + form-body (str "--XXXX\r\n" + "Content-Disposition: form-data;" + "name=foo; filename=" invalid-filename "\r\n" + "Content-Type: text/plain\r\n\r\n" + "foo\r\n" + "--XXXX--") + request {:headers {"content-type" + (str "multipart/form-data; boundary=XXXX; charset=US-ASCII")} + :body (string-input-stream form-body "ISO-8859-1")} + invalid-filename-exception + (try + (org.apache.commons.fileupload.util.Streams/checkFileName invalid-filename) + (catch Exception e (println (.getName e)) e)) + err-response (default-invalid-filename-handler + {::multipart-params/invalid-filename + (.getName invalid-filename-exception)})] + (testing "Synchronous error response" + (let [handler (wrap-multipart-params identity)] + (is (= err-response (handler request))))) + (testing "Asynchronous error response" + (let [handler (wrap-multipart-params (fn [req respond _] + (respond req))) + response (promise) + exception (promise) + request (assoc request :body (string-input-stream form-body "ISO-8859-1"))] + (handler request response exception) + (is (= err-response (deref response 2000 :timed-out))) + (is (not (realized? exception)))))))