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

ClassCastException in ring.util.jakarta.servlet/set-headers #515

Closed
ejschoen opened this issue Nov 19, 2024 · 3 comments
Closed

ClassCastException in ring.util.jakarta.servlet/set-headers #515

ejschoen opened this issue Nov 19, 2024 · 3 comments

Comments

@ejschoen
Copy link

This is weird. We just upgraded a pretty large app from Ring 1.8.0 to Ring 1.11.0, and with it from Jetty 9 to Jetty 11 (and while're we were at it, Clojure 1.8 to Clojure 1.11). After a bunch of changes to accomodate necessary upgrades to 3rd party stuff, the app works fine. However, I noticed that now there are certain POST requests that fail if run from swagger-ui (we use compojure-api, ring-swagger-ui, ring-swagger). This is new behavior.

                                                             java.lang.Thread.run                     Thread.java:  829
                          org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run           QueuedThreadPool.java: 1149
                     org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob           QueuedThreadPool.java: 1194
                              org.eclipse.jetty.util.thread.QueuedThreadPool.runJob           QueuedThreadPool.java:  969
                               org.eclipse.jetty.io.SelectableChannelEndPoint$1.run  SelectableChannelEndPoint.java:   53
                                         org.eclipse.jetty.io.FillInterest.fillable               FillInterest.java:  100
                     org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded         AbstractConnection.java:  314
                                 org.eclipse.jetty.server.HttpConnection.onFillable             HttpConnection.java:  287
                                        org.eclipse.jetty.server.HttpChannel.handle                HttpChannel.java:  501
                                      org.eclipse.jetty.server.HttpChannel.dispatch                HttpChannel.java:  753
                  org.eclipse.jetty.server.HttpChannel$RequestDispatchable.dispatch                HttpChannel.java: 1598
                                             org.eclipse.jetty.server.Server.handle                     Server.java:  563
                             org.eclipse.jetty.server.handler.HandlerWrapper.handle             HandlerWrapper.java:  122
                          org.eclipse.jetty.server.handler.StatisticsHandler.handle          StatisticsHandler.java:  173
                           org.eclipse.jetty.server.handler.gzip.GzipHandler.handle                GzipHandler.java:  822
                             org.eclipse.jetty.server.handler.HandlerWrapper.handle             HandlerWrapper.java:  122
                              org.eclipse.jetty.server.handler.ScopedHandler.handle              ScopedHandler.java:  129
                            org.eclipse.jetty.server.handler.ContextHandler.doScope             ContextHandler.java: 1303
                           org.eclipse.jetty.server.handler.ScopedHandler.nextScope              ScopedHandler.java:  174
 ring.adapter.jetty.proxy$org.eclipse.jetty.servlet.ServletHandler$ff19274a.doScope                                :
                                   org.eclipse.jetty.servlet.ServletHandler.doScope             ServletHandler.java:  484
                           org.eclipse.jetty.server.handler.ScopedHandler.nextScope              ScopedHandler.java:  176
                           org.eclipse.jetty.server.handler.ContextHandler.doHandle             ContextHandler.java: 1381
                          org.eclipse.jetty.server.handler.ScopedHandler.nextHandle              ScopedHandler.java:  221
ring.adapter.jetty.proxy$org.eclipse.jetty.servlet.ServletHandler$ff19274a.doHandle                                :
                                                ring.adapter.jetty/proxy-handler/fn                       jetty.clj:  111
                                  ring.util.jakarta.servlet/update-servlet-response                     servlet.clj:   86
                                  ring.util.jakarta.servlet/update-servlet-response                     servlet.clj:   95
                                              ring.util.jakarta.servlet/set-headers                     servlet.clj:   66
java.lang.ClassCastException: class clojure.lang.Keyword cannot be cast to class java.lang.String (clojure.lang.Keyword is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap')

This is strange, because ring-swagger-ui just arranges for the same call from the browser, plus some headers, that a non-browser client would make. I captured the request in the browser debugger, exported it as a curl command and then started removing command options until the POST no longer failed. It was the Origin header.

But not every request from swagger UI fails, even with an Origin header. I haven't been through every line of compojure-api to figure out if it's responding to the Origin header in some way that causes it to return a headers in its response map with keyword keys or values. I have confirmed that my code is not returning such a header.

I wonder if ring.util.jakarta.servlet/set-headers could use (name key) and (str val) on line ~67 in the .addHeader call for a little more safety.

@weavejester
Copy link
Member

The error message looks to be reporting a genuine error in your application or in one of the supporting libraries. As far as I can see from the stacktrace, Ring is working as intended.

You may want to add some middleware to print out the response map at different points to see where the invalid header map is being generated.

@ejschoen
Copy link
Author

ejschoen commented Nov 19, 2024

Yes--I had to put the middleware in a couple of places at the very bottom of the stack to find the offender, and then spend a couple of hours figuring it out.

It's due ring.middleware.cors/wrap-cors in [ring-cors "0.1.13"] interacting with the HeaderMap that gets returned by clj-http.headers/header-map as the value of :headers in a response map. The CORs middleware's response handler injects CORS headers into the response, but only when there's an Origin header in the request. After the middleware is done, the Access-Control-Allow-Methods header ends up with the middleware's internal representation and not a servlet-compliant representation:

"Access-Control-Allow-Methods" #{:get :post}

The root cause is subtle. The CORS code tries to do the right thing with the response header, and succeeds if that header object in the response map is a Clojure hash map. But when the header object in the response map is a clj-http.headers/HeaderMap, it fails. In the example below, normalize-headers is a function from ring.middleware.cors:

main=> (normalize-headers (assoc (clj-http.headers/header-map) :access-control-allow-methods #{:get :post}))
{"Access-Control-Allow-Methods" #{:get :post}}

vs.

main=> (normalize-headers (assoc {} :access-control-allow-methods #{:get :post}))
{"Access-Control-Allow-Methods" "GET, POST"}

EDIT: In this specific case, the endpoint I was calling makes a clj-http call to another microservice and returns the response from that microservice as the response from this endpoint, which is how the :headers in the response map end up being a clj-http.headers/HeaderMap.

This is ultimately due to the CORs code injecting :access-control-allow-methods into the response map, which HeaderMap converts to "Access-Control-Allow-Methods," and then subsequently trying to normalize all of the headers it thinks it injected into the response map. But when it enumerates the header map keys after injecting its headers, it gets "Access-Control-Allow-Methods" rather than :access-control-allow-methods, and it simply reinjects the value without converting it to an joined, uppercase string.

Someone else stumbled across this a few years ago (r0man/ring-cors#33) but it hasn't gotten any attention. I guess the only solution at this point is to fork the repo and fix it. There's been no activity on that project for 6 years.

Do you have any opinion as to whether something like wrap-cors ought to be in ring-core?

@weavejester
Copy link
Member

wrap-cors shouldn't be in ring-core, but I have been considering writing my own CORS library for Ring, and releasing it under the org.ring-clojure group.

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