search

eclipse-ee4j / mojarra

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

f:selectItem String null value #5190

Closed pizzi80 closed 1 year ago

pizzi80 commented 1 year ago

Describe the bug

It is not possible to send the null value with f:selectItem when the backing bean property type is String

To Reproduce

Using the example at page 123 of "The definitive guide to Faces in Jakarta EE 10"

<h:form id="select">
    <h:selectOneMenu id="value" value="#{testViewAction.value}" >
        <f:selectItem itemValue="#{null}" itemLabel="-- select one --" />
        <f:selectItem itemValue="one" itemLabel="First item" />
        <f:selectItem itemValue="two" itemLabel="Second item" />
        <f:selectItem itemValue="three" itemLabel="Third item" />
    </h:selectOneMenu>

    <h:commandButton id="submit" value="Submit" action="#{testViewAction.submit}" />
</h:form>
@Named
@RequestScoped 
public class TestViewAction { 

    private static final Logger log = Logger.getLogger(MethodHandles.lookup().getClass().getName());

    private String value;
    public String getValue() {return value;}
    public void setValue(String value) {this.value = value;}

    public void submit() {
       log.info("["+value+"] --> value is null? " + (value==null) + ", value is empty? " + ("".equals(value)) + ", value is whitespace? " + (" ".equals(value) ) );
    }
}

Expected behavior

When you submit the form choosing the first element of the select the backing bean value should be null and not an empty String

Desktop (please complete the following information):

Stack

Faces 4.0 Tomcat 10.1.4 Jdk: 17.0.5

I also tried to replace the EL implementation with the Eclispe's one but the result is the same (empty String instead of null value) https://github.com/eclipse-ee4j/expressly

Additional context

There is a long story behind this (10+ years) on StackOverflow and on @BalusC blog

I tried all the solutions provided, and the problem is still on EL because when you submit the null value:

1) the StringConverter is totaly bypassed 2) the Faces init param jakarta.faces.INTERPRET_EMPTY_STRING_SUBMITTED_VALUES_AS_NULL is totally useless

I don't know which is the best place to solve this, I've implemented a naive solution using a custom EL resolver like the one proposed by @BalusC but with a special String value which I use as null, eg. 'faces.string.null'

public class EmptyToNullStringELResolver extends ELResolver {

    public static final String NULL_VALUE = "faces.string.null";

    @Override
    public Class<?> getCommonPropertyType(ELContext context, Object base) {
        return String.class;
    }

    @Override
    public <T> T convertToType(ELContext context, Object value, Class<T> targetType) {
        if ( NULL_VALUE.equals(value) && targetType == String.class ) {
            context.setPropertyResolved(true);
            return (T) null;
        }
        try {
            return (T) value;
        } catch (ClassCastException e) {
            return null;
        }
    }

    [ .... ]
}

Another solution could be to improve/fix/clarify one time and for all the f:selectItem for example providing an attribute like 'nullValue' boolean indicating that Faces should render a special String value that will be converted as null ... ?

BalusC commented 1 year ago

This is Tomcat specific but it works for me with only jakarta.faces.INTERPRET_EMPTY_STRING_SUBMITTED_VALUES_AS_NULL set to true, without the EL resolver and String converter.

Try reproducing the problem in e.g. WildFly or GlassFish.

pizzi80 commented 1 year ago

basically on Tomcat the jakarta.faces.INTERPRET_EMPTY_STRING_SUBMITTED_VALUES_AS_NULL is ignored

After many headaches I found that the following ELResolver works as expected

public class EmptyToNullStringELResolver extends ELResolver {

    @Override
    public Class<?> getCommonPropertyType(ELContext context, Object base) {
        return String.class;
    }

    @Override
    public <T> T convertToType(ELContext context, Object value, Class<T> targetType) {
        if (    context instanceof org.apache.el.lang.EvaluationContext &&
                String.class == targetType &&
                value == null ) {   
            context.setPropertyResolved(true);
            return (T) null;
        }
        try {
            return (T) value;
        } catch (ClassCastException e) {
            return null;
        }
    }

    [ .... ]
}
pizzi80 commented 1 year ago

This is Tomcat specific but it works for me with only jakarta.faces.INTERPRET_EMPTY_STRING_SUBMITTED_VALUES_AS_NULL set to true, without the EL resolver and String converter.

Try reproducing the problem in e.g. WildFly or GlassFish.

ok

pizzi80 commented 1 year ago

on Wildfly 27.Final and GlassFish 7 it works as expected

BalusC commented 1 year ago

Probably there's more into your project which caused this to fail on your Tomcat deployment? E.g. wrong EL-related dependencies or configuration.

pizzi80 commented 1 year ago

mmm It's a minimal "playground" project where I test small things, basically it's a "JakartaEE10-like" Faces + Rest project with Tomcat 10.1.4 and Java 17.0.5 which are the latest stable versions available

this is Faces config inside web.xml:

<!-- JSF -->
<servlet>
    <servlet-name>FacesServlet</servlet-name>
    <servlet-class>jakarta.faces.webapp.FacesServlet</servlet-class>
    <load-on-startup>1</load-on-startup>
</servlet>
<servlet-mapping>
    <servlet-name>FacesServlet</servlet-name>
    <url-pattern>*.xhtml</url-pattern>
</servlet-mapping>

<!-- JSF Configuration -->
<context-param>
    <param-name>jakarta.faces.FACELETS_BUFFER_SIZE</param-name>
    <param-value>65536</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.STATE_SAVING_METHOD</param-name>
    <param-value>server</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.PROJECT_STAGE</param-name>
    <param-value>Development</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.FACELETS_REFRESH_PERIOD</param-name>
    <param-value>1</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.FACELETS_SKIP_COMMENTS</param-name>
    <param-value>true</param-value>
</context-param>
<!--    <context-param>-->
<!--        <param-name>jakarta.faces.ALWAYS_PERFORM_VALIDATION_WHEN_REQUIRED_IS_TRUE</param-name>-->
<!--        <param-value>true</param-value>-->
<!--    </context-param>-->
<!--    <context-param>-->
<!--        <param-name>jakarta.faces.VALIDATE_EMPTY_FIELDS</param-name>-->
<!--        <param-value>true</param-value>-->
<!--    </context-param>-->
<context-param>
    <param-name>jakarta.faces.INTERPRET_EMPTY_STRING_SUBMITTED_VALUES_AS_NULL</param-name>
    <param-value>true</param-value>
</context-param>
<context-param>
    <param-name>jakarta.faces.DATETIMECONVERTER_DEFAULT_TIMEZONE_IS_SYSTEM_TIMEZONE</param-name>
    <param-value>true</param-value>
</context-param>
<context-param>
    <param-name>com.sun.faces.defaultResourceMaxAge</param-name>
    <param-value>31536000000</param-value> <!-- 1 year -->
</context-param>

<!-- JSF Extensionless Mapping disabled: duplicate content! -->
<context-param>
    <param-name>jakarta.faces.AUTOMATIC_EXTENSIONLESS_MAPPING</param-name>
    <param-value>false</param-value>
</context-param>

<!-- Omnifaces Configuration -->
<context-param>
    <param-name>org.omnifaces.FACES_VIEWS_SCAN_PATHS</param-name>
    <param-value>/*.xhtml</param-value>
</context-param>

<!-- Welcome file -->
<welcome-file-list>
    <welcome-file>index</welcome-file>
</welcome-file-list>

<!-- Error pages -->
<error-page>
    <exception-type>java.lang.Throwable</exception-type>
    <location>/error.xhtml</location> 
</error-page>

<!-- Session Configuration -->
<session-config>
    <session-timeout>5</session-timeout>
    <cookie-config>
        <http-only>true</http-only>
        <secure>false</secure>
    </cookie-config>
    <tracking-mode>COOKIE</tracking-mode>
</session-config>

I use the EL from Tomcat

this is the pom

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>com.twoobeet.jakarta</groupId>
    <artifactId>Playground</artifactId>
    <version>1.0</version>
    <name>Playground</name>
    <packaging>war</packaging>

    <properties>
        <java.version>17</java.version>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <maven.compiler.target>${java.version}</maven.compiler.target>
        <maven.compiler.source>${java.version}</maven.compiler.source>
        <junit.version>5.8.1</junit.version>
        <!--
        Jakarta REST (Jersey)
        https://northcoder.com/post/jakarta-rest-jaxrs-on-tomcat-10/
        https://stackoverflow.com/questions/62927426/getting-java-lang-classnotfoundexception-jakarta-servlet-filter-on-maven-jersey
        -->
        <jersey.version>3.1.0</jersey.version>

    </properties>

    <dependencies>

        <!--
        Jakarta EE 10 | Tomcat 10.1 | Servlet 6 + JASPIC 2 (Auth) + JSP 3.1 + EL 5.0 + Common Annotations 2 (annotation-api) + WebSocket 2.1
        -->

        <!-- Jakarta 10.0 API | Compile only!
        <dependency>
            <groupId>jakarta.platform</groupId>
            <artifactId>jakarta.jakartaee-api</artifactId>
            <version>10.0.0</version>
            <scope>provided</scope>
        </dependency>
        -->

        <dependency>
            <groupId>jakarta.servlet</groupId>
            <artifactId>jakarta.servlet-api</artifactId>
            <version>6.0.0</version>
            <scope>provided</scope>
        </dependency>
        <dependency>
            <groupId>jakarta.servlet.jsp</groupId>
            <artifactId>jakarta.servlet.jsp-api</artifactId>
            <version>3.1.0</version>
            <scope>provided</scope>
        </dependency>
        <dependency>
            <groupId>jakarta.el</groupId>
            <artifactId>jakarta.el-api</artifactId>
            <version>5.0.1</version>
            <scope>provided</scope>
        </dependency>
        <dependency>
            <groupId>jakarta.websocket</groupId>
            <artifactId>jakarta.websocket-api</artifactId>
            <version>2.1.0</version>
            <scope>provided</scope>
        </dependency>

        <!-- JAXB (XML) -->
        <dependency>
            <groupId>org.glassfish.jaxb</groupId>
            <artifactId>jaxb-runtime</artifactId>
            <version>4.0.1</version>
        </dependency>

        <!-- Jakarta JSON -->
        <dependency>
            <groupId>org.glassfish</groupId>
            <artifactId>jakarta.json</artifactId>
            <version>2.0.1</version>
        </dependency>

        <!-- CDI -->
        <dependency>
            <groupId>org.jboss.weld.servlet</groupId>
            <artifactId>weld-servlet-shaded</artifactId>
            <version>5.1.0.Final</version>
        </dependency>

        <!-- JSF 4 | Official release -->
        <dependency>
            <groupId>org.glassfish</groupId>
            <artifactId>jakarta.faces</artifactId>
            <version>4.0.0</version>
        </dependency>

         <!-- JSTL -->
        <dependency>
            <groupId>org.glassfish.web</groupId>
            <artifactId>jakarta.servlet.jsp.jstl</artifactId>
            <version>3.0.1</version>
        </dependency>

        <!-- Omnifaces 4 -->
        <dependency>
            <groupId>org.omnifaces</groupId>
            <artifactId>omnifaces</artifactId>
            <version>4.0.1</version>
        </dependency>

        <!-- Validation -->
        <dependency>
            <groupId>org.hibernate.validator</groupId>
            <artifactId>hibernate-validator</artifactId>
            <version>8.0.0.Final</version>
        </dependency>

        <!-- Jakarta REST | Jersey servlet -->
        <dependency>
            <groupId>org.glassfish.jersey.containers</groupId>
            <artifactId>jersey-container-servlet</artifactId>
            <version>${jersey.version}</version>
        </dependency>
        <dependency>
            <groupId>org.glassfish.jersey.inject</groupId>
            <artifactId>jersey-hk2</artifactId>
            <version>${jersey.version}</version>
        </dependency>
        <!-- Jakarta REST | Jersey Dependency Injection -->
        <dependency>
            <groupId>org.glassfish.jersey.ext.cdi</groupId>
            <artifactId>jersey-cdi1x</artifactId>
            <version>${jersey.version}</version>
        </dependency>
        <dependency>
            <groupId>org.glassfish.jersey.ext.cdi</groupId>
            <artifactId>jersey-cdi1x-servlet</artifactId>
            <version>${jersey.version}</version>
        </dependency>
        <!-- Jakarta REST | Jersey JSON support -->
        <dependency>
            <groupId>org.glassfish.jersey.media</groupId>
            <artifactId>jersey-media-json-jackson</artifactId>
            <version>${jersey.version}</version>
        </dependency>

        <!-- Apache Common -->
        <dependency>
            <groupId>org.apache.commons</groupId>
            <artifactId>commons-lang3</artifactId>
            <version>3.12.0</version>
        </dependency>

    </dependencies>

    <build>

        <finalName>playground</finalName>

        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-war-plugin</artifactId>
                <version>3.3.1</version>
            </plugin>
        </plugins>
    </build>

</project>
BalusC commented 1 year ago

I was wrong, I accidentally tested the case with MyFaces instead of Mojarra. And it turns out that MyFaces has the "empty string to null" EL resolver installed by itself. For Mojarra you'll indeed have to supply your own. Detailed background info on this Tomcat behavior: https://balusc.omnifaces.org/2015/10/the-empty-string-madness.html

I'll look at adding the EL resolver in Mojarra too.

Noted should be that this doesn't affect specifically the h:selectOneMenu but all kinds of inputs. E.g. an empty h:inputText would incorrectly also set empty string instead of null when Tomcat is used.

pizzi80 commented 1 year ago

I was wrong, I accidentally tested the case with MyFaces instead of Mojarra. And it turns out that MyFaces has the "empty string to null" EL resolver installed by itself. For Mojarra you'll indeed have to supply your own.

:+1: :+1:

Detailed background info on this Tomcat behavior: https://balusc.omnifaces.org/2015/10/the-empty-string-madness.html

Yes, I've read your article many times! ... :smiley:

It's frustrating to see that after so many years there is still this bug in Tomcat, which could probably be resolved with 1 o 2 line of code ....

I'll look at adding the EL resolver in Mojarra too.

:top: ... I don't know if, working at jsf level, there is a better technique that is more performant, for example skipping "EL String conversion" during input binding ... (??)

Another solution is to fork Tomcat and modify the incriminated 2 o 3 lines of code :smiley:

... Also @arjantijms would be happy to fork and add a getRequest method inside Tomcat request facade since many years and a ServletContext initializer with priority ... :smiley:

Noted should be that this doesn't affect specifically the h:selectOneMenu but all kinds of inputs. E.g. an empty h:inputText would incorrectly also set empty string instead of null.

Yes, exactly, I've reported the f:selectItem case because it's a classic example where the null String it's a requirements of basic webapp binding ... but also for inputText it's very important, otherwise one will end with the DB full of empty string values instead of null(s) ...

BalusC commented 1 year ago

Closing off, was already merged.