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: Bauke Scholtz <balusc_at_gmail.com>
Date: Thu, 27 Oct 2016 10:07:59 +0200

I have reopened
https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1396

Cheers, B

On Fri, Oct 21, 2016 at 4:08 AM, Leonardo Uribe <leonardo.uribe_at_irian.at>
wrote:

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