glassfish
  1. glassfish
  2. GLASSFISH-2751

If the hosting server does not want to use ErrorReportValve for serving errorpages, GF web container must honour it at all times.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 9.1pe
    • Fix Version/s: 9.1pe_dev
    • Component/s: web_container
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      2,751

      Description

      In Web Server, we don't use ErrorReportValve and leave error pages part to core
      for uniformity. We do this by explicitly calling setErrorReportValveClass(null)
      during startup.

      Fix for 6374990 uses ErrorReportValve class unconditionally and causes
      RequestDispatcher.forward() to be served by ErrorReportValve in case of error
      thus causing inconsistency for WS. B/c of this, if the forward-dispatch happens
      to a non-existing resource, ErrorReportValve sets the error page w/o letting the
      core to serve the error page thus causing non-uniformity for Web Server.

        Activity

        Hide
        kmeduri added a comment -

        Suggested fix for PWC12Dev_Branch:
        ===================================
        The following diff also takes care of flushing the response when status < 400.
        With this, the orginal issue reported under BT CR 6374990 is resolved for Web
        Server and complies with Section SRV.8.4 that states "before the forward()
        method of the RequestDispatcher interface returns, the response content must be
        sent and committed, and closed by the servlet container."

        Index:
        appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java
        ===================================================================
        RCS file:
        /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v
        retrieving revision 1.4.8.1
        diff -u -r1.4.8.1 ApplicationDispatcherForward.java

        appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java
        9 Nov 2006 03:30:56 -0000 1.4.8.1
        +++
        appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java
        28 Mar 2007 12:41:48 -0000
        @@ -77,29 +77,37 @@
        throws IOException, ServletException {

        int statusCode = getStatus(response);

        • if (statusCode >= 400
        • && request.getAttribute(Globals.EXCEPTION_ATTR) == null) {
        • boolean matchFound = status(request, response, context, wrapper,
        • statusCode);
        • if (!matchFound) {
        • serveDefaultErrorPage(request, response, statusCode);
          + String errorReportValveStr =
          + ((StandardHost)(context.getParent())).getErrorReportValveClass();
          + if (errorReportValveStr != null) {
          + if (statusCode >= 400
          + && request.getAttribute(Globals.EXCEPTION_ATTR) == null)
          Unknown macro: {+ boolean matchFound = status(request, response, context, wrapper,+ statusCode);+ if (!matchFound) { + serveDefaultErrorPage(request, response, statusCode); + } }

          }

        • try { - PrintWriter writer = response.getWriter(); - writer.close(); - }

          catch (IllegalStateException e) {
          + if ((statusCode < 400) || (errorReportValveStr != null)) {
          try

          { - ServletOutputStream stream = response.getOutputStream(); - stream.close(); - }

          catch (IllegalStateException f)

          { - ; - }

          catch (IOException f)

          { + PrintWriter writer = response.getWriter(); + writer.flush(); + writer.close(); + }

          catch (IllegalStateException e)

          Unknown macro: {+ try { + ServletOutputStream stream = response.getOutputStream(); + stream.flush(); + stream.close(); + } catch (IllegalStateException f) { + ; + } catch (IOException f) { + ; + }+ }

          catch (IOException e)

          { ; }
        • } catch (IOException e) { - ; }

          }

        Show
        kmeduri added a comment - Suggested fix for PWC12Dev_Branch: =================================== The following diff also takes care of flushing the response when status < 400. With this, the orginal issue reported under BT CR 6374990 is resolved for Web Server and complies with Section SRV.8.4 that states "before the forward() method of the RequestDispatcher interface returns, the response content must be sent and committed, and closed by the servlet container." Index: appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java =================================================================== RCS file: /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v retrieving revision 1.4.8.1 diff -u -r1.4.8.1 ApplicationDispatcherForward.java — appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java 9 Nov 2006 03:30:56 -0000 1.4.8.1 +++ appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java 28 Mar 2007 12:41:48 -0000 @@ -77,29 +77,37 @@ throws IOException, ServletException { int statusCode = getStatus(response); if (statusCode >= 400 && request.getAttribute(Globals.EXCEPTION_ATTR) == null) { boolean matchFound = status(request, response, context, wrapper, statusCode); if (!matchFound) { serveDefaultErrorPage(request, response, statusCode); + String errorReportValveStr = + ((StandardHost)(context.getParent())).getErrorReportValveClass(); + if (errorReportValveStr != null) { + if (statusCode >= 400 + && request.getAttribute(Globals.EXCEPTION_ATTR) == null) Unknown macro: {+ boolean matchFound = status(request, response, context, wrapper,+ statusCode);+ if (!matchFound) { + serveDefaultErrorPage(request, response, statusCode); + } } } try { - PrintWriter writer = response.getWriter(); - writer.close(); - } catch (IllegalStateException e) { + if ((statusCode < 400) || (errorReportValveStr != null)) { try { - ServletOutputStream stream = response.getOutputStream(); - stream.close(); - } catch (IllegalStateException f) { - ; - } catch (IOException f) { + PrintWriter writer = response.getWriter(); + writer.flush(); + writer.close(); + } catch (IllegalStateException e) Unknown macro: {+ try { + ServletOutputStream stream = response.getOutputStream(); + stream.flush(); + stream.close(); + } catch (IllegalStateException f) { + ; + } catch (IOException f) { + ; + }+ } catch (IOException e) { ; } } catch (IOException e) { - ; } }
        Hide
        kmeduri added a comment -

        Committed to PWC12Dev_Branch:

        Checking in src/java/org/apache/catalina/core/ApplicationDispatcherForward.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v
        <-- ApplicationDispatcherForward.java
        new revision: 1.4.8.2; previous revision: 1.4.8.1
        done

        Show
        kmeduri added a comment - Committed to PWC12Dev_Branch: Checking in src/java/org/apache/catalina/core/ApplicationDispatcherForward.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v <-- ApplicationDispatcherForward.java new revision: 1.4.8.2; previous revision: 1.4.8.1 done
        Hide
        gfbugbridge added a comment -

        <BT6543256>

        Show
        gfbugbridge added a comment - <BT6543256>
        Hide
        jluehe added a comment -

        Reassigning to myself

        Show
        jluehe added a comment - Reassigning to myself
        Hide
        jluehe added a comment -

        Checking in ApplicationDispatcherForward.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v
        <-- ApplicationDispatcherForward.java
        new revision: 1.7; previous revision: 1.6
        done

        Show
        jluehe added a comment - Checking in ApplicationDispatcherForward.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v <-- ApplicationDispatcherForward.java new revision: 1.7; previous revision: 1.6 done
        Hide
        kmeduri added a comment -

        Committed to PWC12Dev_Branch:
        ------------------------------
        Changes:

        • Port the original fix to PWC12Dev_Branch
        • Avoid (errorReportValveClass != null) check before calling status() method.
        • Avoid committing response (for Web Server only) in case of
          response.setStatus(404)

        Checking in org/apache/catalina/core/ApplicationDispatcherForward.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v
        <-- ApplicationDispatcherForward.java
        new revision: 1.4.8.3; previous revision: 1.4.8.2
        done
        Checking in org/apache/coyote/tomcat5/CoyoteResponseFacade.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponseFacade.java,v
        <-- CoyoteResponseFacade.java
        new revision: 1.5.12.3; previous revision: 1.5.12.2
        done
        ------------------------------

        Show
        kmeduri added a comment - Committed to PWC12Dev_Branch: ------------------------------ Changes: Port the original fix to PWC12Dev_Branch Avoid (errorReportValveClass != null) check before calling status() method. Avoid committing response (for Web Server only) in case of response.setStatus(404) Checking in org/apache/catalina/core/ApplicationDispatcherForward.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/catalina/core/ApplicationDispatcherForward.java,v <-- ApplicationDispatcherForward.java new revision: 1.4.8.3; previous revision: 1.4.8.2 done Checking in org/apache/coyote/tomcat5/CoyoteResponseFacade.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponseFacade.java,v <-- CoyoteResponseFacade.java new revision: 1.5.12.3; previous revision: 1.5.12.2 done ------------------------------

          People

          • Assignee:
            jluehe
            Reporter:
            kmeduri
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: