MyFaces Core
  1. MyFaces Core
  2. MYFACES-3166

org.apache.myfaces.el.VariableResolverImpl throws java.lang.IllegalStateException when it unsets the scope as null

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.8
    • Fix Version/s: 1.2.11, 2.0.8, 2.1.2
    • Component/s: None
    • Labels:
      None
    • Environment:
      Linux
      Geronimo 2.1.7

      Description

      I am running an application in Geronimo 2.1.7 that requires valid scope and it fails with the following exception

      Servlet.service() for servlet jsp threw exception
      java.lang.IllegalStateException: unknown scope defined: null
      at
      org.apache.myfaces.el.VariableResolverImpl.resolveVariable(VariableResol
      verImpl.java:71)
      at
      org.apache.myfaces.el.convert.VariableResolverToELResolver.getValue(Vari
      ableResolverToELResolver.java:93)
      at
      javax.el.CompositeELResolver.getValue(CompositeELResolver.java:53)
      ---------------
      ---------------
      Servlet.service() for servlet jsp threw exception
      java.lang.NullPointerException
      at
      javax.el.CompositeELResolver.getValue(CompositeELResolver.java:53)
      at
      org.apache.myfaces.el.unified.resolver.FacesCompositeELResolver.access$3
      01(FacesCompositeELResolver.java:46)
      at
      org.apache.myfaces.el.unified.resolver.FacesCompositeELResolver$4.invoke
      (FacesCompositeELResolver.java:108)
      at
      org.apache.myfaces.el.unified.resolver.FacesCompositeELResolver.invoke(F
      acesCompositeELResolver.java:148)
      at
      org.apache.myfaces.el.unified.resolver.FacesCompositeELResolver.getValue
      (FacesCompositeELResolver.java:104)
      at
      javax.el.CompositeELResolver.getValue(CompositeELResolver.java:53)

      After investigating on codebase i see that the following is the code snippet where one thread sets the scope and some other thread on the same context unsets scope to null before a call made to org.apache.myfaces.el.VariableResolverImpl.resolveVariable and it leads the above exception

      try

      { setScope(requestMap); super.setValue(context, base, property, val); }

      finally

      { unsetScope(requestMap); }

      So to fix the scope i have changed the following code snippet in the FacesCompositeELResolver to put back previous scope value

      try

      { tmpScope = getScope(requestMap); setScope(requestMap); super.setValue(context, base, property, val); }

      finally
      {
      if( tmpScope != null)

      { setScope(requestMap, tmpScope); }

      else

      { unsetScope(requestMap); }

      }

      I am attaching the code with all the changes to fix this issue.Please review and provide your comments.

        Activity

        Mohan Reddy created issue -
        Mohan Reddy made changes -
        Field Original Value New Value
        Attachment MYFACES-3166.patch [ 12481221 ]
        Hide
        Leonardo Uribe added a comment -

        I have checked it and the scenario proposed is not very common. Which setup do you have? why two theads try to resolve an EL Expressions for the same request? Are you using portlets? Are you overriding Application.getELResolver()?

        It this probably this bug will not be in 2.0 and upper versions, because in that case facesContext attribute map is used (in 1.2 is request map). But before commit this it is necessary to know the conditions this bug happens, so we can check if the proposed solution will work well.

        Show
        Leonardo Uribe added a comment - I have checked it and the scenario proposed is not very common. Which setup do you have? why two theads try to resolve an EL Expressions for the same request? Are you using portlets? Are you overriding Application.getELResolver()? It this probably this bug will not be in 2.0 and upper versions, because in that case facesContext attribute map is used (in 1.2 is request map). But before commit this it is necessary to know the conditions this bug happens, so we can check if the proposed solution will work well.
        Hide
        Mohan Reddy added a comment -

        Actually it is not by two threads.It's only one thread calling getValue() in nested way.
        The call sequence for this scenario as given below
        1) Invokes getValue() and sets scope to JSP
        2) and internally the call further invokes getValue() which sets scope to FACES and then it sets scope to null

        So when the call searches for scope JSP by the time the scope already been set to null and leads to above exception.What we modified here is before setting the scope to null we are checking the previous scope and if it is not null then setting the scope as previous one otherwise setting it to null.

        The proposed solution is working for customer and i see from the new code that it does not harm other functionality as this works in similar manner as it is working currently when there is no netsted calls to getValue()

        Show
        Mohan Reddy added a comment - Actually it is not by two threads.It's only one thread calling getValue() in nested way. The call sequence for this scenario as given below 1) Invokes getValue() and sets scope to JSP 2) and internally the call further invokes getValue() which sets scope to FACES and then it sets scope to null So when the call searches for scope JSP by the time the scope already been set to null and leads to above exception.What we modified here is before setting the scope to null we are checking the previous scope and if it is not null then setting the scope as previous one otherwise setting it to null. The proposed solution is working for customer and i see from the new code that it does not harm other functionality as this works in similar manner as it is working currently when there is no netsted calls to getValue()
        Hide
        Leonardo Uribe added a comment -

        I have checked the solution proposed and with the new info provided we can commit it. This is valid for 1.2.x and upper versions.

        Thanks to Mohan Reddy for provide this patch.

        Show
        Leonardo Uribe added a comment - I have checked the solution proposed and with the new info provided we can commit it. This is valid for 1.2.x and upper versions. Thanks to Mohan Reddy for provide this patch.
        Leonardo Uribe made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Leonardo Uribe [ lu4242 ]
        Fix Version/s 1.2.11 [ 12316650 ]
        Fix Version/s 2.0.8 [ 12316514 ]
        Fix Version/s 2.1.2 [ 12316512 ]
        Resolution Fixed [ 1 ]
        Leonardo Uribe made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Leonardo Uribe
            Reporter:
            Mohan Reddy
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development