Skip to content

server: HandshakeRequest: add getServletContext() #416

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

morgwai
Copy link

@morgwai morgwai commented Aug 19, 2023

Currently there is no easy universal way to obtain reference to the ServletContext from ServerEndpointConfig.Configurator's level if the websocket container is running as a part of a Java Servlet app. This may be useful to access app init params or app wide attributes. As of 2.1.1 the standard practice is to obtain ServletContext ref via HttpSession in modifyHandshake(...):

public void modifyHandshake(ServerEndpointConfig config, HandshakeRequest request, HandshakeResponse response) {
	final var httpSession = request.getHttpSession();
	if (httpSession != null) {
		final var servletCtx = ((HttpSession) httpSession).getServletContext();
		// use servletCtx to obtain params, attributes and put them into userProperties
	} else {
		// give up or perform some really ugly hacks...
	}
}

This however requires to enforce creating of session for each handshake request to Endpoints using such Configurator (with Filters or Listeners). This in turn, may not always be acceptable due to various reasons (cookies disabled, user explicitly refusing any data storage etc) and may not serve any other purpose than this server-internal workaround.
Other workarounds include really ugly hacks such as storing a ref to ServletContext on a static var...

Adding getServletContext() removes necessity for any such hacks. This should also be very easy to implement by existing websocket server containers, for example here is an example 6 line patch for Jetty.

Please forgive me, as I'm not sure if just posting a pull-request here is the right way to get something included in this API: if I receive positive initial feedback regarding this, I will get myself familiar with the process and will perform other necessary steps.

Thanks!

@morgwai
Copy link
Author

morgwai commented Aug 19, 2023

An alternative to this PR could be adding a method like Object getWrappedHttpServletRequest() in case it turns there are other usecases that need yet another data obtainable from HttpServletRequest...

@arjantijms
Copy link
Contributor

Object getWrappedHttpServletRequest()

For EE 12 (IFF anyone has the time for it), we talked about a generic representation of the http request. A bare minimum interface that can be used / implemented by Servlet, Portlet, REST and potentially others that have their own request type.

This could perhaps be used here instead of Object.

@joakime
Copy link
Contributor

joakime commented Jun 19, 2025

A bare minimum interface that can be used / implemented by Servlet, Portlet, REST and potentially others that have their own request type.

That kind of generic request object would be better defined by a new Jakarta Spec.
Something like Jakarta Core HTTP.

Note that Jakarta WebSocket could benefit from a generic HTTP Server Request (for websocket server handshake) and a generic HTTP Client Request object (for websocket client handshake).

@joakime
Copy link
Contributor

joakime commented Jun 19, 2025

For the record, getting access to the ServletContext can be done now with any existing Jakarta WebSocket implementation, without forks of the various implementations.

You'll just want to make a ServerEndpointConfig.Configurator instance that has the ServletContext passed into it.
Then use that instance to register your websocket endpoints during the Servlet Context Initialization phase.

Many third party libraries that use Jakarta WebSocket do that now.

@arjantijms
Copy link
Contributor

That kind of generic request object would be better defined by a new Jakarta Spec.
Something like Jakarta Core HTTP.

That's exactly the proposal put on the table of EE 12 ;)

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

Successfully merging this pull request may close these issues.

3 participants