Click
  1. Click
  2. CLK-323

CompressionServletResponseWrapper throws NPE in Jetty

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.4
    • Component/s: None
    • Labels:
      None
    • Environment:
      Jetty 5.1.4, Websphere 6

      Description

      Ricardo reported a compression issue while using PerformanceFilter in Jetty 5.1.4.

      Here is the stack trace:

      java.lang.NullPointerException
      at net.sf.click.extras.filter.CompressionServletResponseWrapper.flushBuffer(CompressionServletResponseWrapper.java:149)
      at org.mortbay.jetty.servlet.Default.passConditionalHeaders(Default.java:501)
      at org.mortbay.jetty.servlet.Default.handleGet(Default.java:327)
      at org.mortbay.jetty.servlet.Default.service(Default.java:232)
      at javax.servlet.http.HttpServlet.service(HttpServlet.java:689)
      at org.mortbay.jetty.servlet.ServletHolder.handle(ServletHolder.java:427)
      at org.mortbay.jetty.servlet.WebApplicationHandler$CachedChain.doFilter(WebApplicationHandler.java:832)
      at net.sf.click.extras.filter.PerformanceFilter.doFilter(PerformanceFilter.java:378)

      Source info:

      #1 http://www.servlets.com/archive/servlet/ReadMsg?msgId=498901&listName=jetty-support
      #2 http://www.servlets.com/archive/servlet/ReadMsg?msgId=196252&listName=tomcat-dev

      it sounds like the problem comes from a null stream when the serving a 304 (not modified) response.

      The following change needs to be applied to CompressionServletResponseWrapper#flushBuffer:

      public void flushBuffer() throws IOException {
      if (writer != null)

      { writer.flush(); }

      else if (stream != null)

      { ((CompressionResponseStream) stream).flush(); }

      }

      We should also flush the writer according to #2

        Activity

        Hide
        Ricardo R. Lecheta added a comment -

        I need test it with WebSphere 5 and 6 I use Jetty just for development

        Show
        Ricardo R. Lecheta added a comment - I need test it with WebSphere 5 and 6 I use Jetty just for development
        Hide
        Malcolm Edgar added a comment -

        These bugs must have been lurking the the Apache CompressionServletResponseWrapper for a long time.

        I get a WebLogic environment setup as test against this as well. I think with all the testing you have done this will cover 95% of the app serer market.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - These bugs must have been lurking the the Apache CompressionServletResponseWrapper for a long time. I get a WebLogic environment setup as test against this as well. I think with all the testing you have done this will cover 95% of the app serer market. regards Malcolm Edgar
        Hide
        Bob Schellink added a comment -

        I received the NPE in Websphere 6 and Jetty 5 / 6 consistently.

        It should only occur on 304 (Not-Modified) requests where I assume the stream is never instantiated by these containers.

        I tested now with Websphere6, Jetty 5.1.4 / 5.1.12 / 6.1.5, Tomcat 5.5.23 / 6.0.13.

        I don't see NPE or content-length problems anymore.

        If anybody still have problems let us know.

        Show
        Bob Schellink added a comment - I received the NPE in Websphere 6 and Jetty 5 / 6 consistently. It should only occur on 304 (Not-Modified) requests where I assume the stream is never instantiated by these containers. I tested now with Websphere6, Jetty 5.1.4 / 5.1.12 / 6.1.5, Tomcat 5.5.23 / 6.0.13. I don't see NPE or content-length problems anymore. If anybody still have problems let us know.
        Hide
        Ricardo R. Lecheta added a comment -

        Hi,

        >>do you mean you could not re-produce withe current patch or with the previous code.

        I could not even re-produce the NPE exception in the previous code. I tried many times.

        I thought that was because the DirectPage demo, but I accesss it many times and it worked without errors.

        Show
        Ricardo R. Lecheta added a comment - Hi, >>do you mean you could not re-produce withe current patch or with the previous code. I could not even re-produce the NPE exception in the previous code. I tried many times. I thought that was because the DirectPage demo, but I accesss it many times and it worked without errors.
        Hide
        Bob Schellink added a comment -

        Fix check into trunk. Needs more testing

        Show
        Bob Schellink added a comment - Fix check into trunk. Needs more testing
        Hide
        Bob Schellink added a comment -

        I was only testing with Jetty 5.1.12 and Jetty 6.1.4. Jetty 5.1.4 (which Ricardo uses) gives the context-length problem.

        Here are more light on the subject: http://www.servlets.com/archive/servlet/ReadMsg?msgId=496822&raw=true&listName=jetty-support

        Need to patch CompressionResponseServletWrapper some more:

        public void setIntHeader(String header, int value) {
        if (!"Content-Length".equals(header))

        { super.setIntHeader(header, value); }

        }

        public void setHeader(String header, String value) {
        if (!"Content-Length".equals(header))

        { super.setHeader(header, value); }

        }

        I have not tested this with other containers but at least 5.1.4 seems happy.

        Show
        Bob Schellink added a comment - I was only testing with Jetty 5.1.12 and Jetty 6.1.4. Jetty 5.1.4 (which Ricardo uses) gives the context-length problem. Here are more light on the subject: http://www.servlets.com/archive/servlet/ReadMsg?msgId=496822&raw=true&listName=jetty-support Need to patch CompressionResponseServletWrapper some more: public void setIntHeader(String header, int value) { if (!"Content-Length".equals(header)) { super.setIntHeader(header, value); } } public void setHeader(String header, String value) { if (!"Content-Length".equals(header)) { super.setHeader(header, value); } } I have not tested this with other containers but at least 5.1.4 seems happy.
        Hide
        Malcolm Edgar added a comment -

        Hi Ricardo,

        do you mean you could not re-produce withe current patch or with the previous code.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Hi Ricardo, do you mean you could not re-produce withe current patch or with the previous code. regards Malcolm Edgar
        Hide
        Ricardo R. Lecheta added a comment -

        Hi,

        I could not reproduce the NPE anymore. It's strange because I access the application a lot of times, but the flushBuffer method wasn't called.

        The warnings about the Invalid length of the css and js stuff are still there, and sometimes the page is not rendered properly

        Show
        Ricardo R. Lecheta added a comment - Hi, I could not reproduce the NPE anymore. It's strange because I access the application a lot of times, but the flushBuffer method wasn't called. The warnings about the Invalid length of the css and js stuff are still there, and sometimes the page is not rendered properly
        Hide
        Malcolm Edgar added a comment -

        OK I have applied your fix to SVN.

        public void flushBuffer() throws IOException {
        if (writer != null)

        { writer.flush(); }

        else if (stream != null)

        { stream.flush(); }

        }

        Richardo could you please test this patch.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - OK I have applied your fix to SVN. public void flushBuffer() throws IOException { if (writer != null) { writer.flush(); } else if (stream != null) { stream.flush(); } } Richardo could you please test this patch. regards Malcolm Edgar
        Hide
        Malcolm Edgar added a comment -

        Good research, please apply this change.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Good research, please apply this change. regards Malcolm Edgar

          People

          • Assignee:
            Malcolm Edgar
            Reporter:
            Bob Schellink
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development