Fixed NPE in case of passing null refs to "c:set" tag

[davidecavestro]

This should fix issues arising after an upgrade to JSP 2.1 (and above, I guess).
i.e. there's an issue filed at https://sourceforge.net/p/freemarker/bugs/204/
The problem lies on the fact that SetSupport relies on javax.el.VariableMapper.setVariable(String, ValueExpression) supporting null refs on the last parameter. So at org.apache.taglibs.standard.tag.common.core.SetSupport.doEndTag() it does things like

// Make sure to clear any previous mapping for this
// variable in the variable mapper.
if (scope ==  PageContext.PAGE_SCOPE) {
  VariableMapper vm =
    pageContext.getELContext().getVariableMapper();
  if (vm != null) {
    vm.setVariable(var, null);//PASSING NULL REF HERE
  }
}

while the original implementation doesn't provide null handling

[ddekany]

I can't merge without Contributor License Agreement (I will send it if you are interested), but no problem, I take it as bug report and fix it myself. As of bug #204, that's a different issue with c:foreEach.

[davidecavestro]

No problem, let's keep things simple and go for fixing it yourself.
As of bug #204 you are right: I originally had a similar problem with the c:set tag (let's call it 1st order problem) and worked around it temporarily setting the default JspFactory the following way:

JspFactory oldFactory = JspFactory.getDefaultFactory();
JspFactory.setDefaultFactory(new FreeMarkerJspFactory21Wrapper());
try {
  ...
} finally {
  JspFactory.setDefaultFactory(oldFactory);
}

where the wrapper simply delegates every call to an internal instance of FreeMarkerJspFactory21.
This solved the original problem, hence leading to the 2nd order problem (the one this pull request is about), common both to c:set and c:forEach tags.
In fact even LoopTagSupport.unExposeVariables passes a null reference to the variable mapper, so I'll expect a NPE even in that case:

    private void unExposeVariables() {
        // "nested" variables are now simply removed
    if (itemId != null) {
            pageContext.removeAttribute(itemId, PageContext.PAGE_SCOPE);
            VariableMapper vm = pageContext.getELContext().getVariableMapper();//1ST ORDER PROBLEM
            if (vm != null)
                vm.setVariable(itemId, null);//2ND ORDER PROBLEM
        }

So - to make a long story short - I'd like to hear from you if there's a better way to solve the problem originally signaled at bug #204 (other than the workaround I put in place).

[ddekany]

I don't even know it there's a real solution for all this... and I have to add that I'm not the author of the JSP support (or the original author of anything), nor I do know much about JSP internals. The problem is, as fas as I see, that if both FTL-s and real JSP-s are working in the same application, then one of them will end up using the JspApplicationContext implementation of the other, which sounds scary. In this case you force FreeMarker's implementation, but won't some real JSP-s fail now? And why not? Any insight is welcome...

Despite of this, a lot of serious users rely on the JSP support (Liferay and such). I guess the secret is not using JSTL tags in FTL, and in fact you aren't supposed to use them in FTL (kills the point of using FTL pretty much). (Of course, other custom taglibs could depend on the same EL-related features too, but apparently they don't do it, or not often.) Why do you need to use them? Could that need be solved otherwise maybe?

[davidecavestro]

In this particular case FreeMarker templates ran within some some jBPM processes to provide dynamically generated HTML on the top of contents from pre-existing JSPs.
Since those JSPs make heavy use of standard and custom JSP tags, freemarker templates started as a translation of JSP syntax to freemarker one, hence ended up using them.
That was reasonable because in this particular scenario Freemarker simply filled up the limitation of JSPs being not able to load templates from resources accessed through arbitrarily complex classloading machinery.

[ddekany]

Maybe replacing the JSTL tags with FreeMarker equivalents will be the cheapest and also the most stable solution... Of course I don't know how much template this is, and if uses something that has no FTL equivalent.