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 getCryptographicallyStrongTokenForWebsocketSession(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
>>
>>