You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jspwiki.apache.org by Andrew Jaquith <an...@mac.com> on 2008/02/11 17:01:04 UTC

Putting the 'JSP' in JSPWiki 3

All --

I've been working hard on the second part of the core JSPWiki/Stripes  
integration for version 3. The core integration has been working  
nicely for a while. Unit tests are running nearly clean (4 failures, 1  
error).

Two weeks ago, I shifted my efforts to the JSP/user tier. In the  
absence of a 3.0 branch on the Apache SVN server (a continuing  
annoyance, might I add, but it looks like it will be addressed soon),  
I thought I'd share a few thoughts on a few key JSP-tier changes I'm  
contemplating. They are actually more than "contemplations," in the  
sense that I have them running now. But nothing is etched in stone  
yet. I wanted to mention three of the most important changes so that I  
could get some comments from all of you.

First, I'm doing quite a bit of experimentation with JSP expression  
language. The idea is to use EL variables quite a bit more. For  
example, it's very convenient to be able to simply express something  
like ${wikiSession.userPrincipal} and have it print out the value --  
without needing scriptlet code, or even JSP tags. For the moment, I  
have rigged it so that we inject some standard variables into every  
JSP that invokes a WikiContext (WikiActionBean, really).  Overall, the  
goal is to provide some nice "scaffolding" variables so that JSP  
template authors (including the JSP dev team, of course) doesn't need  
to write much, if any, scriptlet code.

In my local builds, here are the names of the attributes that JSPWiki  
injects into the PageContext request scope automatically:

- "wikiEngine"
- "wikiSession"
- "wikiActionBean" (aka the wiki context)
-" wikiPage" (if action bean is a wiki context, it's set to its page;  
otherwise the front page)

All of these are available as EL variables (e.g., ${wikiEngine}, $ 
{wikiPage}).

You might be asking, how do we "rig" it so that these variables are  
injected into request scope? Excellent question.

That brings me to the second part of the changes -- the introduction  
of the "WikiInterceptor," which intercepts the Stripes ActionBean  
resolution process and does a bunch of magic. As you may know from my  
previous writings (and from the Stripes website), when Stripes sees a  
&lt;stripes:useActionBean&gt; tag in a JSP -- or filters a URL that  
ends in .action -- it tries to find and inject the correct ActionBean  
into the request. In JSPWiki 3, WikiContexts are a type of ActionBean.  
In our case, I have configured Stripes so that when it finds our  
WikiContext/WikiActionBean, it causes WikiInterceptor to fire and to  
our extra magic.

Here's what it does. WikiInterceptor's intercept() methods fires after  
the correct ActionBean is resolved, but before the user actually sees  
the JSP. The interceptor:
- Examines whether the user's desired event handler ("view", "save"  
etc.) requires a particular Permission to execute
- If the event handler requires a Permission, it checks to see whether  
the user possess it
- If not, the user is redirected to the login page
- If yes, it injects variables for the WikiEngine, WikiSession,  
resolved WikiActionBean, and WikiPage (if the action bean is a wiki  
context)

What this means, practically speaking, is that top-level JSPs are  
about to get a LOT slimmer. They won't need to do anything special to  
look up common objects like the WikiEngine, and won't have the need to  
do any access-checking. The various *Content JSPs will likely get  
slimmer too.

Speaking of JSPs, let me speak briefly about the third big change.

Bug [JSPWiki-81] requires us to move JSPs that aren't supposed to be  
instantiated directly into WEB-INF. I am doing this now. In addition  
to having security benefits, this also simplifies the JSP template  
directory a bit. It  means also that PageActions, commonheader, etc.  
all move inside WEB-INF.

The path I'm using for the template-specific JSPs is WEB-INF/jsp/ 
templates/default. So far, I've been able to move the necessary JSPs  
into WEB-INF without requiring many changes. The principal changes  
I've needed to make to the template-specific JSPs included:
- Replacing scriptlet code with ${} variables, where appropriate
- Removal of &lt;fmt:setBundle basename="templates.default"&gt; and  
LocaleSupport.getLocalizedMessage() -- because we set the default in  
web.xml, and because Stripes sets the request locale automatically
- Replacement of WikiContext c =  
WikiContext.findContext( pageContext ) with the newer technique  
WikiContext c = (WikiContext)WikiInterceptor.findActionBean();

Top-level JSPs also change quite a bit. Here, for example, is the  
*complete* contents of Wiki.jsp (angle brackets omitted):

%@ page contentType="text/html;charset=UTF-8" language="java" %
%@ taglib uri="http://stripes.sourceforge.net/stripes.tld"  
prefix="stripes" %
%@ taglib uri="/WEB-INF/jspwiki.tld" prefix="wiki" %

stripes:useActionBean  
beanclass="com.ecyrd.jspwiki.action.ViewActionBean" /
stripes:layout-render name="/WEB-INF/jsp/templates/default/ 
ViewTemplate.jsp"
stripes:layout-component name="contents"
      wiki:Include page="/WEB-INF/jsp/templates/default/ 
PageContent.jsp" /
/stripes:layout-component
/stripes:layout-render

At the moment, the paths for the template JSPs are hard-coded. This  
will change, though. I will massage the wiki:include tag a bit until  
it behaves properly. I think it might also be useful to set up an EL  
variable that makes getting template resources easy. For example, a  
map-style variable expression would be elegant and concise:

${templateResources["PageContent.jsp"]}

The expression would return the correct path, namely /WEB-INF/jsp/ 
templates/default/PageContent.jsp.

Overall, my goal is to make the existing tags work just the way they  
do now, taking into account the new location for JSPs. But I think it  
might be nice, also, to use EL variables as aggressively as we can,  
and deprecate certain tags over time.

What do you think? I welcome your comments and feedback, as always.

Andrew

Re: Putting the 'JSP' in JSPWiki 3

Posted by Janne Jalkanen <Ja...@ecyrd.com>.
> Makes sense to do it in 2.8, assuming we have a gentlemen's  
> agreement that injection point would move from wherever we put it  
> in 2.8 (WikiServletFilter, probably) to WikiInterceptor in 3.0. The  
> end-user (and indeed, template developers) won't care how/where we  
> do this, of course.

Obviously.

> As Ounce Labs noted in [JSPWiki-81], it is considered best practice  
> to put secondary JSPs inside WEB-INF. Notwithstanding the minor  
> inconvenience this might cause a few dozen template developers,  
> it's just one less thing for the core team to worry about, and it  
> will reduce the potential for confusion for thousands of end-users.

I am not worries about template developers - I am worried about  
thousands of template _deployers_.  The security benefits of this are  
marginal, yet they will create a Windows-like situation where some  
files go in one directory and some others go in some other  
directory.  I much prefer the ability to deploy templates as a single  
directory - far less confusing to the users.  (Not to developers, but  
to actual users.)  For example, when removing a template you might  
accidentally leave some scripts or some JSP behind.

> Everything's a trade-off, right? Best-practice v. developer  
> convenience, as usual. :)  On balance, I lean towards best practice  
> -- not least because we *did* say we'd do this in 3.0. If not, we  
> should mark this issue as WONTFIX.

I think that was too snappy a judgement - at least from my side.

> Good point. I think it would be better for TemplateResources (which  
> would probably be a refactored TemplateManager) to *implement* the  
> Map interface, rather than for itself to be a HashMap subclass, for  
> example. That would give developers a nice way to extend things,  
> but it would still preserve the simplicity of being able to use it  
> as an EL variable expression.

Mmm...  Isn't that sort of confusing the boundaries?  I don't  
particularly care for classes which have multiple, non-obvious roles  
in the system.  If we want to offer a Map for this kind of stuff, it  
should be a separate class with a clearly defined role.

The function and role of the class should be obvious from its name  
and methods.  If you have to rely on documentation, I think the class  
is already too complex ;-)  (And yeah, we have some classes which are  
that way.)

/Janne

Re: Putting the 'JSP' in JSPWiki 3

Posted by Andrew Jaquith <an...@mac.com>.
>> - "wikiEngine"
>> - "wikiSession"
>> - "wikiActionBean" (aka the wiki context)
>> -" wikiPage" (if action bean is a wiki context, it's set to its  
>> page; otherwise the front page)
>
> Good, I was planning to do this too.
>
> If possible, I'd like to see this in 2.8 already to allow template  
> writers to take advantage of it.

Makes sense to do it in 2.8, assuming we have a gentlemen's agreement  
that injection point would move from wherever we put it in 2.8  
(WikiServletFilter, probably) to WikiInterceptor in 3.0. The end-user  
(and indeed, template developers) won't care how/where we do this, of  
course.

>> Bug [JSPWiki-81] requires us to move JSPs that aren't supposed to  
>> be instantiated directly into WEB-INF. I am doing this now. In  
>> addition to having security benefits, this also simplifies the JSP  
>> template directory a bit. It  means also that PageActions,  
>> commonheader, etc. all move inside WEB-INF.
>
> There's a problem - the static content cannot be served from the WEB- 
> INF, and therefore templates will need to be split in two: static  
> content and dynamic content.
>
> I'd like to discuss this a bit further before we make this change.   
> Is there a real security benefit (with WikiInterceptor and all), or  
> is this just going to make life more complicated for template  
> developers and deployers?  I like the idea of being able to deploy a  
> template as a single directory so that it does not leave files lying  
> around everywhere.

That's true, but the rule would be pretty simple: template JSPs go  
behind WEB-INF; everything else goes in front of it.

 From the security standpoint, it won't make much of a difference: if  
you instantiate a template JSP directly today, all that happens is you  
get a humongous error message and a stack trace, because the template  
doesn't can't grab a WikiContext. I'm not aware of many security  
vulnerabilities that could arise from this: without a WikiContext, you  
can't affect the state of JSPWiki. It's possible that getting the  
error page might enable an attacker to inject a cross-site script into  
the error page. But, I'm reaching here.

If you hide the template JSPs inside WEB-INF, you at least have the  
ability to prevent the error messages from appearing, which is good  
from the user standpoint -- less opportunities to be confused.

This question won't go away in 3.0, even with WikiInterceptor. If we  
keep template JSPs outside WEB-INF, direct-addressing them will still  
result in error messages.

As Ounce Labs noted in [JSPWiki-81], it is considered best practice to  
put secondary JSPs inside WEB-INF. Notwithstanding the minor  
inconvenience this might cause a few dozen template developers, it's  
just one less thing for the core team to worry about, and it will  
reduce the potential for confusion for thousands of end-users.

Everything's a trade-off, right? Best-practice v. developer  
convenience, as usual. :)  On balance, I lean towards best practice --  
not least because we *did* say we'd do this in 3.0. If not, we should  
mark this issue as WONTFIX.

>> ${templateResources["PageContent.jsp"]}
>
> I'd rather make it an EL function - but there's a point in making it  
> a tag as well.  Tags allow you to have far deeper control during  
> execution, so it might be nicer for a 3rd party to provide their own  
> tag extending from our tag instead of forcing them to muck around in  
> what is essentially a private array.  An EL function might be a nice  
> compromise.

Good point. I think it would be better for TemplateResources (which  
would probably be a refactored TemplateManager) to *implement* the Map  
interface, rather than for itself to be a HashMap subclass, for  
example. That would give developers a nice way to extend things, but  
it would still preserve the simplicity of being able to use it as an  
EL variable expression.

Andrew

Re: Putting the 'JSP' in JSPWiki 3

Posted by Janne Jalkanen <Ja...@ecyrd.com>.
> In my local builds, here are the names of the attributes that  
> JSPWiki injects into the PageContext request scope automatically:
>
> - "wikiEngine"
> - "wikiSession"
> - "wikiActionBean" (aka the wiki context)
> -" wikiPage" (if action bean is a wiki context, it's set to its  
> page; otherwise the front page)

Good, I was planning to do this too.

If possible, I'd like to see this in 2.8 already to allow template  
writers to take advantage of it.

> Bug [JSPWiki-81] requires us to move JSPs that aren't supposed to  
> be instantiated directly into WEB-INF. I am doing this now. In  
> addition to having security benefits, this also simplifies the JSP  
> template directory a bit. It  means also that PageActions,  
> commonheader, etc. all move inside WEB-INF.

There's a problem - the static content cannot be served from the WEB- 
INF, and therefore templates will need to be split in two: static  
content and dynamic content.

I'd like to discuss this a bit further before we make this change.   
Is there a real security benefit (with WikiInterceptor and all), or  
is this just going to make life more complicated for template  
developers and deployers?  I like the idea of being able to deploy a  
template as a single directory so that it does not leave files lying  
around everywhere.

> At the moment, the paths for the template JSPs are hard-coded. This  
> will change, though. I will massage the wiki:include tag a bit  
> until it behaves properly. I think it might also be useful to set  
> up an EL variable that makes getting template resources easy. For  
> example, a map-style variable expression would be elegant and concise:
>
> ${templateResources["PageContent.jsp"]}

I'd rather make it an EL function - but there's a point in making it  
a tag as well.  Tags allow you to have far deeper control during  
execution, so it might be nicer for a 3rd party to provide their own  
tag extending from our tag instead of forcing them to muck around in  
what is essentially a private array.  An EL function might be a nice  
compromise.

/Janne