ngrunwald / ring-middleware-format

Ring middleware for parsing parameters and emitting responses in JSON or other formats
163 stars 49 forks source link

Defaulting `Accept` to `Content-Type` breaks stuff #66

Closed ianmcorvidae closed 7 years ago

ianmcorvidae commented 7 years ago

Looking at this line (this is current master as of this writing), the preferred-encoder function is trying to grab the Accept header but falling back to the Content-Type one. This fails in at least one case, with the Content-Type header of multipart/form-data; boundary=gKtLqMkKn1FxJcRHJBQKcLCToSaLff8sj9UmAY3; charset=US-ASCII (this is for a file upload). In this case, the code in parse-accept-header* assumes the second parameter is a quality metric and then tries to parse the charset as a number.

Since the docstring of preferred-encoder claims "If no Accept header is found, return the first encoder.", it'd seem like this defaulting shouldn't happen, but the code's been there for quite a while, since 2013 (https://github.com/ngrunwald/ring-middleware-format/commit/888b0981638b0015f0eba7f126514b8e84f468eb).

I'm not sure what the right way to fix this is, but the exception that's coming out in this case is:

java.lang.NumberFormatException: For input string: "US-ASCII"
    at sun.misc.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:2043) ~[na:1.8.0_92-internal]
    at sun.misc.FloatingDecimal.parseDouble(FloatingDecimal.java:110) ~[na:1.8.0_92-internal]
    at java.lang.Double.parseDouble(Double.java:538) ~[na:1.8.0_92-internal]
    at ring.middleware.format_response.invoke(format_response.clj:70) ~[data-info-standalone.jar:na]
    at clojure.core.invoke(core.clj:2644) ~[data-info-standalone.jar:na]
    at clojure.lang.LazySeq.sval(LazySeq.java:40) ~[data-info-standalone.jar:na]
    at clojure.lang.LazySeq.seq(LazySeq.java:49) ~[data-info-standalone.jar:na]
    at clojure.lang.RT.seq(RT.java:521) ~[data-info-standalone.jar:na]
    at clojure.core.invokeStatic(core.clj:137) ~[data-info-standalone.jar:na]
    at clojure.core.invokeStatic(core.clj:3004) ~[data-info-standalone.jar:na]
    at clojure.core.invokeStatic(core.clj:3010) ~[data-info-standalone.jar:na]
    at clojure.core.invoke(core.clj:3010) ~[data-info-standalone.jar:na]
    at ring.middleware.format_response.invokeStatic(format_response.clj:44) ~[data-info-standalone.jar:na]
    at ring.middleware.format_response.invoke(format_response.clj:42) ~[data-info-standalone.jar:na]
    at ring.middleware.format_response.invokeStatic(format_response.clj:75) ~[data-info-standalone.jar:na]
    at ring.middleware.format_response.invoke(format_response.clj:50) ~[data-info-standalone.jar:na]
    at clojure.lang.AFn.applyToHelper(AFn.java:154) ~[data-info-standalone.jar:na]
    at clojure.lang.AFn.applyTo(AFn.java:144) ~[data-info-standalone.jar:na]
    at clojure.core.invokeStatic(core.clj:646) ~[data-info-standalone.jar:na]
    at clojure.core.invoke(core.clj:641) ~[data-info-standalone.jar:na]
    at clojure.core.memoize.invoke(memoize.clj:66) ~[data-info-standalone.jar:na]
    at clojure.core.cache.invoke(cache.clj:55) ~[data-info-standalone.jar:na]
    at clojure.core.memoize.invoke(memoize.clj:65) ~[data-info-standalone.jar:na]
    at clojure.core.memoize.deref(memoize.clj:54) ~[data-info-standalone.jar:na]
    at clojure.core.invokeStatic(core.clj:2228) ~[data-info-standalone.jar:na]
    at clojure.core.invoke(core.clj:2214) ~[data-info-standalone.jar:na]
    at clojure.core.memoize.doInvoke(memoize.clj:152) ~[data-info-standalone.jar:na]
    at clojure.lang.RestFn.applyTo(RestFn.java:137) ~[data-info-standalone.jar:na]
    at clojure.lang.AFunction.doInvoke(AFunction.java:29) ~[data-info-standalone.jar:na]
    at clojure.lang.RestFn.invoke(RestFn.java:408) ~[data-info-standalone.jar:na]
    at ring.middleware.format_response.invokeStatic(format_response.clj:94) ~[data-info-standalone.jar:na]
    at ring.middleware.format_response.invoke(format_response.clj:84) ~[data-info-standalone.jar:na]
    at ring.middleware.format_response.invoke(format_response.clj:186) ~[data-info-standalone.jar:na]
    at ring.middleware.keyword_params.invoke(keyword_params.clj:35) ~[data-info-standalone.jar:na]
    at ring.middleware.nested_params.invoke(nested_params.clj:86) ~[data-info-standalone.jar:na]
    at ring.middleware.params.invoke(params.clj:64) ~[data-info-standalone.jar:na]
    at compojure.api.middleware.invoke(middleware.clj:74) ~[data-info-standalone.jar:na]
    at compojure.api.routes.Route.invoke(routes.clj:74) ~[data-info-standalone.jar:na]
    at ring.adapter.jetty.invoke(jetty.clj:24) ~[na:na]
    at ring.adapter.jetty.proxy.eclipse.jetty.server.handler.AbstractHandler.handle(Unknown Source) ~[na:na]
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97) ~[data-info-standalone.jar:na]
    at org.eclipse.jetty.server.Server.handle(Server.java:497) ~[data-info-standalone.jar:na]
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:310) ~[data-info-standalone.jar:na]
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257) [data-info-standalone.jar:na]
    at org.eclipse.jetty.io.AbstractConnection.run(AbstractConnection.java:540) [data-info-standalone.jar:na]
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635) [data-info-standalone.jar:na]
    at org.eclipse.jetty.util.thread.QueuedThreadPool.run(QueuedThreadPool.java:555) [data-info-standalone.jar:na]
    at java.lang.Thread.run(Thread.java:745) [na:1.8.0_92-internal]
Deraen commented 7 years ago

I released 0.7.1 with a fix.