jsr372-experts@javaserverfaces-spec-public.java.net

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

From: Bauke Scholtz <balusc_at_gmail.com>
Date: Sat, 15 Oct 2016 08:19:52 +0200

Hi,

As to clustering, do server and WS specific specifics really have to end up
in JSF specification too?

As to auto-reconnect (not auto-connect), yes I agree that some API-provided
configuration settings would be useful. Mojarra's implementation does 25
attempts with an incremental interval of 500ms, starting with 0ms. I only
wonder what parts should be configureable by API. The amount of reconnect
attempts? If zero or negative, then it's considered as disabled.

Cheers, B


On Fri, Oct 14, 2016 at 8:06 PM, Leonardo Uribe <leonardo.uribe_at_irian.at>
wrote:

> Hi
>
> BS>> As to clustering, see writeObject() and readObject() methods of
> OmniFaces
> BS>> o:socket SocketChannelManager class (on which Mojarra's f:websocket
> BS>> WebsocketChannelManager class is largely based).
>
> Please note I'm writing an implementation of f:websocket from scratch, so
> I'm
> trying do not see omnifaces code. I have checked the methods and anyway it
> doesn't solve the problem related to Session migration, because this code
> is
> not detecting when the Session instance has been migrated. But it supports
> the
> point that the token is not application scope, because you need to move
> the related data using a session scope bean. But as I said, this is
> implementation specific, doesn't change the spec.
>
> BS>> As to dropping "connected", I don't understand why you consider it an
> BS>> application wide setting instead of a socket specific setting. How do
> you
> BS>> want to let the client have control over manually opening a specific
> BS>> socket via jsf.push.open("channel") during e.g. an onclick event
> only?
> BS>> See also javax.faces.push.Push javadoc and o:socket showcase.
>
> My mistake here, I just read this in the javadoc:
>
> "... You can use the optional connected attribute to control whether to
> auto-connect the web socket or not. ..."
>
> But I was thinking about how the websockets connects in case of a
> disconnection between the client and the server.
>
> In other words, what I wanted to say is the way how the client attempts to
> reconnect to the server is something that you would like to control using
> a
> web config param. How many attemps, the timeout and so on. Maybe pass
> these config as an array in jsf.push.init(...) is a good idea.
>
> Please note I haven't implemented this attribute on my code yet, so I'm
> reporting on the mailing list what I'm seeing. I have not checked the
> showcase, because I do not want to bias my own judgment. The idea is
> the documentation in the spec should be good enough to give an idea
> about how it works. That's the way we ensure spec's quality. So, I have
> not digested every single line, but I'm on the way.
>
> regards,
>
> Leonardo Uribe
>
> 2016-10-14 2:39 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
>
>> Hi,
>>
>> 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>
>> wrote:
>>
>>> 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
>>>>>
>>>>>
>>>
>>
>