Changes based on review 3

- Reformat docstring
- Ensure we're not catching InvalidFileNameExceptions thrown by other
  middleware.
- Move try/catch down the callstack to reduce code duplication.

Author: expez

ring-core/src/ring/middleware/multipart_params.clj

@@ -112,33 +112,38 @@
          func  (load-var store)]
      (func))))
 
+(defn default-invalid-filename-handler [request e]
+  (response/bad-request (.getMessage e)))
+
 (defn multipart-params-request
   "Adds :multipart-params and :params keys to request.
   See: wrap-multipart-params."
   {:added "1.2"}
   ([request]
    (multipart-params-request request {}))
   ([request options]
-   (let [store           (or (:store options) @default-store)
-         forced-encoding (:encoding options)
-         req-encoding    (or forced-encoding
-                             (:fallback-encoding options)
-                             (req/character-encoding request)
-                             "UTF-8")
-         progress        (:progress-fn options)
-         params          (if (multipart-form? request)
-                           (parse-multipart-params request
-                                                   req-encoding
-                                                   forced-encoding
-                                                   store
-                                                   progress)
-                           {})]
-     (merge-with merge request
-                 {:multipart-params params}
-                 {:params params}))))
-
-(defn default-invalid-filename-handler [request e]
-  (response/bad-request (.getMessage e)))
+   (try
+     (let [store           (or (:store options) @default-store)
+           forced-encoding (:encoding options)
+           req-encoding    (or forced-encoding
+                               (:fallback-encoding options)
+                               (req/character-encoding request)
+                               "UTF-8")
+           progress        (:progress-fn options)
+           params          (if (multipart-form? request)
+                             (parse-multipart-params request
+                                                     req-encoding
+                                                     forced-encoding
+                                                     store
+                                                     progress)
+                             {})]
+       (merge-with merge request
+                   {:multipart-params params}
+                   {:params params}))
+     (catch InvalidFileNameException e
+       (let [invalid-filename-handler
+             (:invalid-filename-handler options default-invalid-filename-handler)]
+         (with-meta (invalid-filename-handler request e) {::error? true}))))))
 
 (defn wrap-multipart-params
   "Middleware to parse multipart parameters from a request. Adds the
@@ -149,47 +154,48 @@
 
   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 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."
+  :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 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."
   ([handler]
    (wrap-multipart-params handler {}))
   ([handler options]
    (fn
      ([request]
-      (let [invalid-file-name-handler
-            (:invalid-filename-handler options default-invalid-filename-handler)]
-        (try
-          (handler (multipart-params-request request options))
-          (catch InvalidFileNameException e
-            (invalid-file-name-handler request e)))))
+      (let [multipart-params-request (multipart-params-request request options)]
+        (if (contains? (meta multipart-params-request) ::error?)
+          multipart-params-request
+          (handler multipart-params-request))))
      ([request respond raise]
-      (let [invalid-file-name-handler
-            (:invalid-filename-handler options default-invalid-filename-handler)]
-        (try
-          (handler (multipart-params-request request options) respond raise)
-          (catch InvalidFileNameException e
-            (respond (invalid-file-name-handler request e)))))))))
+      (let [multipart-params-request (multipart-params-request request options)]
+        (if (contains? (meta multipart-params-request) ::error?)
+          (handler multipart-params-request respond raise)
+          (respond multipart-params-request)))))))