Merge branch 'better-multipart'

@@ -1,27 +1,19 @@
 (ns ring.middleware.multipart-params
   "Parse multipart upload into params."
   (:use [ring.middleware.params :only (assoc-param)])
-  (:import (org.apache.commons.fileupload
-             FileUpload RequestContext)
-           (org.apache.commons.fileupload.disk
-             DiskFileItemFactory
-             DiskFileItem)))
+  (:import [org.apache.commons.fileupload.util Streams]
+           [org.apache.commons.fileupload
+             RequestContext
+             FileItemIterator
+             FileItemStream
+             FileUpload]))
 
 (defn- multipart-form?
   "Does a request have a multipart form?"
   [request]
   (if-let [^String content-type (:content-type request)]
     (.startsWith content-type "multipart/form-data")))
 
-(def ^{:private true
-       :type FileUpload
-       :doc "Uploader class to save multipart form values to temporary files."}
-  file-upload
-  (FileUpload.
-    (doto (DiskFileItemFactory.)
-      (.setSizeThreshold -1)
-      (.setFileCleaningTracker nil))))
-
 (defn- request-context
   "Create a RequestContext object from a request map."
   {:tag RequestContext}
@@ -32,47 +24,73 @@
     (getCharacterEncoding [this] encoding)
     (getInputStream [this]       (:body request))))
 
-(defn- file-map
-  "Create a file map from a DiskFileItem."
-  [^DiskFileItem item]
-  (with-meta
-    {:filename     (.getName item)
-     :size         (.getSize item)
-     :content-type (.getContentType item)
-     :tempfile     (.getStoreLocation item)}
-    {:disk-file-item item}))
+(defn- file-item-iterator-seq
+  "Create a lazy seq from a FileItemIterator instance."
+  [^FileItemIterator it]
+  (lazy-seq
+    (if (.hasNext it)
+      (cons (.next it) (file-item-iterator-seq it)))))
+
+(defn- file-item-seq
+  "Create a seq of FileItem instances from a request context."
+  [context]
+  (file-item-iterator-seq
+    (.getItemIterator (FileUpload.) context)))
+
+(defn- parse-file-item
+  "Parse a FileItemStream into a parameter value. If the request is a file the
+  supplied store function is used to save it."
+  [^FileItemStream item store]
+  (if (.isFormField item)
+    (Streams/asString (.openStream item))
+    (store {:filename     (.getName item)
+            :content-type (.getContentType item)
+            :stream       (.openStream item)})))
 
-(defn parse-multipart-params
+(defn- parse-multipart-params
   "Parse a map of multipart parameters from the request."
-  [request encoding]
-  (reduce
-    (fn [param-map, ^DiskFileItem item]
-      (assoc-param param-map
-        (.getFieldName item)
-        (if (.isFormField item)
-          (if (.get item) (.getString item encoding))
-          (file-map item))))
-    {}
-    (.parseRequest
-       file-upload
-       (request-context request encoding))))
+  [request encoding store]
+  (into {}
+    (for [item (file-item-seq (request-context request encoding))]
+      [(.getFieldName item)
+       (parse-file-item item store)])))
+
+(defn- load-var
+  "Returns the var named by the supplied symbol, or nil if not found. Attempts
+  to load the var namespace on the fly if not already loaded."
+  [sym]
+  (require (symbol (namespace sym)))
+  (find-var sym))
+
+(def default-store
+  'ring.middleware.multipart-params.temp-file/temp-file-store)
 
 (defn wrap-multipart-params
   "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
-  Takes an optional configuration map. Recognized keys are:
+
+  This middleware takes an optional configuration map. Recognized keys are:
+
     :encoding - character encoding to use for multipart parsing. If not
                 specified, uses the request character encoding, or \"UTF-8\"
-                if no request character encoding is set."
+                if no request character encoding is 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."
   [handler & [opts]]
   (fn [request]
     (let [encoding (or (:encoding opts)
                        (:character-encoding request)
                        "UTF-8")
+          store    (or (:store opts)
+                       (load-var default-store))
           params   (if (multipart-form? request)
-                     (parse-multipart-params request encoding)
+                     (parse-multipart-params request encoding store)
                      {})
           request  (merge-with merge request
                      {:multipart-params params}

@@ -0,0 +1,8 @@
+(ns ring.middleware.multipart-params.byte-array
+  (:import org.apache.commons.io.IOUtils))
+
+(defn byte-array-store
+  "Stores multipart file parameters as an array of bytes."
+  [item]
+  (-> (select-keys item [:filename :content-type])
+      (assoc :bytes (IOUtils/toByteArray (:stream item)))))

@@ -0,0 +1,42 @@
+(ns ring.middleware.multipart-params.temp-file
+  (:require [clojure.java.io :as io])
+  (:import java.io.File))
+
+(defmacro ^{:private true} do-every [delay & body]
+  `(future
+     (while true
+       (Thread/sleep (* ~delay 1000))
+       (try ~@body
+            (catch Exception ex#)))))
+
+(defn- expired? [file expiry-time]
+  (< (.lastModified file)
+     (- (System/currentTimeMillis)
+        (* expiry-time 1000))))
+
+(defn- remove-old-files [file-set expiry-time]
+  (doseq [file @file-set]
+    (when (expired? file expiry-time)
+      (.delete file)
+      (swap! file-set disj file))))
+
+(defn- make-temp-file [file-set]
+  (let [temp-file (File/createTempFile "ring-multipart-" nil)]
+    (swap! file-set conj temp-file)
+    (.deleteOnExit temp-file)
+    temp-file))
+
+(defn temp-file-store
+  "Stores multipart file parameters as a temporary file."
+  ([] (temp-file-store {:expire-in 3600}))
+  ([{:keys [expires-in]}]
+     (fn [item]
+       (let [file-set  (atom #{})
+             temp-file (make-temp-file file-set)]
+         (io/copy (:stream item) temp-file)
+         (when expires-in
+           (do-every expires-in
+             (remove-old-files file-set expires-in)))
+         (-> (select-keys item [:filename :content-type])
+             (assoc :tempfile temp-file
+                    :size (.length temp-file)))))))

@@ -0,0 +1,13 @@
+(ns ring.middleware.multipart-params.byte-array-test
+  (:use clojure.test
+        ring.util.test
+        ring.middleware.multipart-params.byte-array))
+
+(deftest test-byte-array-store
+  (let [result (byte-array-store
+                {:filename "foo.txt"
+                 :content-type "text/plain"
+                 :stream (string-input-stream "foo")})]
+    (is (= (:filename result) "foo.txt"))
+    (is (= (:content-type result) "text/plain"))
+    (is (= (String. (:bytes result)) "foo"))))

@@ -0,0 +1,27 @@
+(ns ring.middleware.multipart-params.temp-file-test
+  (:use clojure.test
+        ring.util.test
+        ring.middleware.multipart-params.temp-file))
+
+(deftest test-temp-file-store
+  (let [store  (temp-file-store)
+        result (store
+                {:filename "foo.txt"
+                 :content-type "text/plain"
+                 :stream (string-input-stream "foo")})]
+    (is (= (:filename result) "foo.txt"))
+    (is (= (:content-type result) "text/plain"))
+    (is (= (:size result) 3))
+    (is (instance? java.io.File (:tempfile result)))
+    (is (.exists (:tempfile result)))
+    (is (= (slurp (:tempfile result)) "foo"))))
+
+(deftest test-temp-file-expiry
+  (let [store  (temp-file-store {:expires-in 2})
+        result (store
+                {:filename "foo.txt"
+                 :content-type "text/plain"
+                 :stream (string-input-stream "foo")})]
+    (is (.exists (:tempfile result)))
+    (Thread/sleep 2500)
+    (is (not (.exists (:tempfile result))))))

@@ -1,36 +1,33 @@
 (ns ring.middleware.multipart-params-test
   (:use clojure.test
-        ring.middleware.multipart-params)
-  (:require [ring.util.test :as tu])
-  (:import java.io.File))
+        ring.middleware.multipart-params
+        ring.util.test)
+  (:import java.io.InputStream))
 
-(def ^{:private true}
-  upload-content-type
-  "multipart/form-data; boundary=----WebKitFormBoundaryAyGUY6aMxOI6UF5s")
-
-(def ^{:private true}
-  upload-content-length
-  188)
-
-(def ^{:private true}
-  upload-body
-  (tu/string-input-stream
-    "------WebKitFormBoundaryAyGUY6aMxOI6UF5s\r\nContent-Disposition: form-data; name=\"upload\"; filename=\"test.txt\"\r\nContent-Type: text/plain\r\n\r\nfoo\r\n\r\n------WebKitFormBoundaryAyGUY6aMxOI6UF5s--"))
-
-(def ^{:private true}
-  wrapped-echo
-  (wrap-multipart-params identity))
+(defn string-store [item]
+  (-> (select-keys item [:filename :content-type])
+      (assoc :content (slurp (:stream item)))))
 
 (deftest test-wrap-multipart-params
-  (let [req {:content-type   upload-content-type
-             :content-length upload-content-length
-             :body           upload-body
-             :params         {"foo" "bar"}}
-        resp (wrapped-echo req)]
-    (is (= "bar" (get-in resp [:params "foo"])))
-    (let [upload (get-in resp [:params "upload"])]
-      (is (= "test.txt" (:filename upload)))
-      (is (= 5 (:size upload)))
-      (is (= "text/plain" (:content-type upload)))
-      (is (instance? File (:tempfile upload)))
-      (is (= "foo\r\n" (slurp (:tempfile upload)))))))
+  (let [form-body (str "--XXXX\r\n"
+                       "Content-Disposition: form-data;"
+                       "name=\"upload\"; filename=\"test.txt\"\r\n"
+                       "Content-Type: text/plain\r\n\r\n"
+                       "foo\r\n"
+                       "--XXXX\r\n"
+                       "Content-Disposition: form-data;"
+                       "name=\"baz\"\r\n\r\n"
+                       "qux\r\n"
+                       "--XXXX--")
+        handler (wrap-multipart-params identity {:store string-store})
+        request {:content-type "multipart/form-data; boundary=XXXX"
+                 :content-length (count form-body)
+                 :params {"foo" "bar"}
+                 :body (string-input-stream form-body)}
+        response (handler request)]
+    (is (= (get-in response [:params "foo"]) "bar"))
+    (is (= (get-in response [:params "baz"]) "qux"))
+    (let [upload (get-in response [:params "upload"])]
+      (is (= (:filename upload)     "test.txt"))
+      (is (= (:content-type upload) "text/plain"))
+      (is (= (:content upload)      "foo")))))