ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.77k stars 520 forks source link

ring.middleware.params/assoc-form-params uses slurp instead of ring.util.request/body-string #335

Closed jconti closed 6 years ago

jconti commented 6 years ago

Routes that use wrap-params fail if a call to the ring.util.request/body-string multimethod have replaced a request's body stream with a string. This is a bug in my code, because the ring SPEC clearly states that the type of the :body key's value is java.io.InputStream (probably straight from the servlet API). So by using ring.util.request/body-string my code is breaking that explicit contract for the request map, which causes wrap-params to fail in ring.middleware.params/assoc-form-params.

However it seems that the ability of a route to return nil so another route can attempt processing the request is problematic unless the request map is immutable. The body's stream prevents that immutability from existing. On some requests, of course, the stream must be retained and only processed once. However, others can be converted to strings.

So I have made it the habit (maybe erroneously) to apply this multimethod to the request very early in the processing of my routes, since all my input is forcefully limited in size. Unfortunately it breaks wrap-params. If wrap-params used the same body-string multimethod however, instead of slurp, it would work fine.

To do this I assume the SPEC would need to change the value of :body to be something compatible with the definition of ``body-string```. Maybe a protocol?

weavejester commented 6 years ago

The SPEC can't be changed without breaking compatibility, since existing handlers expect that the body of the request is an InputStream if present.

However, this is something I plan to address in Ring 2.0. My current thinking is that the body of the request should satisfy certain protocols, depending on whether the request is synchronous or asynchronous.

jconti commented 6 years ago

That sounds like a useful idea for 2.0. Thanks for sharing your thoughts and possible directions.