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: Tue, 18 Oct 2016 15:33:35 -0500

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