Adding middleware keyword to a route definition can change the order of middleware

[johnmcconnell]

Using the list (https://clojuredocs.org/clojure.core/list) data structure to define the main middleware:

Before

(ring/router
  ["/" {:handler h-fn}]
  :data {:middleware (cons m1 [m2 m3 m4])})

Works as expected

After

(ring/router
  ["/" {:handler h-fn
          :middleware []}]
  :data {:middleware (cons m1 [m2 m3 m4])})

Now the middleware chain is executed out of order

FYI I didn't verify this, but I am fairly confident this happens or something like this happens

[johnmcconnell]

Maybe make sure all middleware lists are converted to vectors before merging or applying new ones

[johnmcconnell]

Also, I appreciate the work. Keep up the great job

[dawranliou]

Here's a complete example:

(require '[reitit.core :as r])
(require '[reitit.ring :as ring])

(defn mw
  [name]
  {:name name
   :wrap #(constantly %)})

(r/routes
 (ring/router
  ["/" {:handler (constantly {:status 200})}]
  {:data {:middleware (list (mw :1) (mw :2) (mw :3))}}))

;; [["/"
;;   {:middleware
;;    ({:name :1, :wrap #function[user/mw/fn--51003]}
;;     {:name :2, :wrap #function[user/mw/fn--51003]}
;;     {:name :3, :wrap #function[user/mw/fn--51003]}),
;;    :handler #function[clojure.core/constantly/fn--5672]}]]

(r/routes
 (ring/router
  ["/" {:handler    (constantly {:status 200})
        :middleware []}]
  {:data {:middleware (list (mw :1) (mw :2) (mw :3))}}))

;; [["/"
;;   {:middleware
;;     {:name :3, :wrap #function[user/mw/fn--51003]}
;;     {:name :2, :wrap #function[user/mw/fn--51003]}
;;     {:name :1, :wrap #function[user/mw/fn--51003]}),
;;    :handler #function[clojure.core/constantly/fn--5672]}]]

The middleware chain was reversed in the second router.

What I found is that: when meta-merge.core/meta-merge merges two collections, it concats the two then put the result into the type of first collection. In this case, the first collection is a list so the result was reversed:

(meta-merge {:middleware '(:a :b :c)} {:middleware []})
;; => {:middleware (:c :b :a)}

To solve this, besides to convert middlewares into a vector as @johnmcconnell suggests, perhaps we can have the reitit.ring.spec/validate to validate it.

[ikitommi]

Yes, it is unfortunate that lists and vectors merge differently. Validate of this would be great.

[opqdonut]

Fixed already in 2019 by #319