jakartaee / expression-language

Jakarta Expression Language
https://eclipse.org/ee4j/el
Other
68 stars 49 forks source link

Implement an EL 2.2 backward compatible switch in RI #38

Closed glassfishrobot closed 10 years ago

glassfishrobot commented 10 years ago

EL 3.0 introduces new coercion rules from nulls to non-primitive types (such as Boolean). This is not compatible with EL 2.2. The RI needs to implement a switch to make EL 3.0 behave like EL 2.2 for such coercions.

This switch can also be used for any future backward incompatible features.

glassfishrobot commented 6 years ago
glassfishrobot commented 10 years ago

@glassfishrobot Commented Reported by kchung

glassfishrobot commented 10 years ago

@glassfishrobot Commented kchung said: The following is the proposed mechanism that allows an EL client (such as the JSP container) to pass an option to turn on the EL 2.2 backward compatibility for EL 3.0 and above.

  1. The option is passed as a property in a java.util.Properties, to the method``` ExpressionFactory#newInstance(Properties properties)

  2. The key for the property is "javax.el.bc2.2", and the value for it is "true".

  3. In addition, the instance of ExpressionFactory used in the evaluation of an EL expressions is passed to ELContext. This is achieved by invoking ELContext#putContext, using ExpressionFactory.class as the key:``` myELContext.putContext(javax.el.ExpressionFactory.class, myExpressionFactory);

glassfishrobot commented 10 years ago

@glassfishrobot Commented kchung said: Dongbin Nie proposed the following mechanism that allows a WLS user to specify such an option in a WLS application. Other web container can implement something similar. The actual work is not part of uel project, but we include comments here for completeness.

Dongbin:

We plan to add a option named in weblogic.xml, e.g.

<weblogic-web-app>
  <jsp-descriptor>
    <el-2.2-backward-compatible>true</el-2.2-backward-compatible>
  </jsp-descriptor>
</weblogic-web-app>

In Weblogic JSP container's JspApplicationContextImpl, the ExpressionFactory will be created with ExpressionFactory.newInstance(Properties properties) based on the option in weblogic.xml, and the JspApplictionContext.getExpressionFactory() will return the created ExpressionFactory for the application.

glassfishrobot commented 10 years ago

@glassfishrobot Commented @edburns said:

On Mon, 03 Mar 2014 09:52:12 -0800, Kin-man Chung said:

KC> On 2/28/14, 1:52 PM, Edward Burns wrote:

On Fri, 28 Feb 2014 09:55:20 -0800, Kin-man Chung said:

KC> But what are you doing with JSP stuff in JSF anyway? Do you actually KC> create a JspApplicationContext in JSF? My guess is that are from JSF KC> 1.2. Do you call ExpressionFactory#newInstance directly in 2.2? Can you KC> make the change to call ExpressionFactory#newInstance in JSF 1.2 as well?

EB> Even in the very latest 2.2 of JSF, we still use that technique to get EB> the ExpressionFactory. It is specified in section "7.1.10 Acquiring EB> ExpressionFactory Instance". Because the very lastest JSF 2.2 is EB> completely backward compatible with JSF 1.0, we need to do this, even if EB> the app doesn't happen to use JSP.

KC> Understood.

On Thu, 27 Feb 2014 16:09:53 -0800, Kin-man Chung said: KC> 2. Use ExpressionFactory#newInstance(Properties p) to pass the BC option KC> to EL.

KC> Pros: Specific to each application. Done at deployment. KC> Cons: 2a. Affects all EL clients (JSP, JSF, CDI?) KC> 2b. Needs a new configuration for user to specify the BC option.

EB> At what point in the initialization of the system must your option 2 be EB> invoked? Must it be before any other ExpressionFactory instances are EB> created?

KC> If you cache ExpressionFactory, then yes. But also allow each KC> application to have its own ExpressionFactory, and the option only KC> applies to its its instance of ExpressionFactory.

There are circumstances when we do cache the ExpressionFactory. Therefore, I assert we must ensure the configuration setting must be conveyed on the first invocation that causes an ExpressionFactory to be created.

On Fri, 28 Feb 2014 09:55:20 -0800, Kin-man Chung said:

KC> Currently both of JspFactory.getDefaultFactory() and KC> jspAppContext.getExpressionFactory() use the zero argument version of KC> ExpressionFactory#newInstance, so they won't work unless I inject the bc KC> property at these methods.

Kin-Man, can you please do that? That would be the easiest and most resilient for us. We do have a Mojarra specific context param that enables you to pass in your own ExpressionFactory instance but when the user does that, of course they are taking responsibility for its creation themselves.

On Tue, 04 Mar 2014 14:11:16 -0800, Kin-man Chung said:

KC> Dongbin, KC> Attached please find the jar for the RI for your testing. I'll KC> eventually push it to Maven Central, but I am looking at other bugs in KC> EL and might include the fixes also.

KC> Note the jar now includes both the API as well as the KC> implementation.

Kin-man, does the impl you attached on 2014-03-04 inject the backward compatibility setting when JspFactory.getDefaultFactory() and jspAppContext.getExpressionFactory() are used?

DN> After a rough code search in Mojarra codebase, it seems that Mojarra DN> does not only retrieve the ExpressionFactory from DN> JspApplicationContext, but can also create ExpressionFactory instance DN> through ExpressionFactory.newInstance() method if InitParameter DN> "com.sun.faces.expressionFactory" is configured in web.xml by user, DN> in this case maybe it's better to give the user the ability to also DN> configure the ExpressionFactory's properties in web.xml if required.

On Mon, 03 Mar 2014 10:14:55 -0800, Kin-man Chung said:

KC> Please check this. Can you not call KC> JspApplictionContext.getExpressionFactory() instead? That way, you KC> don't need to worry about the BC option.

I have checked this. Here is the code path for initializing the ExpressionFactory in JSF.

During our ConfigureListener.contextInitialized(), the first ExpressionFactory related action is this:

registerELResolverAndListenerWithJsp(context, false);

There are two cases to consider.

CASE_1_NOT_SET. The com.sun.faces.expressionFactory is NOT SET.

In this case, the first JSP call is this:

if (JspFactory.getDefaultFactory() == null) {
            return false;
        }
        try {
            JspFactory.class.getMethod("getJspApplicationContext",
    ServletContext.class);
        } catch (Exception e) {
            return false;
        }
        try {
            JspFactory.getDefaultFactory().getJspApplicationContext(context);
        } catch (Throwable e) {
            return false;
        }
        return true;

Then we call

// JSP 2.1 specific check
            if (JspFactory.getDefaultFactory().getJspApplicationContext(context) == null) {
return;
            }

With JSP 2.1, this returns non-null so we continue to:

// get JspApplicationContext.
            JspApplicationContext jspAppContext = JspFactory.getDefaultFactory()    .getJspApplicationContext(context);

            // cache the ExpressionFactory instance in ApplicationAssociate
            if (associate != null) {
associate.setExpressionFactory(jspAppContext.getExpressionFactory());
            }

As you can see, this does cache the ExpressionFactory.

CASE_1_IS_SET. The com.sun.faces.expressionFactory IS SET.

Here we simply use the no-arg ctor of the class provided by the context-param and cache it in installExpressionFactory().

Here is the relevant code.

public void registerELResolverAndListenerWithJsp(ServletContext context, boolean reloaded) {

        if (webConfig.isSet(WebContextInitParameter.ExpressionFactory)
|| !isJspTwoOne(context)) {

            // first try to load a factory defined in web.xml
            if (!installExpressionFactory(context,
    webConfig.getOptionValue(
            WebContextInitParameter.ExpressionFactory))) {

throw new ConfigurationException(
        MessageUtils.getExceptionMessageString(
MessageUtils.INCORRECT_JSP_VERSION_ID,
WebContextInitParameter.ExpressionFactory.getDefaultValue(),
WebContextInitParameter.ExpressionFactory.getQualifiedName()));

            }

        } else {

            // JSP 2.1 specific check
            if (JspFactory.getDefaultFactory().getJspApplicationContext(context) == null) {
return;
            }

            // register an empty resolver for now. It will be populated after the
            // first request is serviced.
            FacesCompositeELResolver compositeELResolverForJsp =
    new ChainTypeCompositeELResolver(FacesCompositeELResolver.ELResolverChainType.JSP);
            ApplicationAssociate associate =
    ApplicationAssociate.getInstance(context);
            if (associate != null) {
associate.setFacesELResolverForJsp(compositeELResolverForJsp);
            }

            // get JspApplicationContext.
            JspApplicationContext jspAppContext = JspFactory.getDefaultFactory()    .getJspApplicationContext(context);

            // cache the ExpressionFactory instance in ApplicationAssociate
            if (associate != null) {
associate.setExpressionFactory(jspAppContext.getExpressionFactory());
            }

            // register compositeELResolver with JSP
            try {
jspAppContext.addELResolver(compositeELResolverForJsp);
            }
            catch (IllegalStateException e) {
ApplicationFactory factory = (ApplicationFactory)
        FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY);
Application app = factory.getApplication();
if (app.getProjectStage() != ProjectStage.UnitTest && !reloaded) {
    throw e;
}
            }

            // register JSF ELContextListenerImpl with Jsp
            ELContextListenerImpl elContextListener = new ELContextListenerImpl();
            jspAppContext.addELContextListener(elContextListener);
        }
    }

    private static boolean isJspTwoOne(ServletContext context) {

        // The following try/catch is a hack to work around
        // a bug in Tomcat 6 where JspFactory.getDefaultFactory() will
        // return null unless JspRuntimeContext has been loaded.
        try {
            Class.forName("org.apache.jasper.compiler.JspRuntimeContext");
        } catch (ClassNotFoundException ignored) {
            if (LOGGER.isLoggable(Level.FINEST)) {
LOGGER.log(Level.FINEST, "Dected JSP 2.1", ignored);
            }
        }

        if (JspFactory.getDefaultFactory() == null) {
            return false;
        }
        try {
            JspFactory.class.getMethod("getJspApplicationContext",
    ServletContext.class);
        } catch (Exception e) {
            return false;
        }
        try {
            JspFactory.getDefaultFactory().getJspApplicationContext(context);
        } catch (Throwable e) {
            return false;
        }
        return true;

    }

    private boolean installExpressionFactory(ServletContext sc,
             String elFactoryType) {

        if (elFactoryType == null) {
            return false;
        }
        try {
            ExpressionFactory factory = (ExpressionFactory)
    Util.loadClass(elFactoryType, this).newInstance();
            ApplicationAssociate associate =
    ApplicationAssociate.getInstance(sc);
            if (associate != null) {
associate.setExpressionFactory(factory);
            }
            return true;
        } catch (Exception e) {
            if (LOGGER.isLoggable(Level.SEVERE)) {
LOGGER.severe(MessageFormat.format("Unable to instantiate ExpressionFactory ''{0}''",
        elFactoryType));
            }
            return false;
        }

    }
glassfishrobot commented 10 years ago

@glassfishrobot Commented kchung said: Ed,

  1. The work that ensures that JspApplicationContext#getExpressionFactory will pass the backward compatibility option to EL will be done in the JSP container. Dongbin is doing the work for WLS. I probably will modify the JSP container in Glassfish someday, but this is of much lower priority.
  2. The codes you've shown looks OK, as long as you get the ExpressionFactory by calling JspApplication#getExpressionFactory. It will be OK to cache the ExpressionFactory gotten this way.
  3. If you have any part of the code that calls ExpressionFactory#newInstance directly, you'll be responsible for getting the BC option from weblogic.xml and passing it to EL.
  4. The codes you've shown seems to allow users to specify an ExpressionFactory in web.xml. This is probably OK, since EL you allowed is probably 2.2 or earlier.
glassfishrobot commented 10 years ago

@glassfishrobot Commented @edburns said: Thanks. Your responses make this perfectly clear.

glassfishrobot commented 10 years ago

@glassfishrobot Commented kchung said: It turns out that it is really difficult to get the instance of ExpressionFactory that was instantiated with the BC option during expression evaluation. So I have edited the original proposal to included item 3.

JSF just needs to call JspApplication#getExpression to get a ExpressionFactory instance, and pass that to an ELContext per item 3.

Thanks Dongbin for suggesting item 3.

glassfishrobot commented 10 years ago

@glassfishrobot Commented kchung said: Fixed

glassfishrobot commented 10 years ago

@glassfishrobot Commented Issue-Links: is related to JAVASERVERFACES-3454

glassfishrobot commented 10 years ago

@glassfishrobot Commented Was assigned to kchung

glassfishrobot commented 7 years ago

@glassfishrobot Commented This issue was imported from java.net JIRA UEL-38

glassfishrobot commented 10 years ago

@glassfishrobot Commented Marked as fixed on Monday, June 2nd 2014, 11:05:36 am