jsr372-experts@javaserverfaces-spec-public.java.net

[jsr372-experts] Re: [1359-views] DISCUSSION

From: Edward Burns <edward.burns_at_oracle.com>
Date: Wed, 4 Mar 2015 14:10:03 -0800

Hello Volunteers,

Before I continue on this thread, I want to say that I view it as part
of my job to push back by default against the natural desire to revise
the spec so it is 'the way it should have been in the first place'
(though I admit no-one is saying that explicitly) while at the same time
allowing changes where they make sense and add value.

>>>>> On Thu, 26 Feb 2015 11:06:39 +0100, Frank Caputo <frank_at_frankcaputo.de> said:

EB> Conceptually, I do not look at the Facelet files in a contract as a top
EB> level view. Thus, I would be in favor of tightening the spec to say
EB> that Facelet files in a contract must not be loadable as top level
EB> views.

EB> Frank, are you ok with that?

FC> Unfortunately a customer of mine is using this functionality. They
FC> have a contract for mobile clients and they need the possibility to
FC> replace complete views. This is the project, from which resource
FC> library contracts derived. So I would not delete this functionality.

If we put this feature behind the famous "2.3" switch, could you accept
it then?

FC> Another argument against this is the argumentation we followed in
FC> 2.2, where we said, a facelet is just a facelet, without any need to
FC> differentiate between different purposes of a facelet.

FC> IMO 1359 brings no benefit to the spec. It just adds complexity to
FC> the whole resource handling, which is already complicated enough.

EB> Hi Frank, you're right in pointing out 1359-views adds little value.
EB> Just the same, for the two reasons cited in the following text, I can
EB> support it.

FC> Arjan, can you please explain a little bit more, what are the benefits?

>>>>>>> On Tue, 24 Feb 2015 12:03:45 +0100, arjan tijms <arjan.tijms_at_gmail.com> said:

AT> The first is that in the default configuration JSF now has a *source
AT> code exposure vulnerability*. []

>>>>> On Thu, 26 Feb 2015 12:29:39 +0100, arjan tijms <arjan.tijms_at_gmail.com> said:

AT> I personally think it's a vulnerability that should be taken
AT> seriously.

Yes, I find this a very compelling motivation.

>>>>> On Sun, 1 Mar 2015 18:11:30 +0100, Frank Caputo <frank_at_frankcaputo.de> said:

FC> I understand this. The first thing I usually do in a new project is
FC> removing all those X-Powered-By headers, which are also on by
FC> default.

While that and agressively adding <security-constraint> elements are
well known best practices, we have an opportunity here to be a little
more secure by default. I think it's worth taking that opportunity.

FC> This was also discussed in 2.2 and we decided to not care about
FC> this. Currently ViewResources in Mojarra dont expose the input
FC> stream (which id like to see). It is easy to hide facelets.

But it is even easier to not hide them, and that's the problem.

AT> The second problem is that in Facelets top level views, includes,
AT> templates, etc all have the same extension; .xhtml. The runtime cannot
AT> automatically make a distinction between what are top level views, and
AT> what are other Facelets artefacts and in fact what are just files with
AT> an .xhtml extension not being related to JSF at all. This again
AT> introduces some security concerns, like /resources/mycomposite.xhtml
AT> being requestable via a URL, even when it's not intended to be
AT> requested as such.

The absence of a specific filename for Facelet template clients vs.
Facelet views was intentional, to ease adoption. I stand 100% behind
this choice for simplicity.

AT> In general, having a clear idea of what are top-level views and what
AT> are not is a powerful tool that extension libraries can take advantage
AT> of for all kinds of things.

I'll note that the scope of 1359-views is for views in the current
meaning of what you put in the webapp root: templates or template
clients. If you want to propose another folder for templates, we must
discuss that separately on another thread.

AT> I do have to stress that the proposed /views folder is -an- enabler,
AT> but I'm of course open to explore other options to reach the same goal
AT> (prevent source code exposure and determine if a Facelet is a
AT> top-level view or not).

EB> One other aspect not mentioned is alignment with the MVC spec.

FC> This one is the only valid one for me. Unfortunately I was really
FC> busy the last months and didnt follow the MVC discussions very
FC> well. Ill take a deeper look into this.

Thanks for making your position more clear.

AT> For extensionless mapping it's kinda important to know which things
AT> are views, and which things are not views.

FC> I still think, scanning all facelets on startup and looking for an
FC> f:view tag is an option to find all views. I have a project with
FC> more than 1300 facelets doing this. It takes about a second.

Extensionless mapping is a different issue.

AT> It's not so nice to just randomly map the FacesServlet to every
AT> *.xhtml file the runtime encounters. Security aside, you just don't
AT> want to have http://myhost.net/includes/dataForm resolve to anything
AT> if /includes/dataform is not a top-level view.

FC> We can send a 403 if there is no f:view tag in the requested
FC> facelet.

That would be an even bigger backward compatibility headache than
saying, "all your views must be in WEB-INF/views". Using something in
the file as the signal of whether or not to send 403 is much harder than
using the location of the file itself.

FC> I have another point about extensionless mapping. If the main
FC> purpose is SEO, than this only helps for english web sites.

Ok, at this point I will create another top level thread to host the
"extensionless mapping" discussion.

Coming back to the core issue of 1359-views, here is the latest
proposal.

* Modify the spec for ResourceHandler.createViewResource() to replace the
  text "Considering the web app root." with the following:

  "Considering the value ResourceHandler.WEBAPP_VIEWS_PARAM_NAME
   property."

* Add a new constant on ResourceHandler

  WEBAPP_VIEWS_PARAM_NAME = "javax.faces.WEBAPP_VIEWS_DIRECTORY"

  If a context-param with the param name equal to the value of
  WEBAPP_RESOURCES_DIRECTORY_PARAM_NAME exists, the runtime must
  interpret its value as a path, relative to the web app root, where VDL
  views must be located. This param value must not start with a "/",
  though it may contain "/" characters. If no such <context-param>
  exists, or its value is invalid, the effective value depends on the
  version of the WEB-INF/faces-config.xml file. If the version is 2.3
  or greater, the effective version is "WEB-INF/views". Otherwise the
  effective value is the empty string "".

* Include a promiment note in the changelog section of the spec calling
  out this opt-in non-backward-compatible change and what to do to "get
  the old behavior back".

I'll note that this new constant has an inconsistent value with its
siblings WEBAPP_CONTRACTS_DIRECTORY_PARAM_NAME and
WEBAPP_RESOURCES_DIRECTORY_PARAM NAME. One could argue that we should
have defined those as WEB-INF/resources and WEB-INF/contracts
(respectively) in the first place. If you want to argue that, you may
create a thread with

Subject: WEBAPP_{CONTRACTS,RESOURCES}_DIRECTORY Discussion

Personally, I'm content to live with these as they are and advise people
to set them to be WEB-INF/contracts and WEB-INF/resources manually.

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
|  3 days til DevNexus 2015
| 13 days til JavaLand 2015
| 23 days til CONFESS 2015