Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address ambiguity of <url-pattern> in faces-config.xml #1973

Open
BalusC opened this issue Sep 13, 2024 · 3 comments
Open

Address ambiguity of <url-pattern> in faces-config.xml #1973

BalusC opened this issue Sep 13, 2024 · 3 comments

Comments

@BalusC
Copy link
Member

BalusC commented Sep 13, 2024

eclipse-ee4j/mojarra#5503

The <url-pattern> element is used in faces-config.xml in 2 places:

  1. Resource contracts:
    <contract-mapping>
        <url-pattern>/foo.xhtml</url-pattern>
        <url-pattern>/bar.xhtml</url-pattern>
    </contract-mapping>
  2. Protected views:
    <protected-views>
        <url-pattern>/foo.xhtml</url-pattern>
        <url-pattern>/bar.xhtml</url-pattern>
    </protected-views>

Both have got nothing to do with the <url-pattern> as used in <servlet-mapping> of web.xml as per Servlet spec 12.1. Yet the literal XML element name <url-pattern> is implying that and it's confusing a lot of developers who don't carefully consult the spec/documentation. In reality, they're compared against the Faces view ID (which is always the same irrespective of Faces Servlet mapping used) and they're thus not compared against the Servlet request URI as intuitively expected by those familiar with the Servlet spec.

There's one more difference: the <url-pattern> for resource contracts support wildcards as in * and /subfolder/* (but not *.ext). However the one for protected views doesn't at all support wildcards. This adds yet more confusion. Do note that the * wildcard is even not supported by Servlet spec.

We should probably rename <url-pattern> for clarity even though both features are used very rarely.

Proposal: <view-id-pattern> for those supporting wildcards and <view-id> for those not supporting wildcards.

@tandraschko
Copy link

Why not just allow a real URL pattern? Wildcard matching would be great for directories
AFAIR its even implemented like this in MF

@BalusC
Copy link
Member Author

BalusC commented Sep 14, 2024

Allowing real URL pattern would require more work for implementors and induce yet more ambiguity/confusion for endusers. E.g. when using minimal config, FacesServlet by default already listens on /faces/*, *.faces, *.jsf and *.xhtml. If developers need to register protected views on an actual URL pattern rather than view ID, then that would end up to be cumbersome by being forced to register 4 URL patterns for each single view in order to make sure it's really protected upon every possible request URL referencing the specific view.

In other words, if Servlet 12.1 spec were actually mandated by Faces protected views spec, then in order to completely secure /foo.xhtml with default faces servlet config, you would need:

<protected-views>
    <url-pattern>/foo.xhtml</url-pattern>
    <url-pattern>/foo.jsf</url-pattern>
    <url-pattern>/foo.faces</url-pattern>
    <url-pattern>/faces/foo.xhtml</url-pattern>
</protected-views>

while you should be able to get away with

<protected-views>
    <view-id>/foo.xhtml</view-id>
</protected-views>

I however do agree that it could be beneficial if protected views also support wildcards. But only in flavor of "view ID pattern" not "URL pattern".

<protected-views>
    <view-id-pattern>/protected-views/*</view-id-pattern>
</protected-views>

And that would be a new feature request. The real issue here is only that the term "URL pattern" is technically incorect when looking at the spec and therefore needs to be renamed. For protected views the current Faces spec basically says these should be <view-id> and for resource contracts the current Faces spec basically says these should be <view-id-pattern>.

@BalusC
Copy link
Member Author

BalusC commented Nov 16, 2024

The alternative would be to adjust the spec on this but this ends up ugly as it would still potentially confuse developers who don't carefully read the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants