Pluto
  1. Pluto
  2. PLUTO-554

Infinite invocation when a view page of a portlet tries to include some result from other servlet path.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.0.0
    • Component/s: None
    • Labels:
      None

      Description

      If a dispatched page from a portlet to render itself tries to include other page, then the original page is being invoked infinitely.
      Here's the steps to reproduce this problem in my local environment:

      (1) Build and deploy pluto-2.0.0-SNAPSHOT into Tomcat.
      (2) Start Tomcat, log on to the pluto portal and move to the Test Page.
      (3) Edit /testsuite/jsp/introduction.jsp to include the following somewhere:

      <c:import url="/jsp/help.jsp">
      </c:import>

      (4) Try to view the Test Page again.

      In the $CATALINA_HOME/logs/testsuite.yyyy-MM-dd.log file, I can find huge error logs like the following:

      Apr 22, 2009 2:20:18 PM org.apache.catalina.core.ApplicationDispatcher invoke
      SEVERE: Servlet.service() for servlet jsp threw exception
      java.lang.StackOverflowError
      at org.apache.catalina.core.ApplicationHttpRequest.getAttribute(ApplicationHttpRequest.java:210)
      at org.apache.catalina.core.ApplicationHttpRequest.getAttribute(ApplicationHttpRequest.java:222)
      at org.apache.catalina.core.ApplicationHttpRequest.getAttribute(ApplicationHttpRequest.java:222)
      <SNIP>
      at org.apache.catalina.core.ApplicationHttpRequest.getAttribute(ApplicationHttpRequest.java:222)
      at org.apache.pluto.container.impl.HttpServletPortletRequestWrapper.getAttribute(HttpServletPortletRequestWrapper.java:516)
      at org.apache.jasper.servlet.JspServlet.service(JspServlet.java:212)
      <SNIP>
      at org.apache.jsp.jsp.introduction_jsp._jspx_meth_c_005fimport_005f0(introduction_jsp.java:351)
      at org.apache.jsp.jsp.introduction_jsp._jspService(introduction_jsp.java:136)
      at org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:98)
      at javax.servlet.http.HttpServlet.service(HttpServlet.java:729)
      <SNIP>
      at org.apache.jsp.jsp.introduction_jsp._jspx_meth_c_005fimport_005f0(introduction_jsp.java:351)
      at org.apache.jsp.jsp.introduction_jsp._jspService(introduction_jsp.java:136)
      at org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:98)
      at javax.servlet.http.HttpServlet.service(HttpServlet.java:729)
      <SNIP>
      at org.apache.jsp.jsp.introduction_jsp._jspx_meth_c_005fimport_005f0(introduction_jsp.java:351)
      at org.apache.jsp.jsp.introduction_jsp._jspService(introduction_jsp.java:136)
      at org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:98)
      at javax.servlet.http.HttpServlet.service(HttpServlet.java:729)
      <SNIP>

      I tried to find the cause of this problem.
      Currently I found a suspicious line in org.apache.pluto.container.impl.HttpServletPortletRequestWrapper#getAttribute(String).
      The method will return a pre-stored attribute value for "javax.servlet.include.servlet_path" attribute name.
      Therefore, if the container tries to look up the included servlet path, then this wrapper request returns the same jsp page.
      That's why this problem happen, I think.

      I'd like to look into this more and find a solution to fix this.

        Activity

        Hide
        Woonsan Ko added a comment -

        Servlet spec requires the request attributes such as javax.servlet.include.servlet_path (SRV.8.3.1) should be replaced when the request is subsequently included.
        So, I added kind of flag setting methods, setDispatching() and isDispatching() and add some lines in getAttribute() to make it behave as normal when it is dispatching to other.

        Previously, the HttpServletPortletRequestWrapper uses the pre-stored path info for some reason. (I cannot figure out the reason(s) yet though.)
        OTOH, PortletRequestDispatcherImpl has two pairs for include() and forward(). One pair is for portlet request/response and the other pair is for servlet request/response.
        The methods for servlet request/response pair, include() and forward(), try to restore the path info after dispatching.
        However, these two methods for servlet request/response pair do not seem to be invoked ever because containers like Tomcat uses its own RequestDispatcher implementation. (Maybe there're some use cases I'm missing.)

        Anyway, I tried to fix the problem without making any side effects this time.

        Show
        Woonsan Ko added a comment - Servlet spec requires the request attributes such as javax.servlet.include.servlet_path (SRV.8.3.1) should be replaced when the request is subsequently included. So, I added kind of flag setting methods, setDispatching() and isDispatching() and add some lines in getAttribute() to make it behave as normal when it is dispatching to other. Previously, the HttpServletPortletRequestWrapper uses the pre-stored path info for some reason . (I cannot figure out the reason(s) yet though.) OTOH, PortletRequestDispatcherImpl has two pairs for include() and forward(). One pair is for portlet request/response and the other pair is for servlet request/response. The methods for servlet request/response pair, include() and forward(), try to restore the path info after dispatching. However, these two methods for servlet request/response pair do not seem to be invoked ever because containers like Tomcat uses its own RequestDispatcher implementation. (Maybe there're some use cases I'm missing.) Anyway, I tried to fix the problem without making any side effects this time.
        Hide
        Ate Douma added a comment -

        Hi Woonsan,

        I'm afraid your changes now break the JSR-286 TCK with 6 tests failing.

        As I wrote the path_info handling, which was needed to be compliant with both the servlet and portlet spec, I'll look into it and see what is causing the original error.

        Show
        Ate Douma added a comment - Hi Woonsan, I'm afraid your changes now break the JSR-286 TCK with 6 tests failing. As I wrote the path_info handling, which was needed to be compliant with both the servlet and portlet spec, I'll look into it and see what is causing the original error.
        Hide
        Ate Douma added a comment -

        I've committed a new fix which is compliant with the JSR-286 TCK and also now properly supports ServletContext.getRequestDispatcher instead of only request.getRequestDispatcher

        But, as an important note: the current implementation is very much written and tested against Tomcat and we already know this will not work correctly on other webcontainers like Jetty or Websphere...

        We're currently searching very hard for a way to "solve" this more generically and (if even possible) webcontainer independent,
        but right now it very much looks like we're hitting the wall and limitations of the servlet spec and portlet spec, which might mean
        a solution for those other webcontainers won't be possible without using intimate knowledge and proprietary api.

        If we find any way to solve this, we'll create a separate issue for it.

        Show
        Ate Douma added a comment - I've committed a new fix which is compliant with the JSR-286 TCK and also now properly supports ServletContext.getRequestDispatcher instead of only request.getRequestDispatcher But, as an important note: the current implementation is very much written and tested against Tomcat and we already know this will not work correctly on other webcontainers like Jetty or Websphere... We're currently searching very hard for a way to "solve" this more generically and (if even possible) webcontainer independent, but right now it very much looks like we're hitting the wall and limitations of the servlet spec and portlet spec, which might mean a solution for those other webcontainers won't be possible without using intimate knowledge and proprietary api. If we find any way to solve this, we'll create a separate issue for it.

          People

          • Assignee:
            Ate Douma
            Reporter:
            Woonsan Ko
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development