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

NullPointerException from call to UpgradeRequest#getUserPrincipal with Jetty 12 #10498

Closed
rstoyanchev opened this issue Sep 12, 2023 · 3 comments · Fixed by #10494
Closed

NullPointerException from call to UpgradeRequest#getUserPrincipal with Jetty 12 #10498

rstoyanchev opened this issue Sep 12, 2023 · 3 comments · Fixed by #10494
Assignees
Labels
Bug For general bugs on Jetty side
Milestone

Comments

@rstoyanchev
Copy link

rstoyanchev commented Sep 12, 2023

Jetty version(s)
12.01

Jetty Environment
ee10

Java version/vendor (use: java -version)
17.0.7

OS type/version
Ubuntu 23.04

Description
I work on the Spring Framework and our Jetty WebSocket integration runs into a NPE when trying to obtain the Principal from the @OnWebSocketOpen method of our handler. This is the stacktrace:

java.lang.NullPointerException: Cannot invoke "org.eclipse.jetty.server.Request.getAttribute(String)" because "request" is null
	at org.eclipse.jetty.server.Request.getAuthenticationState(Request.java:897) ~[jetty-server-12.0.1.jar:12.0.1]
	at org.eclipse.jetty.security.AuthenticationState.getAuthenticationState(AuthenticationState.java:45) ~[jetty-security-12.0.1.jar:12.0.1]
	at org.eclipse.jetty.ee10.servlet.ServletApiRequest.getAuthentication(ServletApiRequest.java:124) ~[jetty-ee10-servlet-12.0.1.jar:12.0.1]
	at org.eclipse.jetty.ee10.servlet.ServletApiRequest.getUndeferredAuthentication(ServletApiRequest.java:129) ~[jetty-ee10-servlet-12.0.1.jar:12.0.1]
	at org.eclipse.jetty.ee10.servlet.ServletApiRequest.getUserPrincipal(ServletApiRequest.java:341) ~[jetty-ee10-servlet-12.0.1.jar:12.0.1]
	at org.eclipse.jetty.ee10.websocket.server.internal.DelegatedServerUpgradeRequest.getUserPrincipal(DelegatedServerUpgradeRequest.java:208) ~[jetty-ee10-websocket-jetty-server-12.0.1.jar:12.0.1]
	at org.springframework.web.socket.adapter.jetty.JettyWebSocketSession.initializeNativeSession(JettyWebSocketSession.java:195) ~[spring-websocket-6.1.0-SNAPSHOT.jar:6.1.0-SNAPSHOT]
	at org.springframework.web.socket.adapter.jetty.JettyWebSocketHandlerAdapter.onWebSocketOpen(JettyWebSocketHandlerAdapter.java:72) ~[spring-websocket-6.1.0-SNAPSHOT.jar:6.1.0-SNAPSHOT]
	at org.eclipse.jetty.websocket.common.JettyWebSocketFrameHandler.onOpen(JettyWebSocketFrameHandler.java:142) ~[jetty-websocket-jetty-common-12.0.1.jar:12.0.1]

How to reproduce?

I can reproduce it with a very basic project via https://start.spring.io with Spring Boot 3.2.0-SNAPSHOT for the Jetty 12 integration, jetty-websocket-demo.zip.

Downgrading to Boot 3.1.3 (Jetty 11.0.15) works, just need to add <jakarta-servlet.version>5.0.0</jakarta-servlet.version> as a property in the Maven pom to pin the Servlet API version.

@rstoyanchev rstoyanchev added the Bug For general bugs on Jetty side label Sep 12, 2023
@joakime joakime added this to the 12.0.x milestone Sep 12, 2023
@lachlan-roberts
Copy link
Contributor

I think this is because by the time onOpen() is called the ServletChannel has already been recycled so has no request. I think we might need to avoid recycling the Request and ServletChannel if an upgrade has occurred.

I will do this work in PR #10494 which was originally just for issue #10490 which is quite similar to this.

@sbordet
Copy link
Contributor

sbordet commented Sep 13, 2023

@lachlan-roberts in CometD we copy all the relevant information before calling onOpen(), so I think we should keep the current recycling, but copy the data in UpgradeRequest (URI, headers, etc.).

@gregw gregw moved this to 🏗 In progress in Jetty 12.0.2 FROZEN Sep 13, 2023
lachlan-roberts added a commit that referenced this issue Sep 14, 2023
@lachlan-roberts
Copy link
Contributor

Will be fixed in 12.0.2.

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.

4 participants