weavejester / compojure

A concise routing library for Ring/Clojure
Eclipse Public License 1.0
4.08k stars 259 forks source link

Failed coercion in context parameters results in NPE #183

Closed svdm closed 6 years ago

svdm commented 6 years ago

If a context has a coercion in its route parameters, and a request comes in that matches the context route prefix but fails the coercion, a NullPointerException is thrown (instead of the match failing/404ing).

I reproduced this in a failing testcase:

(is (nil? ((context "/foo/:x" [x :<< coercions/as-int]
             (GET "/" [] (str x)))
           (mock/request :get "/foo/bar"))))

The stacktrace shows it eventually blows up in make-context, in the call ((make-handler request) request):

java.lang.NullPointerException: null
 at compojure.core$make_context$handler__4077.invoke (core.clj:285)
    compojure.core$make_context$fn__4079.invoke (core.clj:293)

AFAICT the underlying cause is in the context macro that supplies the make-handler function. In its implementation it uses let-request, which handles bindings and coercion for the context parameters. Problem is that it will return nil if a coercion fails. Because make-context assumes it will always get a valid handler function from make-handler, it ends up calling (nil request).

One way to fix this is adding nil checks in make-context here. If that seems like the right approach I'd be happy to make a PR.

weavejester commented 6 years ago

Thanks for spotting and investigating this. A nil check does seem the right approach here. A fix and a test to catch regressions would be very appreciated.