javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-3215

UIRepeat needlessly evaluates EL for EditableValueHolders when saving state which can cause exceptions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2.7
    • Component/s: state saving
    • Labels:
      None

      Description

      The UIRepeat component gets the value of all its child components of type EditableValueHolder for every iteration of the collection bound to it when saving state. This happens in com.sun.faces.facelets.component.UIRepeat.SavedState.populate(EditableValueHolder):

      public void populate(EditableValueHolder evh) {
          this.value = evh.getValue();
          this.valid = evh.isValid();
          this.submittedValue = evh.getSubmittedValue();
          this.localValueSet = evh.isLocalValueSet();
      }
      

      This is problematic for those cases where the child component's value is bound to a bean property that does not exist for a specific iteration, and is guarded at rendering time by some means. E.g. when property a only exists for class test.TypeA and b only exists for class test.TypeB:

      <ui:repeat var="object" value="#{myBean.objects}">
          <h:panelGroup rendered="#{object.class.name eq 'test.TypeA'}">
              <h:inputText value="#{object.a}" />
          </h:panelGroup>
                      
          <h:panelGroup rendered="#{object.class.name eq 'test.TypeB'}">
              <h:inputText value="#{object.b}" />
          </h:panelGroup>
      </ui:repeat>
      

      By contrast, UIData does not have this problem. The main difference is that when saving state it gets the localValue of the EditableValueHolder. This happens in javax.faces.component.UIData.saveDescendantState(UIComponent, FacesContext) as follows:

      state.setValue(input.getLocalValue());
      state.setValid(input.isValid());
      state.setSubmittedValue(input.getSubmittedValue());
      state.setLocalValueSet(input.isLocalValueSet());
      

      Changing UIRepeat to do the same seems to fix the issue, e.g.;

      public void populate(EditableValueHolder evh) {
          this.value = evh.getLocalValue();
          this.valid = evh.isValid();
          this.submittedValue = evh.getSubmittedValue();
          this.localValueSet = evh.isLocalValueSet();
      }
      

      A reproducer is available online; https://github.com/arjantijms/mojarra-uirepeat

        Activity

        Hide
        Manfred Riem added a comment -

        Applied to 2.2 branch,

        svn commit -m "Fixes https://java.net/jira/browse/JAVASERVERFACES-3215, make ui:repeat use getLocalValue to be inline with h:dataTable."
        Sending jsf-ri/src/main/java/com/sun/faces/facelets/component/UIRepeat.java
        Adding test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueBean.java
        Adding test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeA.java
        Adding test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeB.java
        Adding test/agnostic/facelets/ui/src/main/webapp/repeatUseLocalValue.xhtml
        Adding test/agnostic/facelets/ui/src/test/java/com/sun/faces/test/agnostic/facelets/ui/Issue3215IT.java
        Transmitting file data ......
        Committed revision 13078.

        Show
        Manfred Riem added a comment - Applied to 2.2 branch, svn commit -m "Fixes https://java.net/jira/browse/JAVASERVERFACES-3215 , make ui:repeat use getLocalValue to be inline with h:dataTable." Sending jsf-ri/src/main/java/com/sun/faces/facelets/component/UIRepeat.java Adding test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueBean.java Adding test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeA.java Adding test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeB.java Adding test/agnostic/facelets/ui/src/main/webapp/repeatUseLocalValue.xhtml Adding test/agnostic/facelets/ui/src/test/java/com/sun/faces/test/agnostic/facelets/ui/Issue3215IT.java Transmitting file data ...... Committed revision 13078.
        Hide
        Manfred Riem added a comment -

        Applied to 2.2 branch,

        svn commit -m "Minor rework on the test to make Tomcat happy"
        Sending test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeA.java
        Sending test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeB.java
        Sending test/agnostic/facelets/ui/src/main/webapp/repeatUseLocalValue.xhtml
        Transmitting file data ...
        Committed revision 13079.

        Show
        Manfred Riem added a comment - Applied to 2.2 branch, svn commit -m "Minor rework on the test to make Tomcat happy" Sending test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeA.java Sending test/agnostic/facelets/ui/src/main/java/com/sun/faces/test/agnostic/facelets/ui/RepeatUseLocalValueTypeB.java Sending test/agnostic/facelets/ui/src/main/webapp/repeatUseLocalValue.xhtml Transmitting file data ... Committed revision 13079.
        Hide
        arjan tijms added a comment -

        Thanks for the fast fix!

        As the issue is also in 2.1, it might make sense to backport the fix to that branch as well.

        Show
        arjan tijms added a comment - Thanks for the fast fix! As the issue is also in 2.1, it might make sense to backport the fix to that branch as well.
        Hide
        Manfred Riem added a comment -

        Please file a backport task and link it to this issue

        Show
        Manfred Riem added a comment - Please file a backport task and link it to this issue

          People

          • Assignee:
            Manfred Riem
            Reporter:
            arjan tijms
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: