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

Passing values from ServerEndpointConfig.Configurator to @ServerEndpoint #235

Closed
glassfishrobot opened this issue Mar 13, 2015 · 10 comments
Closed
Labels
API (Server) enhancement Adding a new feature or improving an existing one Jakarta EE 10

Comments

@glassfishrobot
Copy link

As discussed in http://stackoverflow.com/questions/28939581/access-useragent-in-websocket-session/28970245 :

There is no proper way to pass information from a ServerEndpointConfig.Configurator's modifyHandshake() method to the @OnOpen/@OnMessage methods in the @serverendpoint annotated class.

Such a possibility would allow to e.g. retrieve the User-Agent HTTP header from the HandshakeRequest inside the modifyHandshake() method and then use it later on in the ServerEndpoint.

Currently in modifyHandshake() we have:

  • The ServerEndpointConfiguration - which is shared among all endpoint instances and multiple upgrade requests can be done concurrently
  • The HandshakeRequest object
    • getHeaders() - read-only
    • getHttpSession() - empty on Java SE
    • getParameterMap - read-only
  • The HandshakeResponse object which only adds HTTP headers

I'm not sure if the getEndpointInstance() method can be called before modifyHandshake() and the actual handshake completes so that the Session could be made available as optional parameter.

Maybe the HandshakeRequest could at least be made an optional parameter of @onopen? This would be similar to #218 and probably solve most use cases where people just want to get information about the HTTP headers.

Affected Versions

[1.1]

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Reported by lathspell42

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Issue-Links:
is related to
TYRUS-396

@glassfishrobot
Copy link
Author

glassfishrobot commented Feb 10, 2017

@glassfishrobot Commented
slominskir said:
The following is a workaround using ThreadLocal that seems to work across all implementations:

RequestDataServerEndpointConfigurator.java

public class RequestDataServerEndpointConfigurator extends ServerEndpointConfig.Configurator {

    @Override
    public void modifyHandshake(ServerEndpointConfig config, HandshakeRequest request, HandshakeResponse response) {

        Map<String, List<String>> headers = request.getHeaders();
        String remoteAddr = (String) ((HttpSession) request.getHttpSession()).getAttribute("remoteAddr");

        // We don't use config.getUserProperties.add because it isn't always one-to-one with a web socket connection; we use ThreadLocal instead
        WebSocketRequestDataContext.setCurrentInstance(new WebSocketRequestDataContext(headers, remoteAddr));
    }
}

WebSocketRequestDataContext.java

public class WebSocketRequestDataContext {

    private final Map<String, List<String>> headers;
    private final String remoteAddr;
    private static final ThreadLocal<WebSocketRequestDataContext> INSTANCE
            = new ThreadLocal<WebSocketRequestDataContext>() {

        @Override
        protected WebSocketRequestDataContext initialValue() {
            return null;
        }
    };

    public WebSocketRequestDataContext(Map<String, List<String>> headers, String remoteAddr) {
        this.headers = headers;
        this.remoteAddr = remoteAddr;
    }

    public Map<String, List<String>> getHeaders() {
        return headers;
    }

    public String getRemoteAddr() {
        return remoteAddr;
    }

    public static WebSocketRequestDataContext getCurrentInstance() {
        return INSTANCE.get();
    }

    public static void setCurrentInstance(WebSocketRequestDataContext context) {
        if (context == null) {
            INSTANCE.remove();
        } else {
            INSTANCE.set(context);
        }
    }
}

MyEndpoint.java

@ServerEndpoint(value = "/myendpoint", configurator = RequestDataServerEndpointConfigurator.class)
public class MyEndpoint {

    @OnOpen
    public void open(Session session, EndpointConfig config) {
            WebSocketRequestDataContext context = WebSocketRequestDataContext.getCurrentInstance();

            if (context != null) {
Map<String, List<String>> headers = context.getHeaders();
String remoteAddr = context.getRemoteAddr();

session.getUserProperties().put("headers", headers);
session.getUserProperties().put("remoteAddr", remoteAddr);

// Prevent classloader leak
WebSocketRequestDataContext.setCurrentInstance(null);
            }
        }
    }

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
This issue was imported from java.net JIRA WEBSOCKET_SPEC-235

@glassfishrobot
Copy link
Author

@jcompagner
Copy link

is there any development on these specs?
Because i think this feature is quite important, now many people really just use those userProperties() and that is very dangerous.
Maybe we can even make the implementation of that user properties that it is always a copy? then nothing api needs to change, except maybe the doc that the user properties are always a copy that can be used from the configurator to the end point and values that you put in are unique for that end point creation.
Or is that set user property really used at that point?

@joakime
Copy link
Contributor

joakime commented Feb 8, 2019

UserProperties being a copy or not is actually not defined in the current spec nor the TCK.

Some implementations always treat User Properties as a copy per Session, others do not.
Those implementations that want to carry over information from the handshake (request headers / httpsession / cookies / etc) to the Endpoint have already implemented UserProperties as a copy.
Those implementations that cannot carry over information from the handshake are treating UserProperties as a singleton per Endpoint Config.

I'd vote for making UserProperties a copy per Endpoint instance.
That way the HttpSession suggestions found on stackoverflow would still be valid.
eg: https://stackoverflow.com/questions/17936440/accessing-httpsession-from-httpservletrequest-in-a-web-socket-serverendpoint/17994303#17994303

@markt-asf
Copy link
Contributor

+1 to explicitly specifying that UserProperties are per Endpoint instance / WebSocket session.

@jansupol
Copy link
Contributor

jansupol commented Mar 1, 2022

The simplest way of having the properties per Session:

class MyServerEndpointConfig implements ServerEndpointConfig {
   public void beforeHandShake() {
     // re-initialize properties
   }
   ....
}

class MyConfigurator extends ServerEndpointConfig.Configurator {
   public void modifyHandshake(ServerEndpointConfig sec,...) {
        if (MyServerEndpointConfig.class.isInstance(sec)) {
            ((MyServerEndpointConfig) sec).beforeHandShake();
        }
        ....
   }
}

@markt-asf
Copy link
Contributor

That approach isn't thread safe.
I think it would be best to keep the discussion in a single place rather than the three different discussions that have been started. As this issue has already been closed, the TCK challenge is the best place for any further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API (Server) enhancement Adding a new feature or improving an existing one Jakarta EE 10
Projects
None yet
Development

No branches or pull requests

5 participants