Click
  1. Click
  2. CLK-557

PerformanceFilter applied to JSP pages does not always return gzip response header

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.1.0
    • Component/s: None
    • Labels:
      None

      Description

      When including JSP pages it seems there are conditions under which the gzip header is not returned to the browser. Thus the raw bytes are displayed.

      More strange is that the issue arise only when the size of the Page exceeds a certain threshold. This threshold does not seem to be related to the PerformanceFilter Gzip threshold of 384 bytes though.

        Activity

        Bob Schellink created issue -
        Hide
        Bob Schellink added a comment -

        Turns out to be Acegi related. If I comment the filter out things go back to normal. Will try and figure out what Acegi is doing because it seems to remove the Content-Encoding header.

        We should probably use Spring security instead as that seems to be the recommended library?

        Show
        Bob Schellink added a comment - Turns out to be Acegi related. If I comment the filter out things go back to normal. Will try and figure out what Acegi is doing because it seems to remove the Content-Encoding header. We should probably use Spring security instead as that seems to be the recommended library?
        Hide
        Malcolm Edgar added a comment -

        Reviewing the changes in Spring Security

        http://jira.springframework.org/browse/SEC?report=com.atlassian.jira.plugin.system.project:changelog-panel

        There appears to be no content encoding related fixes

        Show
        Malcolm Edgar added a comment - Reviewing the changes in Spring Security http://jira.springframework.org/browse/SEC?report=com.atlassian.jira.plugin.system.project:changelog-panel There appears to be no content encoding related fixes
        Hide
        Malcolm Edgar added a comment -

        Have checked in a work around in Click Examples to only filter the ACEGI examples. I haven't tried the Spring Security yet.

        Show
        Malcolm Edgar added a comment - Have checked in a work around in Click Examples to only filter the ACEGI examples. I haven't tried the Spring Security yet.
        Hide
        Bob Schellink added a comment - - edited

        I have converted to Spring Security locally (not checked in yet), however the problem persists. Scanning through the Spring Security source code provides no clue as to what might be happening.

        For example I've added the following snippet to CompressionResponseStream:

        public void writeToGZip(byte b[], int off, int len) throws IOException {
        if (gzipstream == null) {
        response.addHeader("Content-Encoding", "gzip");
        System.out.println("Contains encoding? " + response.containsHeader("Content-Encoding"));

        This code simply prints whether the response contains the header that was added in the previous line.
        For JSP pages that extends BorderPage the value is false.

        I cannot find a trace in Spring Security where they override or swallow this method. Unless they use a Proxy to intercept method invocation.

        Regarding Spring Security vs Acegi, it is mostly the same except for the configuration which, to me, looks much simpler.

        Any objections on switching to Spring Security?

        Show
        Bob Schellink added a comment - - edited I have converted to Spring Security locally (not checked in yet), however the problem persists. Scanning through the Spring Security source code provides no clue as to what might be happening. For example I've added the following snippet to CompressionResponseStream: public void writeToGZip(byte b[], int off, int len) throws IOException { if (gzipstream == null) { response.addHeader("Content-Encoding", "gzip"); System.out.println("Contains encoding? " + response.containsHeader("Content-Encoding")); This code simply prints whether the response contains the header that was added in the previous line. For JSP pages that extends BorderPage the value is false. I cannot find a trace in Spring Security where they override or swallow this method. Unless they use a Proxy to intercept method invocation. Regarding Spring Security vs Acegi, it is mostly the same except for the configuration which, to me, looks much simpler. Any objections on switching to Spring Security?
        Hide
        Malcolm Edgar added a comment -

        No objections to Spring Security, better to be on this version.

        I think we should probably document this issue in the PerformanceFilter Javadoc

        Show
        Malcolm Edgar added a comment - No objections to Spring Security, better to be on this version. I think we should probably document this issue in the PerformanceFilter Javadoc
        Hide
        Bob Schellink added a comment -

        Turns out that Spring Security (Acegi) does not interfere with setting headers but it could be a catalyst.

        According to the JSP spec section 5.4, when a JSP fragment is included through <jsp:include>, no headers can be set on the HttpServletResponse which is associated with that fragment. From what I can tell Spring Security triggers a flush to the underlying Writer while the fragment is being parsed. This in turn forces the GZIPOutputStream to be created and the Content-Encoding header to be set. But because this occurs within the fragment, no header is set and compressed bytes are displayed by the browser as is.

        To fix it we can change the border-template.js include statement from:

        <jsp:include page='$

        {forward}'/>
        to
        <jsp:include page='${forward}

        ' flush="true"/>

        The extra flush statement forces a flush "before" the include occurs while the HttpServletResponse header can still be set.

        I'm also making some changes to CompressionStream that if a header cannot be set, no compression should occur. Thus if users forget to flush before the include, the Page will still render correctly, albeit without being compressed.

        Lastly we should probably set the "Vary" header tag as well. See here for details: http://wiki.squid-cache.org/KnowledgeBase/VaryNotCaching

        Show
        Bob Schellink added a comment - Turns out that Spring Security (Acegi) does not interfere with setting headers but it could be a catalyst. According to the JSP spec section 5.4, when a JSP fragment is included through <jsp:include>, no headers can be set on the HttpServletResponse which is associated with that fragment. From what I can tell Spring Security triggers a flush to the underlying Writer while the fragment is being parsed. This in turn forces the GZIPOutputStream to be created and the Content-Encoding header to be set. But because this occurs within the fragment, no header is set and compressed bytes are displayed by the browser as is. To fix it we can change the border-template.js include statement from: <jsp:include page='$ {forward}'/> to <jsp:include page='${forward} ' flush="true"/> The extra flush statement forces a flush "before" the include occurs while the HttpServletResponse header can still be set. I'm also making some changes to CompressionStream that if a header cannot be set, no compression should occur. Thus if users forget to flush before the include, the Page will still render correctly, albeit without being compressed. Lastly we should probably set the "Vary" header tag as well. See here for details: http://wiki.squid-cache.org/KnowledgeBase/VaryNotCaching
        Hide
        Bob Schellink added a comment -

        Fix checked in. This also fixes a problem reported by Florin where PerformanceFilter can throw exceptions if the underlying response stream is closed more than once.

        Show
        Bob Schellink added a comment - Fix checked in. This also fixes a problem reported by Florin where PerformanceFilter can throw exceptions if the underlying response stream is closed more than once.
        Bob Schellink made changes -
        Field Original Value New Value
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 2.1.0 [ 12313730 ]
        Resolution Fixed [ 1 ]
        Hide
        Adrian A. added a comment -

        closing issue fixed a long time ago.

        Show
        Adrian A. added a comment - closing issue fixed a long time ago.
        Adrian A. made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development