Reassess that 'View Protection' feature is working as specified

[BalusC]

As per jakartaee/faces#1369 the 'View Protection' implementation in Mojarra has at first sight incorrect behavior when virtual URLs are used. We need to reassess if this is as per the spec and create TCK-compatible integration tests and eventually fix the implementation where necessary.

[BalusC]

Facts:

1. <protected-views> element in faces-config.xml expects child elements with tag name <url-pattern>, hereby implying that those are Servlet-specific "URL patterns" and thus not Faces-specific "view IDs" (defined in 2.2.1)
2. The web-facesconfig_4_0.xsd literally says "The "url-pattern" element specifies the collection of views [...]" so hereby implying that those should represent Faces-specific "view IDs" and thus not Servlet-specific "URL patterns".
3. The Faces spec explicitly refers to servlet spec when saying "URL pattern" in two places: 11.1.12 and 11.2.6.2, both related to FacesServlet mapping, but the other two occurrences saying "<url-pattern>" don't explicitly refer to servlet spec: 7.7.2.2 and 11.3.2.1, they instead refer to other section saying it should be compared against "view ID".
4. The ViewHandler#getProtectedViewsUnmodifiable() is mentioned in 2.2.1 but no mention of URL pattern is made while the javadoc in turn says "Compliant implementations must return a Set that is the concatenation of the contents of all the <url-pattern> elements within all the <protected-views> in all of the application configuration resources in the current application.", so they are used "plain" without taking into account any FacesServlet mapping.
5. The source code of getActionURL(), where protected views are checked, is in Mojarra implemented as follows:

   Set<String> urlPatterns = viewHandler.getProtectedViewsUnmodifiable();
   
   // Implement section 12.1 of the Servlet spec.
   if (urlPatterns.contains(viewId)) {
   

   Note the the surprising comment. This isn't explicitly requested in the Faces spec but the impl itself actually also doesn't adhere section 12.1 of the Servlet spec (view ID is compared directly against <protected-views><url-pattern> instead of that request URI is compared against <protected-views><url-pattern> according Servlet spec URL pattern matching rules with wildcards and all!). This is most probably a leftover of initial attempts / proof of concepts whereby the original developer / spec author in the end ultimately decided to actually treat them as view IDs instead of URL patterns and then forgot to remove the comment line.

I'm not sure how we ended up here with such an ambiguous spec. But if we listen to the Faces spec carefully and literally, then the "URL pattern" / <url-pattern> is in the context of protected views (and resource contracts!) actually a misnomer for "View ID" / <view-id>. In other words, we should be re-interpreting them as view IDs. So we should be specifying <url-pattern>/foo.xhtml</url-pattern> when we want to make the view ID /foo.xhtml a protected view, and this is irrespective of the FacesServlet mapping on *.xhtml, *.jsf, /faces/*, etc. So it should always be protected irrespective of whether you hit it as /contextpath/foo.xhtml, /contextpath/foo.jsf, /contextpath/faces/foo.xhtml, etc. In retrospect I think this is a smart desicion. This way we don't need to change the protected views config in case every time we happen to use a different FacesServlet mapping or even add multiple mappings.

I do admit all of this is confusing to starters. All we can do to improve the Faces spec is one of the following:

1. Deprecate <url-pattern> and rename it to <view-id> for clarity.
2. Or, explicitly mention that impl must actually support Servlet 12.1 spec on parsing and comparing the <url-pattern> values against the view ID.

I think option 1 is the way to go.

cc: @netomi

[netomi]

ty for taking care of this!