From: Bauke Scholtz <balusc_at_gmail.com>
Date: Thu, 08 Dec 2016 07:10:32 +0000

Hi,

API looks good. However, is there any technical reason you use
getServerName() instead of extracting it from getRequestURL()? The latter
is more reliable as this represents the URL that actually hit the server
instead of a server environment dependent configuration setting.

Cheers, B

On Thu, Dec 8, 2016, 04:55 Leonardo Uribe <leonardo.uribe_at_irian.at> wrote:

> Hi Bauke
>
> I have this declaration in MyFaces:
>
> public String getWebsocketURL(FacesContext context, String channel, String
> channelToken)
>
> This is the way I have used it in MyFaces implementation of f:websocket :
>
> In the Renderer instance:
>
> sb.append("'"+facesContext.getExternalContext().encodeWebsocketURL(
> facesContext.getApplication().getViewHandler().getWebsocketURL(
> facesContext, component.getChannel(), (String)
> component.getValue()))+"'");
>
> In ViewHandler:
>
> /**
> * Return a JSF URL that represents a websocket connection for the
> passed channel and channelToken
> *
> * @since 2.3
> * @param context
> * @param channel
> * @param channelToken
> * @return
> */
> public String getWebsocketURL(FacesContext context, String channel,
> String channelToken)
> {
> String url = context.getExternalContext().getRequestContextPath()
> +
> "/javax.faces.push/"+channel+"?"+channelToken;
> return url;
> }
>
> In ExternalContext
>
> public String encodeWebsocketURL(String baseUrl)
> {
> Integer port = WebConfigParamUtils.getIntegerInitParameter(
> _currentFacesContext.getExternalContext(),
> ViewHandler.WEBSOCKET_PORT);
> port = (port == 0) ? null : port;
> if (port != null &&
>
> !port.equals(_currentFacesContext.getExternalContext().getRequestServerPort()))
> {
> String scheme =
> _currentFacesContext.getExternalContext().getRequestScheme();
> String serverName =
> _currentFacesContext.getExternalContext().getRequestServerName();
> String url;
> try
> {
> url = new URL(scheme, serverName, port,
> baseUrl).toExternalForm();
> url = url.replaceFirst("http", "ws");
> return ((HttpServletResponse)
> _servletResponse).encodeURL(url);
> }
> catch (MalformedURLException ex)
> {
> //If cannot build the url, return the base one unchanged
> return baseUrl;
> }
> }
> else
> {
> return baseUrl;
> }
> }
>
> I think there are two different concepts: the "channelName" and the
> "channelToken". The "channelName" or "channel" is the symbolic name to
> identify a channel, but the "channelToken" is meant to setup the context
> of
> that channel. You can join both in a single string, but that means you are
> writing a part of the url (the "?" between both).
>
> ExternalContext.encodeWebsocketURL(String baseUrl) is a point where the
> implementation could add a host/port if required. The idea is both
>
> /test-webapp-cdi/javax.faces.push/clock?e82c26b5ccc2a5651571de6818cbfab0
>
> and
>
>
> ws://localhost:4566/test-webapp-cdi/javax.faces.push/clock?e82c26b5ccc2a5651571de6818cbfab0
>
> are valid encoded urls.
>
> regards,
>
> Leonardo Uribe
>
> 2016-12-07 4:09 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
>
> Hi all and specifically Leo,
>
> How about this ViewHandler#getWebsocketURL() spec? I just copypasted from
> getResourceURL() and altered where necessary. I myself only find the
> wording of "will select the argument xxx for" somewhat unclear, but it is
> as how those other getXxxURL() are specified.
>
> /**
> * <p class="changed_added_2_3">If the value returned from this
> * method is used as the <code>file</code> argument to the
> * four-argument constructor for <code>java.net.URL</code> (assuming
> * appropriate values are used for the first three arguments), then
> * a client making a websocket push handshake request to the
> <code>toExternalForm()</code> of
> * that <code>URL</code> will select the argument <code>channel</code>
> * for connecting the websocket push channel in the current view.
> * It must match the {_at_link PushContext#URI_PREFIX} of the
> endpoint.</p>
> *
> * @param context {_at_link FacesContext} for the current request.
> * @param channel The channel name of the websocket.
> *
> * @throws NullPointerException if <code>context</code> or
> * <code>channel</code> is <code>null</code>.
> *
> * @return the websocket URL.
> * @see PushContext#URI_PREFIX
> */
> public abstract String getWebsocketURL(FacesContext context, String
> channel);
>
>
> A reference implementation would be this:
>
> @Override
> public String getWebsocketURL(FacesContext context, String channel) {
> requireNonNull(context, "context");
> requireNonNull(channel, "channel");
>
> String contextPath =
> context.getExternalContext().getRequestContextPath();
> return contextPath + PushContext.URI_PREFIX + "/" + channel;
> }
>
>
> Cheers, B
>
>
> On Tue, Dec 6, 2016 at 4:25 PM, Bauke Scholtz <balusc_at_gmail.com> wrote:
>
> Hi,
>
> As per
> https://java.net/projects/mojarra/sources/git/revision/acb658925ed54206f0844f7eb88d5035218e22b3
> the WebsocketHandler has been turned into UIWebsocket implementing
> ClientBehaviorHolder. Now f:ajax is supported as per below javadoc update:
>
> * <h3 id="ui"><a href="#ui">Ajax support</a></h3>
> * <p>
> * In case you'd like to perform complex UI updates depending on the
> received push message, then you can nest
> * <code>&lt;f:ajax&gt;</code> inside <code>&lt;f:websocket&gt;</code>.
> Here's an example:
> * <pre>
> * &lt;h:panelGroup id="foo"&gt;
> * ... (some complex UI here) ...
> * &lt;/h:panelGroup&gt;
> *
> * &lt;h:form&gt;
> * &lt;f:websocket channel="someChannel" scope="view"&gt;
> * &lt;f:ajax event="someEvent" listener="#{bean.pushed}"
> render=":foo" /&gt;
> * &lt;/f:websocket&gt;
> * &lt;/h:form&gt;
> * </pre>
> * <p>
> * Here, the push message simply represents the ajax event name. You can
> use any custom event name.
> * <pre>
> * someChannel.send("someEvent");
> * </pre>
>
> And, jsf.push.init/open/close now take client ID instead of channel name.
>
> One thing that's still open is having a ViewHandler#getWebsocketURL(). I
> will work on that later this week.
>
> Cheers, B
>
> On Fri, Oct 28, 2016 at 5:54 AM, Leonardo Uribe <leonardo.uribe_at_irian.at>
> wrote:
>
> Hi
>
> Good to know that. Thanks!
>
> regards,
>
> Leonardo Uribe
>
>
> 2016-10-27 3:07 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
>
> 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
> 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
>
>
>
>
>
>
>
>
>
>
>
>
>