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