MyFaces Core
  1. MyFaces Core
  2. MYFACES-3383

Self nested Composite Component leads to EL Stack Overflow

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.0.15, 2.1.9
    • Component/s: None
    • Labels:
      None
    • Environment:
      Windows

      Description

      If the same Composite Component is used inside itself, e.g. as child of an ui:include that is included in the Composite Component, an StackOverflow happens, if an EL Expression is refering CC interface attributes.

      The use case is a CC to include a facelet, given by name. If the included facelet uses the same CC to include another facelet, "CompositeComponentELUtils.getCompositeComponentBasedOnLocation(..)" does always find the same CC, which leads to an endless loop.

      Please see the attached file for an example of the issue.

      1. TestStackOverflow.war
        3.37 MB
        Michael Dietrich
      2. MYFACES-3383-1.patch
        111 kB
        Leonardo Uribe

        Activity

        Michael Dietrich created issue -
        Michael Dietrich made changes -
        Field Original Value New Value
        Attachment TestStackOverflow.war [ 12502114 ]
        Hide
        Leonardo Uribe added a comment -

        I have checked the example provided, and unfortunately this is not going to work with the current strategy to resolve cc expressions.

        In few words, org.apache.myfaces.view.facelets.tag.TagAttributeImpl wraps every expression that resolve cc into a LocationValueExpression, and use the Location object to find the right instance. This hack has worked well so far but obviously fails in this case, because Location object is the same per composite component, and does not keep into account the position of the component in the tree, which is relevant in this case.

        This is a very, very rare use case. Solve this problem will not going to be easy.

        Show
        Leonardo Uribe added a comment - I have checked the example provided, and unfortunately this is not going to work with the current strategy to resolve cc expressions. In few words, org.apache.myfaces.view.facelets.tag.TagAttributeImpl wraps every expression that resolve cc into a LocationValueExpression, and use the Location object to find the right instance. This hack has worked well so far but obviously fails in this case, because Location object is the same per composite component, and does not keep into account the position of the component in the tree, which is relevant in this case. This is a very, very rare use case. Solve this problem will not going to be easy.
        Hide
        Leonardo Uribe added a comment -

        I reviewed this issue again, trying to do something over LocationValueExpression. The idea was store the current nesting level where the ValueExpression was created related to the current composite component. It works, but it has some side effects like make EL expressions "context dependant" which evidence the initial problem.

        In few words, once the component tree is created by facelets engine, the "page" context get lost. What we really need is some way to preserve the page context, not tweak LocationValueExpression. Anyway, do that seems to be very difficult and inconvenient, because you need to store such relationship in the EL expression and the resulting design can be broken easily if the component is relocated or manipulated in some way.

        Show
        Leonardo Uribe added a comment - I reviewed this issue again, trying to do something over LocationValueExpression. The idea was store the current nesting level where the ValueExpression was created related to the current composite component. It works, but it has some side effects like make EL expressions "context dependant" which evidence the initial problem. In few words, once the component tree is created by facelets engine, the "page" context get lost. What we really need is some way to preserve the page context, not tweak LocationValueExpression. Anyway, do that seems to be very difficult and inconvenient, because you need to store such relationship in the EL expression and the resulting design can be broken easily if the component is relocated or manipulated in some way.
        Leonardo Uribe made changes -
        Attachment MYFACES-3383-1.patch [ 12532074 ]
        Hide
        Leonardo Uribe added a comment -

        I have found a good solution for this issue, that passes all tests done. The idea is create a counter that identify the composite component nesting level and attach it to all created EL expressions, like we do with the Location object. This counter helps to identify the "context" where this EL expression belongs. If the counter is 0, the algorithm works as is working now, but if not, this info is taken into account when the algorithm try to found the composite component.

        The hack works well in complex situations, but since the ccLevel is context dependant information and can change, the EL caching algorithm should change a little bit. I have not found any side effects, all junit tests done in that part works, even the ones using "findComponent expression" hack, used to resolve the library name h:outputScript or h:outputStylesheet expressions.

        I also tried the example provided and it works. It took me a lot of time to understand and solve this one fully.

        If no objections I'll commit the proposed solution soon.

        Show
        Leonardo Uribe added a comment - I have found a good solution for this issue, that passes all tests done. The idea is create a counter that identify the composite component nesting level and attach it to all created EL expressions, like we do with the Location object. This counter helps to identify the "context" where this EL expression belongs. If the counter is 0, the algorithm works as is working now, but if not, this info is taken into account when the algorithm try to found the composite component. The hack works well in complex situations, but since the ccLevel is context dependant information and can change, the EL caching algorithm should change a little bit. I have not found any side effects, all junit tests done in that part works, even the ones using "findComponent expression" hack, used to resolve the library name h:outputScript or h:outputStylesheet expressions. I also tried the example provided and it works. It took me a lot of time to understand and solve this one fully. If no objections I'll commit the proposed solution soon.
        Leonardo Uribe made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Leonardo Uribe [ lu4242 ]
        Fix Version/s 2.0.15 [ 12321753 ]
        Fix Version/s 2.1.9 [ 12321755 ]
        Resolution Fixed [ 1 ]
        Hide
        Michael Dietrich added a comment -

        Hi Leonardo,

        good job, thanks a lot.

        Kind regards,

        Michael

        Show
        Michael Dietrich added a comment - Hi Leonardo, good job, thanks a lot. Kind regards, Michael
        Leonardo Uribe made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Leonardo Uribe
            Reporter:
            Michael Dietrich
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development