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

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

From: Leonardo Uribe <leonardo.uribe_at_irian.at>
Date: Thu, 20 Oct 2016 21:08:52 -0500

Hi

I just wanted to mention the declaration of jsf.push.init(...) that
works better in my personal opinion, based on the previous
discussions:

jsf.push.init(clientId, url, onopen, onmessage, onclose,
                behaviorScripts, connected);

Example:

<div>
    <h:outputText id="currentTime" value="#{clockBean.currentTime}"/>
</div>

<f:websocket channel="clock" scope="application"
             onopen="socketOpen"
             onclose="socketClose">
    <f:ajax event="showCurrentTime"
            execute="currentTime"
            render="currentTime"/>
</f:websocket>

Generates this script:

<script type="text/javascript"><!--
jsf.push.init('mainForm:j_id_p',

'/test-webapp-cdi/javax.faces.push/clock?e82c26b5ccc2a5651571de6818cbfab0',
             socketOpen,
             null,
             socketClose,
             {showCurrentTime:[function(event){

jsf.ajax.request('mainForm:j_id_p',event,{execute:'mainForm:currentTime
',render:'mainForm:currentTime
','javax.faces.behavior.event':'showCurrentTime'})}
              ]},true);
//--></script>

Only if the port is set in the global web config param, the url will
include it full.

The only thing to mention is the channel name and the channel token are
derived from the URL, which means the URL should always end in that way
if it is overriden (portlet case usually overrides everything).
The other option is pass both as parameters of jsf.push.init(...), but maybe
that's not necessary, better to keep it simple.

regards,

Leonardo Uribe


2016-10-18 15:33 GMT-05:00 Leonardo Uribe <leonardo.uribe_at_irian.at>:

> Hi
>
> BS>> As to clustering, do server and WS specific specifics really have to
> end up in JSF specification too?
>
> No, instead we do what we can with what we have. This is not something
> that we can fix from here.
>
> But we could ask websocket api EG about this for clarification.
>
> BS>> As to auto-reconnect (not auto-connect), yes I agree that some
> API-provided configuration settings
> BS>> would be useful. Mojarra's implementation does 25 attempts with an
> incremental interval
> BS>> of 500ms, starting with 0ms. I only wonder what parts should be
> configureable by API. The
> BS>> amount of reconnect attempts? If zero or negative, then it's
> considered as disabled.
>
> I was thinking about use a exponential backoff algorithm. But the problem
> here is it depends of what
> the developer wants. If he/she wants real time, the reconnection could be
> different. The key point is
> this could be a candidate parameter to be decided at deployment stage. If
> this is a config parameter,
> you can override it in web.xml without change the code. But if this is not
> considered relevant for
> the spec, we can just let this as an implementation detail.
>
> This is a design decision. My only intention here is to mention it.
>
> regards,
>
> Leonardo Uribe
>
>
> 2016-10-15 1:19 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
>
>> 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
>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>
>>
>