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

java.lang.ClassCastException: class org.eclipse.jetty.http.HttpField cannot be cast to class org.eclipse.jetty.http.HttpCookie$SetCookieHttpField #8294

Closed
Horcrux7 opened this issue Jul 13, 2022 · 10 comments · Fixed by #8298
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@Horcrux7
Copy link

Jetty version(s)
10.0.11

Java version/vendor
17

Description
After we have call the follow code:

String rfc6265cookie = ...;
response.addHeader( "Set-Cookie", rfc6265cookie );

we get the follow exception:

java.lang.ClassCastException: class org.eclipse.jetty.http.HttpField cannot be cast to class org.eclipse.jetty.http.HttpCookie$SetCookieHttpField (org.eclipse.jetty.http.HttpField and org.eclipse.jetty.http.HttpCookie$SetCookieHttpField are in unnamed module of loader com.inet.plugin.DependencyClassLoader @4b2bac3f)
	at org.eclipse.jetty.server.Request.newPushBuilder(Request.java:355)
	at javax.servlet.http.HttpServletRequestWrapper.newPushBuilder(HttpServletRequestWrapper.java:349)
	at com.inet.remote.gui.angular.AngularContentService.createTemplate(AngularContentService.java:428)
	at com.inet.remote.gui.angular.AngularContentService.serveTemplate(AngularContentService.java:307)
	at com.inet.remote.gui.modules.reporterror.ReportErrorHandler.handleGet(ReportErrorHandler.java:74)
	at com.inet.remote.gui.modules.reporterror.ReportErrorHandler.handle(ReportErrorHandler.java:41)
	at com.inet.remote.gui.angular.AngularContentService.handleError(AngularContentService.java:874)
	at com.inet.remote.gui.angular.RemoteGuiServletErrorHandlerProvider.sendErrorPage(RemoteGuiServletErrorHandlerProvider.java:49)
	at com.inet.http.error.ServletErrorHandler.sendErrorPage(ServletErrorHandler.java:55)
	at com.inet.http.PluginDispatcherServlet.service(PluginDispatcherServlet.java:157)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:764)
	at org.eclipse.jetty.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1665)
	at org.eclipse.jetty.websocket.servlet.WebSocketUpgradeFilter.doFilter(WebSocketUpgradeFilter.java:170)
	at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:202)
@Horcrux7 Horcrux7 added the Bug For general bugs on Jetty side label Jul 13, 2022
@joakime
Copy link
Contributor

joakime commented Jul 13, 2022

Your code snippet ...

String rfc6265cookie = ...;
response.addHeader( "Set-Cookie", rfc6265cookie );

Key piece of information in your stacktrace ...

java.lang.ClassCastException: class org.eclipse.jetty.http.HttpField cannot be cast 
  to class org.eclipse.jetty.http.HttpCookie$SetCookieHttpField (org.eclipse.jetty.http.HttpField 
  and org.eclipse.jetty.http.HttpCookie$SetCookieHttpField are in 
  unnamed module of loader com.inet.plugin.DependencyClassLoader @4b2bac3f)
	at org.eclipse.jetty.server.Request.newPushBuilder(Request.java:355)
	at javax.servlet.http.HttpServletRequestWrapper.newPushBuilder(HttpServletRequestWrapper.java:349)

This occurred during a PushBuilder action, that's fishy.

The PushBuilder only supports Set-Cookie headers that were set via the HttpServletResponse.addCookie(Cookie) method, as the container has to manage the Set-Cookie lines according to the rules of the Cookie, which a raw Set-Cookie header does not support.

@Horcrux7
Copy link
Author

It is clear that a custom cookie is not available in the PushBuilder request. I did not expect that at all. But there should be no exception.

@joakime
Copy link
Contributor

joakime commented Jul 13, 2022

It is clear that a custom cookie is not available in the PushBuilder request. I did not expect that at all. But there should be no exception.

Incorrect, for a Push to succeed, the response headers need to be sane, that is especially true for the Set-Cookie header, as your primary request/response can have a Set-Cookie that is not compatible with the PushBuilder resulting response.
Each response MUST copy the set of Cookie response objects from the primary response to the pushed response and be filtered based on the cookie details (path, time to live, expiration, etc).
This is a requirement if using the PushBuilder. Your error is due to abuse of the Servlet API (not using addCookie(Cookie) in conjunction of using the PushBuilder).
An exception has to occur in this scenario.

Now what that exception is, is a totally different question.
That part we should clean up, but it will absolutely produce an exception after the cleanup.
Probably more in line with IllegalStateException: raw "Set-Cookie" detected in conjunction with "PushBuilder", use "response.addCookie(Cookie)" instead of a raw "Set-Cookie" to resolve this.

@gregw
Copy link
Contributor

gregw commented Jul 13, 2022

@joakime I tend to agree with the OP that we should not be throwing such an exception. The code is making the assumption that all setCookie fields have been set via a particular API, but there are alternative paths.
I don't think there is anything that says the setCookie API must be used to set cookies, and that setting them via addHeader is allowable.

So I think this is a bug.

@gregw gregw self-assigned this Jul 13, 2022
@Horcrux7
Copy link
Author

@joakime
Incorrect, for a Push to succeed, the response headers need to be sane, that is especially true for the Set-Cookie header, as your primary request/response can have a Set-Cookie that is not compatible with the PushBuilder resulting response.
Each response MUST copy the set of Cookie response objects from the primary response to the pushed response and be filtered based on the cookie details (path, time to live, expiration, etc).
This is a requirement if using the PushBuilder.

Can you tell me where this is specified?

Your error is due to abuse of the Servlet API (not using addCookie(Cookie) in conjunction of using the PushBuilder).

The current cookie API is not compatible with RFC6265. That this is a workaround.

@joakime
Copy link
Contributor

joakime commented Jul 13, 2022

The current cookie API is not compatible with RFC6265. That this is a workaround.

Jetty has a CookieCompliance mode for RFC6265.
https://github.com/eclipse/jetty.project/blob/jetty-10.0.11/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java

https://github.com/eclipse/jetty.project/blob/d988aa016e0bb2de6fba84c1659049c72eae3e32/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java#L81

Which is the default for Jetty 10+

https://github.com/eclipse/jetty.project/blob/jetty-10.0.11/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java#L76-L77

https://github.com/eclipse/jetty.project/blob/d988aa016e0bb2de6fba84c1659049c72eae3e32/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java#L76-L77

Is there some specific detail of RFC6265 that you are struggling with?
Example:

If you want to manage Same-Site ...

  • That can be managed via a ServletContext attribute of key org.eclipse.jetty.cookie.sameSiteDefault with a value of either a org.eclipse.jetty.http.HttpCookie$SameSite object, or a string of one of the following None, Strict, or Lax).
  • Or via a specific javax.servlet.http.Cookie using the setComment(String) method with one of the strings from HttpCookie.

https://github.com/eclipse/jetty.project/blob/d988aa016e0bb2de6fba84c1659049c72eae3e32/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java#L38-L44

@gregw
Copy link
Contributor

gregw commented Jul 13, 2022

@joakime I'm not sure that calling addHeader for a set cookie is really an "abuse" of the API. I think it is a "do so at your own risk" kind of thing.
In this particular case, we are looking for a maxAge on the set cookie and assumed we could use our own internal type to get it. Because the API provides an alternative path, we should support that, even if it means reparsing that cookie to look for the max age.

@Horcrux7 As indicated above, we do use he cookie comment field as a work around for the limitations of the Cookie API, so you should be able to set most (all?) fields you need via that API. The benefit then is that we are internally more efficient and don't have to re-parse any set-cookie added via other APIs.

gregw added a commit that referenced this issue Jul 13, 2022
Reparse cookie added with addCookie

Signed-off-by: Greg Wilkins <[email protected]>
@gregw gregw linked a pull request Jul 13, 2022 that will close this issue
@gregw
Copy link
Contributor

gregw commented Jul 13, 2022

I've done a fix is PR #8298, but it uses java.net.HttpCookie to reparse the cookie. So if you do anything in your set cookie that breaks that parser, then you'll just get a different exception. @Horcrux7 is that OK?

@joakime
Copy link
Contributor

joakime commented Jul 13, 2022

It would be good to have an example of the RFC6265 Set-Cookie header that @Horcrux7 is using, so we can ensure the reparse technique will work.

gregw added a commit that referenced this issue Jul 14, 2022
Added extra test to ensure maxAge is being parsed

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Jul 14, 2022
Added extra test to ensure maxAge is being parsed with other cookie attributes

Signed-off-by: Greg Wilkins <[email protected]>
@Horcrux7
Copy link
Author

Yes, it for the SameSite attribute. Even though most of our customers use our software together with a Jetty, not all of them do. That we can't use Jetty only features. It must also work with other servlet engines.

Here are some sample of cookies that we send. Also If I think that my cookies are not relevant.

AUTHENTICATION_DESCRIPTION_COOKIE9000=windows; Path=/; SameSite=Lax; HttpOnly

AUTHENTICATION_DESCRIPTION_COOKIE9443=windows; Path=/; SameSite=Lax; Secure; HttpOnly

SID9000=WyI2cWo2a3N0OHNiYmwzaXcxb3RpNDF6Zng0IiwiMmZoYnNibHR6NndnNG56N2hxejVuNWh2OSIsIjV1ajFuZmgzMWY0bDF4N3dsNmgyenZzczIiXQ==; Path=/; Expires=Thu, 11 Aug 2022 08:08:44 GMT; Max-Age=2419200; SameSite=Lax; HttpOnly

SID9443=WyI2cWo2a3N0OHNiYmwzaXcxb3RpNDF6Zng0IiwiMDU1azdiNXgwcG5ibGkxaHRpYW4yYzZ5byIsIjgwcTQwcHpnNTlrcjRzbXl6NHk2cDdtd3ciXQ==; Path=/; Expires=Thu, 11 Aug 2022 08:11:16 GMT; Max-Age=2419200; SameSite=Lax; Secure; HttpOnly

PS: You check only the age of the cookie to make it available in the push request. If you want emulate the browser behavior then also the path should check? Also if exotic the push request can in a path that does not match the path for a push resource.

joakime pushed a commit that referenced this issue Jul 18, 2022
* Fix #8294 push added cookie

Reparse cookie added with addCookie
Added extra test to ensure maxAge is being parsed with other cookie attributes

Signed-off-by: Greg Wilkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants