Problem with wrap-defaults site-defaults and compojure wrap-routes

[theophilusx]

Have a problem when using compojure wrap-routes to associate middleware with specific routes. Only seems to occure when using the site-defaults related config i.e. api-defaults seems to not exhibit this issue.

Appears that the wrap-defaults is blowing away the response codes when a page is not found. i.e.

$ curl -v http://localhost:3000/q
* Hostname was NOT found in DNS cache
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET /q HTTP/1.1
> User-Agent: curl/7.35.0
> Host: localhost:3000
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Wed, 21 Jan 2015 02:10:24 GMT
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-XSS-Protection: 1; mode=block
< Content-Length: 0
* Server Jetty(7.6.8.v20121106) is not blacklisted
< Server: Jetty(7.6.8.v20121106)
< 
* Connection #0 to host localhost left intact

instead of

$ curl -v http://localhost:3000/q
* Hostname was NOT found in DNS cache
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET /q HTTP/1.1
> User-Agent: curl/7.35.0
> Host: localhost:3000
> Accept: */*
> 
< HTTP/1.1 404 Not Found
< Date: Wed, 21 Jan 2015 02:12:49 GMT
< Content-Type: text/html;charset=UTF-8
< Content-Length: 9
* Server Jetty(7.6.8.v20121106) is not blacklisted
< Server: Jetty(7.6.8.v20121106)
< 
* Connection #0 to host localhost left intact
Sorry, No%                                                                  

Procedure to reporduce -

1. lein new compojure ctest
2. ran lein ancient and updated dependency versions to latest
3. created the following handler.clj

(ns cc.core.handler
  (:require [compojure.core :refer :all]
            [compojure.route :as route]
            [ring.middleware.defaults :refer [wrap-defaults site-defaults
                                              api-defaults]]))

(defroutes api-routes
  (GET "/api" [] "API endpoint")
  (ANY "/api/order/calc" [] "Calc endpint")
  (ANY "/api/order/place" [] "Place endpoint"))

(defroutes app-routes
  (GET "/" [] "Hello World")
  (route/resources "/"))

(defroutes no-page
  (route/not-found "Sorry, No"))

(defn mw [hdlr msg]
  (fn [req]
    (println msg)
    (hdlr req)))

(def app
  (routes (-> api-routes
              (wrap-routes wrap-defaults api-defaults))
          (-> app-routes
              (wrap-routes wrap-defaults api-defaults))
          no-page))

If you change the second wrap-defaults to use site-defaults, the problem is triggered.

[theophilusx]

Suspect this is related to the injecting headers into nil responses issue.

[weavejester]

Apologies for taking a while to reply.

Compojure provides wrap-routes to make it possible to inject middleware within a route, but this is only useful when working with middleware that can return without evaluating the handler. As wrap-routes has a performance impact, it should only be used in the rare conditions when its use makes sense.

I can certainly fix the nil issue, but you almost certainly shouldn't be using wrap-routes with wrap-defaults.

[theophilusx]

thanks for the response.

Given what you suggest, can you provide some guidance on how to handle the following use case

• Have a clojure web application which needs to serve both application routes (i.e. accessed via a browser) and API routes (accessed by non-browser clients and AJAX calls in clojurescript)
• Want to apply the ring-defaults with the site-defaults or secure-site-defaults for the application routes, but not the API routes as the XRF anti-forgery and other features included in the site defaults are not required or at worst, create significant additional complexity for API and AJAX clients
• Want to apply the api-defaults of wrap-faults to the API routes, but not to the application routes.

I’ve asked around and the wrap-routes solution was recommended by one of the composure contributors. From the feedback I’ve received, this seems to be a fairly common use case which many are having difficulty in solving.

thanks,

Tim

Tim Cross

On 12 Feb 2015, at 7:58 am, James Reeves notifications@github.com wrote:

Apologies for taking a while to reply.

Compojure provides wrap-routes to make it possible to inject middleware within a route, but this is only useful when working with middleware that can return without evaluating the handler. As wrap-routes has a performance impact, it should only be used in the rare conditions when its use makes sense.

I can certainly fix the nil issue, but you almost certainly shouldn't be using wrap-routes with wrap-defaults.

— Reply to this email directly or view it on GitHub https://github.com/ring-clojure/ring-defaults/issues/10#issuecomment-73963361.

[weavejester]

You can just apply wrap-defaults to both sets of middleware. The only caveat is that you need to be careful about side-effectful middleware, such as wrap-params, which consumes the request body upon reading it.

However, since api-defaults is a subset of site-defaults, we can apply the api-defaults to both and the site-defaults to only one, avoiding the wrap-params problem. For example:

(def app
  (-> (routes api-routes (wrap-defaults site-routes site-defaults))
      (wrap-defaults api-defaults)))

[theophilusx]

I’ll go back and look at this again, but your suggestion is pretty much what I tried initially and it did not work. The middleware applied tot the app-routes was also applied to the api-routes. Probably is the side effects you mentioned.

I need the anti-forgery support/feature on the application routes, but not on the API routes. When I tried doing what you suggest, the anti-forgery token requirement was enforced on both so that any POST calls to api routes failed if the anti-forger token wasn’t in the form data or headers. Problem is that the anti-forgery token approach only really works in sessions and not when dealing with sessionless API calls, such as from a script making REST calls to your API

Tim Cross

On 12 Feb 2015, at 2:22 pm, James Reeves notifications@github.com wrote:

You can just apply wrap-defaults to both sets of middleware. The only caveat is that you need to be careful about side-effectful middleware, such as wrap-params, which consumes the request body upon reading it.

However, since api-defaults is a subset of site-defaults, we can apply the api-defaults to both and the site-defaults to only one, avoiding the wrap-params problem. For example:

(def app (-> (routes api-routes (wrap-defaults site-routes site-defaults)) (wrap-defaults api-defaults))) — Reply to this email directly or view it on GitHub https://github.com/ring-clojure/ring-defaults/issues/10#issuecomment-74011279.

[weavejester]

Ah I see. Usually I don't have a problem with the anti-forgery middleware, because the API and site routes are generally separated by a subdomain or a common prefix. However, if both sets of routes are mixed together in a way that they're not readily distinguishable, then that would be a valid use-case for using wrap-routes with wrap-anti-forgery.

I would suggest separating out the anti-forgery middleware from wrap-defaults in such a case, though, and using wrap-routes in a more targeted manner.

[theophilusx]

Thanks James.

Putting the API under a different domain or similar was my fall back solution. I had already thought that given my use case, perhaps I would be better in applying the various middleware components in a more manual and targeted manner.

You indicate that if we used a common prefix, then things would be easier. I’m assuming you mean something like defining the routes within a common context. I’m not clear how this would help - are you suggesting that you could use a context and then wrap the middleware only around the routes defined in that context or are you referring to somehow controlling when middleware is applied using something like middleware.conditional that only applies the middleware based on the route path?

sorry if these are really dumb newbie questions. Have been searching for a good solution here for ages and keep almost getting a solution only to trip at the last hurdle. Quite surprised how little I could find on this as it doesn’t seem like a terribly unusual use case to me.

Tim Cross

On 12 Feb 2015, at 3:12 pm, James Reeves notifications@github.com wrote:

Ah I see. Usually I don't have a problem with the anti-forgery middleware, because the API and site routes are generally separated by a subdomain or a common prefix.

However, if both sets of routes are mixed together in a way that they're not readily distinguishable, then that would be a valid use-case for using wrap-routes with wrap-anti-forgery. I would suggest separating out the anti-forgery middleware from wrap-defaults in such a case, though, and using wrap-routes in a more targeted manner.

— Reply to this email directly or view it on GitHub https://github.com/ring-clojure/ring-defaults/issues/10#issuecomment-74014797.

[weavejester]

Your problem is that you need some middleware to not trigger for certain routes. There are several ways to go about this.

The first is to recognise that Compojure routes are tried in order, so if you place your API routes first, then they should match and dispatch before the site routes, which are wrapped in your anti-forgery middleware. This is what I suggested in my first example.

(routes api-routes (wrap-anti-forgery site-routes))

Another option is to give both your API and site routes different prefixes, and apply your anti-forgery middleware inside the site context. This means that the site prefix needs to match in order for the middleware to be applied. e.g.

(context "/site" []
  (wrap-anti-forgery site-routes))

The third option is to make use of wrap-routes, which injects the middleware into the routes, so that the middleware is only applied if a route matches. This is slightly less efficient, but probably doesn't make a huge difference in practice.

(wrap-routes api-routes wrap-anti-forgery)

[theophilusx]

Thanks James. I will do some more experimentation

[thedavidmeister]

was this fixed with #9 ?

[weavejester]

The nil part of it was. This issue was mostly a question, though.

[thedavidmeister]

@weavejester great, thanks :)