eclipse-ee4j / mojarra

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

EmptyStringToNullELResolver NPE #5226

Closed pizzi80 closed 1 year ago

pizzi80 commented 1 year ago

Describe the bug

with the EmptyStringToNullELResolver active

EL string concatenation has problems

To Reproduce

#{'hello'.concat(null)}

Stack trace: java.lang.NullPointerException: Cannot invoke "String.isEmpty()" because "str" is null 
at java.base/java.lang.String.concat(String.java:2769) 
at java.base/jdk.internal.reflect.GeneratedMethodAccessor598.invoke(Unknown Source) 
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) 
at java.base/java.lang.reflect.Method.invoke(Method.java:568) 
at jakarta.el.BeanELResolver.invoke(BeanELResolver.java:161) 
at jakarta.el.CompositeELResolver.invoke(CompositeELResolver.java:78) 
at org.apache.el.parser.AstValue.getValue(AstValue.java:159) 
at org.apache.el.parser.AstBracketSuffix.getValue(AstBracketSuffix.java:36) 
at org.apache.el.parser.AstValue.getValue(AstValue.java:143) 
at org.apache.el.parser.AstMethodParameters.getParameters(AstMethodParameters.java:33) 
at org.apache.el.parser.AstValue.getValue(AstValue.java:158) 
at org.apache.el.ValueExpressionImpl.getValue(ValueExpressionImpl.java:190) 
at org.jboss.weld.module.web.el.WeldValueExpression.getValue(WeldValueExpression.java:50) 
at com.sun.faces.facelets.el.ELText$ELTextVariable.writeText(ELText.java:208) 
at com.sun.faces.facelets.compiler.TextInstruction.write(TextInstruction.java:44) 
at com.sun.faces.facelets.compiler.UIInstructions.encodeBegin(UIInstructions.java:42) 
at com.sun.faces.renderkit.html_basic.HtmlBasicRenderer.encodeRecursive(HtmlBasicRenderer.java:255) 
at com.sun.faces.renderkit.html_basic.PassthroughRenderer.encodeChildren(PassthroughRenderer.java:75) 
at jakarta.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:557) 
at com.sun.faces.renderkit.html_basic.HtmlBasicRenderer.encodeRecursive(HtmlBasicRenderer.java:257) 
at com.sun.faces.renderkit.html_basic.HtmlBasicRenderer.encodeRecursive(HtmlBasicRenderer.java:262) 
at com.sun.faces.renderkit.html_basic.HtmlBasicRenderer.encodeRecursive(HtmlBasicRenderer.java:262) 
at com.sun.faces.renderkit.html_basic.PassthroughRenderer.encodeChildren(PassthroughRenderer.java:75) 
at jakarta.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:557) 
at jakarta.faces.component.UIComponent.encodeAll(UIComponent.java:1436) 
at jakarta.faces.render.Renderer.encodeChildren(Renderer.java:146) 
at jakarta.faces.component.UIComponentBase.encodeChildren(UIComponentBase.java:557) 
at jakarta.faces.component.UIComponent.encodeAll(UIComponent.java:1436) 
at com.sun.faces.context.PartialViewContextImpl$PhaseAwareVisitCallback.visit(PartialViewContextImpl.java:594) 
at com.sun.faces.component.visit.PartialVisitContext.invokeVisitCallback(PartialVisitContext.java:158) 
at jakarta.faces.component.UIForm.visitTree(UIForm.java:354) 
at jakarta.faces.component.UIComponent.visitTree(UIComponent.java:1279) 
at jakarta.faces.component.UIComponent.visitTree(UIComponent.java:1279) 
at com.sun.faces.context.PartialViewContextImpl.processComponents(PartialViewContextImpl.java:392) 
at com.sun.faces.context.PartialViewContextImpl.processPartial(PartialViewContextImpl.java:307) 
at jakarta.faces.context.PartialViewContextWrapper.processPartial(PartialViewContextWrapper.java:239) 
at org.omnifaces.context.OmniPartialViewContext.processPartial(OmniPartialViewContext.java:122) 
at jakarta.faces.component.UIViewRoot.encodeChildren(UIViewRoot.java:1086) 
at jakarta.faces.component.UIComponent.encodeAll(UIComponent.java:1436) 
at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:414) 
at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:162) 
at jakarta.faces.application.ViewHandlerWrapper.renderView(ViewHandlerWrapper.java:125) 
at jakarta.faces.application.ViewHandlerWrapper.renderView(ViewHandlerWrapper.java:125) 
at org.omnifaces.viewhandler.OmniViewHandler.renderView(OmniViewHandler.java:166) 
at jakarta.faces.application.ViewHandlerWrapper.renderView(ViewHandlerWrapper.java:125) 
at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:93) 
at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:72) 
at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:178) 
at jakarta.faces.webapp.FacesServlet.executeLifecyle(FacesServlet.java:692) 
at jakarta.faces.webapp.FacesServlet.service(FacesServlet.java:449) 
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:205) 
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) 
at org.omnifaces.filter.FacesExceptionFilter.doFilter(FacesExceptionFilter.java:118) 
at org.omnifaces.filter.HttpFilter.doFilter(HttpFilter.java:108) 
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) 
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) 
at org.omnifaces.filter.CacheControlFilter.doFilter(CacheControlFilter.java:239) 
at org.omnifaces.filter.HttpFilter.doFilter(HttpFilter.java:108) 
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) 
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) 
at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53) 
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) 
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) 
at org.omnifaces.facesviews.FacesViewsForwardingFilter.filterExtensionLessToExtension(FacesViewsForwardingFilter.java:187) 
at org.omnifaces.facesviews.FacesViewsForwardingFilter.filterExtensionLess(FacesViewsForwardingFilter.java:144) 
at org.omnifaces.facesviews.FacesViewsForwardingFilter.filterExtensionLess(FacesViewsForwardingFilter.java:130) 
at org.omnifaces.facesviews.FacesViewsForwardingFilter.doFilter(FacesViewsForwardingFilter.java:86) 
at org.omnifaces.filter.HttpFilter.doFilter(HttpFilter.java:108) 
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) 
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) 
at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:166) 
at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90) 
at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:608) 
at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:115) 
at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93) 
at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74) 
at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:341) 
at org.apache.coyote.ajp.AjpProcessor.service(AjpProcessor.java:440) 
at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63) 
at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:894) 
at org.apache.tomcat.util.net.Nio2Endpoint$SocketProcessor.doRun(Nio2Endpoint.java:1664) 
at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52) 
at org.apache.tomcat.util.net.AbstractEndpoint.processSocket(AbstractEndpoint.java:1219) 
at org.apache.tomcat.util.net.Nio2Endpoint.setSocketOptions(Nio2Endpoint.java:328) 
at org.apache.tomcat.util.net.Nio2Endpoint$Nio2Acceptor.completed(Nio2Endpoint.java:468) 
at org.apache.tomcat.util.net.Nio2Endpoint$Nio2Acceptor.completed(Nio2Endpoint.java:404) 
at java.base/sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:129) 
at java.base/sun.nio.ch.Invoker$2.run(Invoker.java:221) 
at java.base/sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:113) 
at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191) 
at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659) 
at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) 
at java.base/java.lang.Thread.run(Thread.java:833)

Expected behavior

hello

Additional context

Faces current master Java 17 Tomcat 10.1 Weld 5.0

pizzi80 commented 1 year ago

fixed

BalusC commented 1 year ago

Can you please put the fix for a separate issue in a separate PR?

pizzi80 commented 1 year ago

fixed with https://github.com/eclipse-ee4j/mojarra/pull/5228

BalusC commented 1 year ago

Left comment in PR.

BalusC commented 1 year ago

Another issue with same root cause: https://github.com/eclipse-ee4j/mojarra/issues/5220

I'll analyze and fix it for 2.3/3.0/4.0 because it also needs to be fixed in 2.3 and 3.0.

BalusC commented 1 year ago

5220 has been fixed.

The propsed PR #5228 causes #{bean.action(null)} / #{bean.setValue(null)} to retrieve empty string as argument which goes against the whole idea behind EmptyStringToNullELResolver.

On second thought I don't think #{'hello'.concat(null)} should work. I think the behavior is expected. I would find it unexpected when #{bean.method(null)} gives an empty string as argument. If you want to automatically coerce null to empty string in this specific case then you should be using the += operator as in #{'hello' += null}. This correctly prints hello without NPE.