wrap-nested-params middleware broken with multipart form submits

[Tomas Sedovic]

Hey there,

I'm in a middle of moving a site from plain Compojure to Noir and I've
discovered this issue: when I do a submit from multipart form, params
in the `array[]` format are not processed properly.

Here's how to reproduce this:

$ lein noir new site
$ lein deps

Append this to site/src/site/views/common.clj:

(defpage "/multipart" []
(layout
[:form {:action "/submit" :enctype "multipart/form-data" :method
"POST"}
[:input {:type :text :name "mything[]" :value "10"}]
[:input {:type :text :name "mything[]" :value "20"}]
[:input {:type :submit}]]))

(defpage "/vanilla" []
(layout
[:form {:action "/submit" :method "POST"}
[:input {:type :text :name "mything[]" :value "10"}]
[:input {:type :text :name "mything[]" :value "20"}]
[:input {:type :submit}]]))

(defpage [:post "/submit"] {:as params}
(str params))


$ lein run

Now go to both /vanilla and /multipart URLs and press Submit.

In both cases, the `params` map should contain a `:mything` key
holding the values ["10", "20"]. With the multipart form, the vector
has only one value: ["20"].

Since this does work in Compojure, I presume it's supposed work with
Noir as well.

Here's the equivalent example in Compojure:

$ lein new compojure-site

Update project.clj:

:dependencies [[org.clojure/clojure "1.2.1"]
[compojure "0.6.4"]]
:dev-dependencies [[lein-ring "0.4.5"]]
:ring {:handler compojure-site.core/app}

$ lein deps

Edit src/compojure_site/core.clj:

(ns compojure-site.core
(:use compojure.core
hiccup.core)
(:require [compojure.route :as route]
[compojure.handler :as handler]))

(defn multipart [_]
(html
[:head [:title "multipart"]]
[:body
[:form {:action "/submit" :enctype "multipart/form-data" :method
"POST"}
[:input {:type :text :name "mything[]" :value "10"}]
[:input {:type :text :name "mything[]" :value "20"}]
[:input {:type :submit}]]]))

(defn vanilla [_]
(html
[:head [:title "vanilla"]]
[:body
[:form {:action "/submit" :method "POST"}
[:input {:type :text :name "mything[]" :value "10"}]
[:input {:type :text :name "mything[]" :value "20"}]
[:input {:type :submit}]]]))

(defn submit [request]
(str (request :params)))

(defroutes main-routes
(GET "/multipart" [] multipart)
(GET "/vanilla" [] vanilla)
(POST "/submit" [] submit)
(route/resources "/")
(route/not-found "Page not found"))

(def app
(handler/site main-routes))


$ lein ring server

Note that here both /vanilla and /multipart requests show the same
`:mything` value: ["10" "20"].

I looked into the Noir code but didn't find what's causing this yet.
Am I missing something or is this a legitimate bug?

Cheers,
Thomas

[Mark Rathwell]

> I looked into the Noir code but didn't find what's causing this yet.
> Am I missing something or is this a legitimate bug?

I can verify this issue. It works as expected with noir 1.1.0, but
works as you have indicated when using noir 1.2.0.

If I had to guess, I would guess maybe the wrap-url-decode middleware
is expecting application/x-www-form-urlencoded url, and is not
correctly handling multipart/form-data, completely baseless though.

- Mark

[Mark Rathwell]

> If I had to guess, I would guess maybe the wrap-url-decode middleware
> is expecting application/x-www-form-urlencoded url, and is not
> correctly handling multipart/form-data, completely baseless though.

Actually, this doesn't seem to be the case. Even when reducing the
middleware down to only wrap-request-map, wrap-params, and
wrap-multipart-params I get the same issue. Don't have time to look
further though.

- Mark

[Chris Granger]

This is broken at the compojure/ring level. I just tried your compojure code with compojure 0.6.5 and it doesn't work. I'm switching back to compojure 0.6.4 in  Noir 1.2.1-SNAPSHOT. I just tried it and it worked. Give 1.2.1-SNAPSHOT a try and let me know if it resolves it for you.

If it does, can you log an issue against compojure?

Cheers,
Chris.

[James Reeves]

Log it as an issue against Ring. The problem occurs in 0.3.11 and
above, and I can see the cause in the source code. I'll get it fixed
soon.

- James

[Chris Granger]

Thanks James :) I'll add an issue for it.

Cheers,
Chris.

[Tomas Sedovic]

Sorry about that. I didn't notice newer Compojure was out.

Thanks for looking into it guys.

On Oct 10, 1:53 am, James Reeves <jree...@weavejester.com> wrote:
> Log it as an issue against Ring. The problem occurs in 0.3.11 and
> above, and I can see the cause in the source code. I'll get it fixed
> soon.
>
> - James
>

> On 10 October 2011 00:13, Chris Granger <ibdk...@gmail.com> wrote:
>
>
>
>
>
>
>
> > This is broken at the compojure/ring level. I just tried your compojure code
> > with compojure 0.6.5 and it doesn't work. I'm switching back to compojure
> > 0.6.4 in  Noir 1.2.1-SNAPSHOT. I just tried it and it worked. Give
> > 1.2.1-SNAPSHOT a try and let me know if it resolves it for you.
> > If it does, can you log an issue against compojure?
>
> > Cheers,
> > Chris.
>

> > On Sun, Oct 9, 2011 at 3:37 PM, Mark Rathwell <mark.rathw...@gmail.com>