>>>>> On Sun, 10 Mar 2013 17:44:32 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:
LU> In my opinion, if we can solve this list and the issues related to
LU> faces flow (OutcomeTarget navigation, nested flow return, ... ), I
LU> can finally give my +1 (non binding) to the spec.
Thank you for spelling out so clearly your conditions for your +1. I
will try my best to meet all of them.
LU> MISCELANEOUS
LU> - Some events were added in JSF 2.2:
LU> PostKeepFlashValueEvent
LU> PostPutFlashValueEvent
LU> PreClearFlashEvent
LU> PreRemoveFlashValueEvent
LU> But section 3.4 doesn't mention them. In theory it should be an
LU> small mention in section 3.4.3.1
Fixed.
LU> - JSF 2.2 section 7.1.1 has a spelling bug just before the end:
LU> "... See the Javadocs for getActionListener() for important
LU> backwards compatability information. ..."
LU> It should be "compatibility".
Fixed.
LU> - JSF 2.2 section 7.6.2.3 says that getActionURL() requires to add
LU> the CSRF token as query parameter, but is really that necessary? In
LU> theory, getActionURL is used in forms, that are executed through a
LU> POST request, protected by javax.faces.ViewState parameter (note
LU> this is not true if stateless views uses another generic token as
LU> hidden value for javax.faces.ViewState). Only if getActionURL is
LU> used by something else it has sense. Instead, getBookmarkableURL()
LU> should do that.
We need it to be the case that every URL rendered by JSF is brought
under the control of view protection. This is why I put it in
getActionURL().
LU> - JSF 2.2 section 7.7.2.5 maybe should include UIViewAction description,
LU> in the same way UIViewParameter is described.
Fixed.
LU> - JSF 2.2 section 11.1.3 In javax.faces.FACELETS_DECORATORS it must be
LU> clear what happen when this param is not set. The javadoc of TagDecorator
LU> says "... The runtime must provide a default implementation of this
LU> interface that performs the following actions ..." ,but if a custom
LU> TagDecorator is defined with this parameter, what happens? it is possible
LU> to create a wrapper for TagDecorator?
LU> Note a previous email was sent related to this problem:
LU> [jsr344-experts] javax.faces.view.facelets.TagDecorator
Fixed, per earlier email.
LU> - JSF 2.2 section 11.4.5 : new factories were added like FlashFactory and
LU> FlowHandlerFactory. This part should be updated to reflect these changes.
Assuming you meant 11.4.6, the section where we list what each kind of
factory is supposed to do, fixed.
LU> - JSF 2.2 section 11.4.7 Delegating Implementation Support
LU> Is FlowHandlerFactory decoratable? if that so, it should be included in the
LU> list.
Yes it is. Fixed.
LU> - JSF 2.2 section 7.7.2.1 has an spelling bug on "... and store the results
LU> as the values of the locale and renderKitId, proeprties ..." It should say
LU> "properties".
Fixed.
LU> RESOURCE LIBRARY CONTRACTS
LU> - JSF 2.2 section 5.6.2.5 Resource ELResolver
LU> In getValue description it says something like this:
EB> "... If property contains a single colon character ':', treat the
EB> content before the ':' as the libraryOrContractName and the content
EB> after the ':' as the resourceName and pass both to
EB> ResourceHandler.createResource(resourceName,
EB> libraryOrContractName). If the value of libraryOrContractName is the
EB> literal string "this" (without the quotes), discover the library
EB> name of the current resource (or the contract name of the current
EB> resource, the two are mutually exclusive) and replace "this" with
EB> that library name (or contract name) before calling
EB> ResourceHandler.createResource( ). If property contains more than
EB> one colon character ':', throw a localized ELException, including
EB> property ..."
LU> The problem here is the contractName is something that is implicit derived
LU> from the view itself (using the viewId and then calling
LU> VDL.calculateResourceLibraryContracts(FacesContext context, String viewId) )
LU> and it does not have any use on the implicit notation, because the idea
LU> of a resource library contract is to be on top of the resource resolution.
I have found that, in practice, this was necessary to allow refering to
resources from CSS files from within a contract.
[...]
LU> In conclusion, the description on JSF 2.2 section 5.6.2.5 should be
LU> removed.
Well, we can't remove it because the ResourceELResolver is something we
added in JSF 2.0. I have put the text back to its state from the most
recent published JCP final document, the JSF 2.1 MR release from
20101108. I have also added this text at the end of that paragraph (the
one that starts with "if property contains a single colon...":
In the case of resource library contracts, libraryName will actually be
the contract name.
LU> - Javadoc of ResourceHandler.createViewResource() still has a pending part.
LU> In theory, it should scan META-INF/contracts/[contract-name],
LU> META-INF/flows/[flow-name] and webapp context. It should say something like
LU> "... the default implementation must ....".
I have fixed this and will commit it with the rest of the changes in
response to this email.
LU> - UIViewRoot does not have a method to store the associated resource library
LU> contracts once are calculated. This is valid if and only if the result of
LU> VDL.calculateResourceLibraryContracts() is the same over the application
LU> lifetime, otherwise there is a risk of make some views unstable. Note
LU> ResourceHandler requires call this method to derive the contract in the
LU> same way it is done for the locale, so at the end the preferred way will be
LU> store the result of the call inside UIViewRoot using a custom attribute,
LU> so if the attribute is present, ResourceHandler will get the value and avoid
LU> the calculation.
We specifically discussed this and decided to make the FacesContext be
the place where this information is stored.
LU> JSF 2.2 section 10.1.3.2 says this:
LU> "... The specification of the tag handler for <f:view> is the one other place
LU> where the resourceLibraryContracts property may be set. This behavior is
LU> normatively specified in the tag handler for <f:view>. ..."
LU> The facelets javadoc of f:view defines an attribute called contracts:
LU> "... A comma separated list of resource library contracts that may be used from
LU> within the Facelets chain. If this attribute is present, it must only be on the
LU> outer-most file in the chain of files that started ultimately with a call to
LU> ViewDeclarationLanguage.createView(). Any use of this attribute on a
LU> non-outer-most file must be silently ignored. ..."
LU> If in f:view there exists the attribute "contracts" and the spec says that a
LU> contract may be set in that location, the missing part is the description that
LU> should say "copy the result of calculateResourceLibraryContracts() in some
LU> UIViewRoot attribute". JSF 2.2 section 7.7.2.1 says that it is necessary
LU> to call calculateResourceLibraryContracts() but it doesn't say what to do with
LU> the result. According to the intention written into the spec, one question
LU> arise,
LU> * Does the result of the assigment of f:view "contracts" attribute override
LU> or add the contract valids for an specific view against
LU> calculateResourceLibraryContracts()? .
Yes, it does.
LU> In my opinion, f:view "contracts" should override the result of
LU> calculateResourceLibraryContracts(), just like f:view locale does.
LU> In conclusion. UIViewRoot should provide a property for
LU> resourceLibraryContracts that can be called from ResourceHandler and JSF 2.2
LU> section 7.7.2.1 needs to be updated to reflect the fact that the result of
LU> calculateResourceLibraryContracts() needs to be copied in that attribute of
LU> UIViewRoot, so f:view tag can later override it.
I'm going to modify the TLDDoc for f:view contracts to clarify that it
overrides whatever else is there.
LU> STATELESS JSF
LU> - If a stateless view is protected, according to
LU> ResponseStateManager.writeState() javadoc:
LU> "... Call FacesContext.getViewRoot(). Call StateHolder.isTransient()
LU> returns true, take implementation specific action so that the following
LU> call to isStateless(javax.faces.context.FacesContext, java.lang.String)
LU> returns true and return. Otherwise, proceed as follows. ..."
LU> This creates a conflict with CSRF protection, because one assumption
LU> already made is that javax.faces.ViewState hidden field is always
LU> generated and included and in that way it protects POST request. Note
LU> also that a postback is detected with the presence of
LU> javax.faces.ViewState in the request (javadoc of
LU> ResponseStateManager.isPostback() )
LU> "... The implementation of this method for the Standard HTML RenderKit
LU> must consult the ExternalContext's requestParameterMap and return true
LU> if and only if there is a key equal to the value of the symbolic
LU> constant VIEW_STATE_PARAM. ..."
LU> In conclusion:
LU> 1. Stateless views must still write a dummy javax.faces.ViewState field
LU> to detect postback.
Yes, the stateless view already does that. We use the existing
ViewState field for stateless as well as non-stateless.
LU> 2. Stateless views that needs to be protected against CSRF attack, must
LU> include its token in javax.faces.ViewState field as if it was the view
LU> state, but only contains the info related to the secret stored in session
LU> or encoded/encrypted when client side state saving is used. In few words
LU> if the view is protected, javax.faces.ViewState generation may requires
LU> additional steps.
I'm glad you brought up this interaction. I have filed
<
http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1171>.
LU> FACES FLOW
LU> - javadoc of FlowHandler.clientWindowTransition() still mentions
LU> Lifecycle.attachWindow(javax.faces.context.FacesContext), but now
LU> it is called from Restore View Phase directly.
Fixed.
LU> - JSF 2.2 section 7.1.3 FlowHandler Property
LU> It says this about Application.getFlowHandler() and setFlowHandler(...):
LU> "... [P1-start flowHandler called after startup] This method may only be
LU> called at application startup, before any Faces requests have been processed.
LU> [P1-end] This is a limitation of the current Specification, and may be lifted
LU> in a future release ..."
LU> In theory setFlowHandler() should be called at application startup but
LU> getFlowHandler() should be called after application startup. setFlowHandler()
LU> should not be called after application startup.
Clarified.
LU> - JSF 2.2 section 11.4.3.2 and 11.4.3.3
LU> If every flow should be declared in a faces-config.xml file, it means there is
LU> no need to scan the classpath or directories to found every available flow in
LU> a webapp, because with the list of the flow names it is possible to derive the
LU> locations where the views related to a flow really are. That's ok.
LU> The problem is, if the views related to the flow are handled by
LU> ResourceHandler.createViewResource(), FlowHandler doesn't expose that list,
LU> so in this point the default algorithm should depend on implementation
LU> details to get the list and know the possible valid paths where it is possible
LU> to look for view files and other resources.
Fixed.
Committed as r11731.
Ed
--