[jsr372-experts] Re: [jsr372-experts mirror] Re: problems with jsf.push.init(...) specification and token generation

From: Bauke Scholtz <balusc_at_gmail.com>
Date: Fri, 14 Oct 2016 09:39:11 +0200


As to clustering, see writeObject() and readObject() methods of OmniFaces
o:socket SocketChannelManager class (on which Mojarra's f:websocket
WebsocketChannelManager class is largely based).

As to dropping "connected", I don't understand why you consider it an
application wide setting instead of a socket specific setting. How do you
want to let the client have control over manually opening a specific socket
via jsf.push.open("channel") during e.g. an onclick event only? See also
javax.faces.push.Push javadoc and o:socket showcase.

Cheers, B

On Fri, Oct 14, 2016 at 4:19 AM, Leonardo Uribe <leonardo.uribe_at_irian.at>

> Hi
> BS>> I do agree that the full url should be generated by the server
> BS>> and passed to the client. I will look into it.
> Ok, thanks. I just want to add that the "port" attribute is something that
> should be set using a global web config param, because this is common
> for all the application and depends on the deployment configuration.
> Please also consider if "connect" can be a global web config param instead
> an attribute too.
> BS>> The "token" is application scoped not session scoped.
> The logic behind this is something I have been thinking for a long time.
> Before start, I want to be clear that the suggested proposal is provide
> a point in the spec to allow customize the token generation by security
> purposes. The way how a token is used is considered an implementation
> specific concern, but in order to enrich the understanding of this
> component I'll put my thoughts about this here.
> There is a precedent that suggest an application scope token is not a good
> idea. See:
> - https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-694
> Current Flash specification breaks clustering
> - https://java.net/jira/browse/JAVASERVERFACES-1449
> Modify flash to support clustering
> The description says: "... Currently the flash implementation is entirely
> stored in the Application scope. This makes it unclusterable. Make it so
> data stored in the flash is cluster friendly. ..."
> Now, as you already know, f:websocket requires to register an Endpoint
> and in that place there is no HttpSession (there is on handshake),
> so from that point the only valid CDI beans are application scoped.
> So, the active websocket Session instances should be registered in an
> application scoped bean.
> There are two points to keep in mind:
> - Clustering relies on session replication, and application scope is not
> replicated among nodes.
> - Application scope means a potential for memory leaks, if the map does not
> have proper cleanup measures.
> If you make the token application scope, that means multiple views share
> the
> same token, but it also means there is no way to keep track of each
> connection in the application scope bean. So, at the end you will end up
> with
> stale session instances, and that's not good.
> If the websocket lifetime is shorter than the view lifetime, it is only
> natural that the token should be view scope. That means, for each
> combination
> of channel, scope and user it should be one channel token per view.
> The channel token is generated on render response time, so the way to do it
> is generate the token and store it in a view scope bean. If @PreDestroy is
> properly implemented for view scope beans, we can rely on that to destroy
> in
> a ordered way the token and the associated websocket Session instances
> from
> the application scope bean. If the session is destroyed, @PreDestroy will
> be called anyway, so with this we can avoid the memory leak.
> If you see the problem from a "cluster" point of view, if the token is
> application scope and the session is migrated to another node, the token
> will
> not be registered in the application scope of the target node, so the
> connection will not be accepted. Please note there is no CDI session scope
> in Endpoint.onOpen, but there is on the handshake request, so the solution
> is to register the valid tokens on session scope, so they can be validated
> later in that point. It could be variants to avoid store the tokens on
> session scope, but the problem is this feature is "CDI centric", so it is
> hard to initialize a FacesContext instance on the Endpoint. It can be done,
> but we need more "plumbing" to do it. In that case, we need a method
> on ViewHandler to validate the websocket token (remember
> ResponseStateManager is tied to RenderKit, and here there is view and
> no renderkit reference).
> Websocket spec says this about clustering:
> "... Websocket implementations that are part of a distributed container
> may
> need to migrate websocket sessions from one node to another in the case of
> a
> failover. ..."
> This blows up any storage of Session into an application scope bean,
> because
> it says the session can be moved between nodes, so in that case, the
> migrated
> Session will not be registered into the application scope bean. But this
> can be accepted, because probably in that case the websocket connection
> could be reopened, and the problem silently dissapear. This could be a
> potential websocket spec pitfall, where the spec says that the websocket
> sessions can be moved between nodes, but does not provide a way to identify
> when this has happened and take proper actions (like register the Session
> in
> the application scope bean).
> In conclusion, the important thing about the websocket token is that it
> helps
> to identify the connection and its associated properties. If two
> f:websocket
> declarations share the same channel, user and scope, both should have the
> same token "at least" at view level. I think both implementations can
> agree
> on that assumption.
> No, thinking on jsf.push.init(...) I would like to propose this structure
> for
> the EG consideration:
> jsf.push.init(clientId, url, onopen, onmessage, onclose, behaviorScripts);
> removing "port" and "connected" (use web config parameters for that).
> The clientId helps to identify from which tag these properties belong, and
> make possible to reuse the same websocket connection among f:websocket
> declarations. I'll explain this in another email, this is a work in
> progress,
> but for now I'm sure we could agree with this function declaration.
> regards,
> Leonardo Uribe
> 2016-10-13 2:10 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
>> Hi,
>> The "token" is application scoped not session scoped. I do agree that the
>> full url should be generated by the server and passed to the client. I will
>> look into it.
>> Cheers ,B
>> On Tue, Oct 4, 2016, 05:36 Leonardo Uribe <leonardo.uribe_at_irian.at>
>> wrote:
>>> Hi
>>> Following a more exhaustive review of f:websockets, I have found some
>>> inconsistencies related to the way how a connection is specified.
>>> Please note there is no mention of jsf.push.init(...) in the
>>> javascript documentation, but the package jsf.push is there.
>>> Here is an example:
>>> <script type="text/javascript">
>>> jsf.push.init(8080,'clock?1e4ac202-2b21-434f-8431-de5d5e4d37fe',
>>> socketOpen,socketListener,socketClose,true);</script>
>>> If you use "jsf" javascript package, every method there should be
>>> specified
>>> without exceptions. I know this feature is a "work in progress" but I
>>> just
>>> wanted to notice this fact.
>>> Other thing to notice is there is a session token used to identify the
>>> connection. This is something related to state management, so I was
>>> expecting
>>> a method inside ResponseStateManager, in the same way there is a method
>>> there called:
>>> public String getCryptographicallyStrongTokenFromSession(FacesContext
>>> context)
>>> to handle protected views. Something like:
>>> public String getCryptographicallyStrongToke
>>> nForWebsocketSession(FacesContext context)
>>> It is only the token generation what we need here, not the websocket
>>> session handling.
>>> At last, there is a problem with passing params to assemble the url used
>>> for the connection in the client. It has been a long standing policy in
>>> JSF
>>> to generate urls on the server and pass them to the client. But in the
>>> javascript documentation a new variable was added called:
>>> jsf.contextpath
>>> with description "... The result of calling
>>> ExternalContext.getRequestContextPath(). ..."
>>> It looks like something recently added, so I guess it should be something
>>> related to jsf.push.init(...) script.
>>> The problem here is there is no way for the server to override this path,
>>> and in cases like portlets for example, it is necessary to provide a way
>>> to override this part properly. So the url used to connect must be built
>>> fully on the server and use the known ViewHandler - ExternalContext
>>> pattern. Additionally, embed this variable in the js file breaks any
>>> caching strategy. If the content is dynamic, embed it as an inline
>>> script,
>>> but in this case it is better to avoid that.
>>> regards,
>>> Leonardo Uribe