Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

route-matches Doesn't Match from Servlet Root #1

Closed
wtetzner opened this issue Apr 14, 2010 · 2 comments
Closed

route-matches Doesn't Match from Servlet Root #1

wtetzner opened this issue Apr 14, 2010 · 2 comments

Comments

@wtetzner
Copy link

(defmethod route-matches [::compiled-route Map]
  [route request]
  (route-matches route (if (:absolute? route)
                         (request-url request)
                         (:uri request))))

This method matches on (:uri request), which works fine when using a single servlet that is mapped to /, but with multiple servlets at different urls, it would be better to match from the root of the servlet. Replacing (:uri request) with (.getPathInfo (:servlet-request request)) should be enough:

(defmethod route-matches [::compiled-route Map]
  [route request]
  (route-matches route (if (:absolute? route)
                         (request-url request)
                         (.getPathInfo (:servlet-request request)))))
@weavejester
Copy link
Owner

:servlet-request is a non-standard key and not guaranteed to exist. Whilst most Ring adapters are based off existing Java web servers, and thus include a :servlet-request key, this is not necessarily the case. I don't want Clout to work with some adapters but crash with others.

A better solution would be to use some middleware that changes the :uri key. For example:

(defn wrap-servlet-path-info [handler]
  (fn [request]
    (if-let [servlet-request (:servlet-request request)]
      (handler (assoc request :uri (.getPathInfo servlet-request)))
      (handler request))))

@wtetzner
Copy link
Author

I see. I didn't realize it was intended to work with more than servlet containers. The middleware solution should work fine.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants