eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
160 stars 109 forks source link

NPE on f:param with null value #3158

Closed ren-zhijun-oracle closed 10 years ago

ren-zhijun-oracle commented 10 years ago

This snipped throws a NPE:

<h:link value="A" outcome="a_valid_outcome">
    <f:param name="x" value="#{null}"/>
</h:link>

Affected Versions

[2.2.5]

ren-zhijun-oracle commented 6 years ago
ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented Reported by dbenegiamo

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: While the attached change bundle fixes the issue there is the matter of is this what the specification intended.

The Javadoc of f:param states it needs to evaluate to java.lang.Object. Does a null qualify for that or not?

If #

{null}

is not a valid value of f:param then we won't apply this, if it is we will.

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented dbenegiamo said: Hi Mandred,

for sure it's a regression issue: Mojarra 1.x accepted "null" values (in fact a discovered it while porting a JEE6 application to JEE7).

BTW, JSR 341 (Expression Language 3.0) specifies that "null" values can be converted to any other type (sections 1.23.1, 1.23.7).

And thanks for the quick fix!

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: Unfortunately you have hit a corner case with respect to h:link. As h:link together with f:param is supposed to generate a GET request the runtime correctly tries to call .toString() on the null value. So this is unfortunately NOT supported. Closing this as "Invalid".

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented dbenegiamo said: I don't understand "the runtime correctly tries to call .toString()", why "correctly"?

Section 1.23.2 of JSR 341 states that ' If A is null: return “” '; so a null value must translate to an empty string, perfectly fine with a GET request (and in fact is how a changed my current application).

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: What are you running JSF 2.2 on?

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented dbenegiamo said: Glassfish 4.0, but with Mojarra 2.2.5 instead of the shipped 2.2.0.

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: OK looking closer you are making the assumption that the EL conversion would use section 1.23.2, which is not the case. The EL conversion would use the following rule:

1.23.7 Coerce A to Any Other Type T ■ If A is null, return null

Because according to the VDL of f:param where it states in the expected type in the Type column: javax.el.ValueExpression (must evaluate to java.lang.Object)

So while it is unfortunate the runtime is doing what the specification requires.

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @edburns said: I'm not clear if you want my review on the attached changebundle or not. Please clarify.

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: No while the change bundle would 'fix' it, it would actually violate the spec so I will go ahead and remove it. Thanks!

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented dbenegiamo said: Thanks Manfred. I still don't find why it should throws an exception if the java.lang.Object resolves to null. Also the VDL of h:inputText reports for the "value" attribute the "javax.el.ValueExpression (must evaluate to java.lang.Object)" notation, but I never see exceptions on null values with it. From your past comment seems that the point is the usage in a GET request, does the specifications states that a null parameter can't be properly encoded?

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: Unfortunately a f:param is a different thing than a h:inputText. A h:inputText goes through a different way of conversion as the submitted value is a string whereas the value of f:param does not have to be a string. And that is exactly what you are observing.

Have you tried using a h:commandButton or h:commandLink instead of h:link?

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented twinj said: The f:param value must be evaluate to an object: agreed.

However a Param in a PararmeterList object must evaluate to a String.

/**
     * @param command the command which may have parameters
     *
     * @return an array of parameters
     */
    protected Param[] getParamList(UIComponent command) {

        if (command.getChildCount() > 0) {
            ArrayList<Param> parameterList = new ArrayList<Param>();

            for (UIComponent kid : command.getChildren()) {
if (kid instanceof UIParameter) {
    UIParameter uiParam = (UIParameter) kid;
    if (!uiParam.isDisable()) {
        Object value = uiParam.getValue();
        Param param = new Param(uiParam.getName(),
(value == null ? NULL :
 value.toString()));
        parameterList.add(param);
    }
}
            }
            return parameterList.toArray(new Param[parameterList.size()]);
        } else {
            return EMPTY_PARAMS;
        }

    }

Therefore this code violates the String conversion rule.

Specifically here: value == null ? NULL : value.toString())

ren-zhijun-oracle commented 7 years ago

@javaserverfaces Commented starrunner said: @mriem: If null is not supported in UrlBuilder, what is the following line good for?

**UrlBuilder.java:327**  values.removeAll(NULL_LIST);

If null values really violate the spec, wouldn't it be better to remove this line as well and maybe throw a more descriptive message instead of the unconditioned NPE.

But please read the above comment and the one in #3359 first. Many commentators seem to think that the specification tends to "null is allowed as parameter value" (as I do, too) - and if that is not made clear enough in the spec, one should better straighten that one instead of sticking to an counterproductive implementation.

ren-zhijun-oracle commented 7 years ago

@javaserverfaces Commented starrunner said: Workaround for those, who rely on the old behavior: Write a ViewHandlerWrapper and implement getBookmarkableURL and getRedirectURL to filter null from the parameters.

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented Issue-Links: is duplicated by JAVASERVERFACES-3190 JAVASERVERFACES-3355

ren-zhijun-oracle commented 7 years ago

@javaserverfaces Commented This issue was imported from java.net JIRA JAVASERVERFACES-3154

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented Marked as invalid on Tuesday, January 28th 2014, 6:25:19 am