Jetty
  1. Jetty
  2. JETTY-502

<jsp:include..> - behaviour change

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 6.1.9
    • Fix Version/s: 7.0.0pre3, 6.1.12rc1
    • Component/s: JSP
    • Labels:
      None
    • Environment:
      Windows XP / 2003
    • Number of attachments :
      4

      Description

      From my index.jsp:

      <jsp:include page="Home.do"/>

      works on jetty6.1.4, but since 6.1.5 additional attribute "flush=true" was needed:

      <jsp:include page="Home.do" flush="true"/>

      Is there any reason for that behaviour ?!

      1. dispatcher6.diff
        16 kB
        Athena Yao
      2. dispatcher-7.diff
        16 kB
        Athena Yao
      3. ServletHandler.diff
        1 kB
        Athena Yao

        Activity

        Hide
        Jan Bartel added a comment -

        The jasper implementation changed between jetty 6.1.4 and jetty 6.1.5. In jetty 6.1.4 it was tag SJSAS-9_1-B39-RC-14_Mar_2007, but upgraded to SJSAS-9_1-B50G-BETA3-27_June_2007 for jetty 6.1.5. I've had a quick look at the glassfish issue list, but couldn't find anything specific: https://glassfish.dev.java.net/servlets/ProjectIssues.

        I can't see anything offhand that may have affected this in the jetty io handling, although that's a possibility. How does Home.do do it's output?

        regards
        Jan

        Show
        Jan Bartel added a comment - The jasper implementation changed between jetty 6.1.4 and jetty 6.1.5. In jetty 6.1.4 it was tag SJSAS-9_1-B39-RC-14_Mar_2007, but upgraded to SJSAS-9_1-B50G-BETA3-27_June_2007 for jetty 6.1.5. I've had a quick look at the glassfish issue list, but couldn't find anything specific: https://glassfish.dev.java.net/servlets/ProjectIssues . I can't see anything offhand that may have affected this in the jetty io handling, although that's a possibility. How does Home.do do it's output? regards Jan
        Hide
        jose ayala added a comment -

        Jan,

        Home.do is a "struts action" mapping.
        I've been testing and some results:

        -> jetty.jar 6.1.4 and jasper*.jar 6.1.4: works fine
        -> jetty.jar 6.1.4 and jasper*.jar 6.1.7: works fine
        -> jetty.jar 6.1.7 and jasper*.jar 6.1.7: don't work
        -> jetty.jar 6.1.7 and jasper*.jar 6.1.4: don't work

        Then, when I upgrade only jetty.jar from 6.1.4 to 6.1.5 (or above) this error happens.

        When didn't work:
        -> http://localhost/index.jsp: result NOK and home_jsp.java wasn't created.
        -> http://localhost/Home.do: result OK

        Please, let me know If you need other information.

        Josť Ayala

        Show
        jose ayala added a comment - Jan, Home.do is a "struts action" mapping. I've been testing and some results: -> jetty.jar 6.1.4 and jasper*.jar 6.1.4: works fine -> jetty.jar 6.1.4 and jasper*.jar 6.1.7: works fine -> jetty.jar 6.1.7 and jasper*.jar 6.1.7: don't work -> jetty.jar 6.1.7 and jasper*.jar 6.1.4: don't work Then, when I upgrade only jetty.jar from 6.1.4 to 6.1.5 (or above) this error happens. When didn't work: -> http://localhost/index.jsp: result NOK and home_jsp.java wasn't created. -> http://localhost/Home.do: result OK Please, let me know If you need other information. Josť Ayala
        Hide
        Jean-Francois Arcand added a comment -

        Hi,

        I think I know why. Mainly, we added a new interface for Jasper to improve performance (hook directly into Grizzly instead of buffering). I will soon update the issue with a patch if my investigation is right. More to come...

        – Jeanfrancois

        Show
        Jean-Francois Arcand added a comment - Hi, I think I know why. Mainly, we added a new interface for Jasper to improve performance (hook directly into Grizzly instead of buffering). I will soon update the issue with a patch if my investigation is right. More to come... – Jeanfrancois
        Hide
        Jean-Francois Arcand added a comment -

        Hum, looks like I was wrong with my investigation. This has nothing to do with Grizzly I've tested the same test in GlassFish and it works...so something is wrong in Jetty.

        A+

        • jeanfrancois
        Show
        Jean-Francois Arcand added a comment - Hum, looks like I was wrong with my investigation. This has nothing to do with Grizzly I've tested the same test in GlassFish and it works...so something is wrong in Jetty. A+ jeanfrancois
        Hide
        Jan Bartel added a comment -

        Greg, did something change with the IO handling associated with include dispatches for jetty-6.1.5?

        cheers
        Jan

        Show
        Jan Bartel added a comment - Greg, did something change with the IO handling associated with include dispatches for jetty-6.1.5? cheers Jan
        Hide
        Jan Bartel added a comment -

        Jose,

        Is this still a problem with jetty-6..1.9? If not, I will close the issue.

        regards
        Jan

        Show
        Jan Bartel added a comment - Jose, Is this still a problem with jetty-6..1.9? If not, I will close the issue. regards Jan
        Hide
        jose ayala added a comment -

        Eclipse Project using Jetty.

        1. Open with Eclipse Europa;
        2. Change source/src/main/web.conf;
        3. Open source/src/main/Start.java;
        4. select "Run As Java Application"

        Zip file contains Jetty libraries at WebApl/WEB-INF/lib.

        There are 2 jetty core files:

        • jetty.jar.new - 6.1.9
        • jetty.jar - 6.1.4

        5. Open url localhost with 6.1.4.
        6. Stop the Run/Debug on eclipse.

        7. Change file name jetty.jar -> jetty.jar.old and
        8. Change file name jetty.jar.new -> jetty.jar

        Run again and see a blank page.

        Show
        jose ayala added a comment - Eclipse Project using Jetty. 1. Open with Eclipse Europa; 2. Change source/src/main/web.conf; 3. Open source/src/main/Start.java; 4. select "Run As Java Application" Zip file contains Jetty libraries at WebApl/WEB-INF/lib. There are 2 jetty core files: jetty.jar.new - 6.1.9 jetty.jar - 6.1.4 5. Open url localhost with 6.1.4. 6. Stop the Run/Debug on eclipse. 7. Change file name jetty.jar -> jetty.jar.old and 8. Change file name jetty.jar.new -> jetty.jar Run again and see a blank page.
        Hide
        Greg Wilkins added a comment -

        athena, can you look at this one.

        Show
        Greg Wilkins added a comment - athena, can you look at this one.
        Hide
        Athena Yao added a comment -

        The problem was introduced by r2008, http://fisheye.codehaus.org/changelog/jetty/jetty/trunk/modules/jetty/src/main/java/org/mortbay/jetty/servlet?cs=2008, which was meant to avoid "hiding forward attribute in include." Letting ForwardAttributes.getAttribute return null when the key starts with __INCLUDE_PREFIX fixes the problem, but probably reintroduces what r2008 is supposed to fix.

        I've dug around a bit, but I can't find any information about what this is supposed to fix/ what SPR-3682 is – are there any examples of this so I can check that any fix I make to resolve this ticket won't reintroduce the old problem?

        Show
        Athena Yao added a comment - The problem was introduced by r2008, http://fisheye.codehaus.org/changelog/jetty/jetty/trunk/modules/jetty/src/main/java/org/mortbay/jetty/servlet?cs=2008 , which was meant to avoid "hiding forward attribute in include." Letting ForwardAttributes.getAttribute return null when the key starts with __INCLUDE_PREFIX fixes the problem, but probably reintroduces what r2008 is supposed to fix. I've dug around a bit, but I can't find any information about what this is supposed to fix/ what SPR-3682 is – are there any examples of this so I can check that any fix I make to resolve this ticket won't reintroduce the old problem?
        Hide
        Jan Bartel added a comment -

        Hi Athena,
        I think you found some more information on this - can you document it here?
        thanks
        Jan

        Show
        Jan Bartel added a comment - Hi Athena, I think you found some more information on this - can you document it here? thanks Jan
        Hide
        Athena Yao added a comment -

        Sorry about the delay! I got caught up finishing another ticket. Mm, going to be braindumping a bit.

        Jetty used to return null when it detected any "javax.servlet.include.*" attribute in a forward. This caused problems in http://jira.springframework.org/browse/SPR-3682, where it appeared that Jetty would return wrong values for certain forward attributes (the returning of the null masked the actual attribute value).

        In the sample project submitted here, the opposite appears to be the case: returning a value for javax.servlet.include.servlet_path masks the forwarded value of the request. (The previous value of null made it fall back upon the value from request.getServletPath()).

        To wit:
        "include.servlet_path" (original): /Home.do – returned in Jetty6.1.5
        request.servletPath() (forwarded): /home.jsp – returned in Jetty6.1.4

        The simplest way for me seems to be to return null for key.equals("javax.servlet.include.servlet_path") within ForwardAttributes#getAttribute (unlike the previous code, it would only affect the include servlet_path, not all include attributes.)

        Otherwise, in ServletHandler, you could check whether the request is a forward which contains an __INCLUDE_SERVLET_PATH, and update the include.servlet_path attribute at that point.

        I've been trying to decide which of these two approaches works bette, but unfortunately, I haven't been able to figure out from the information given in SPR-3682 exactly what was happening, so I can't tell whether either of these two approaches would (re)introduce the bug described there....

        Show
        Athena Yao added a comment - Sorry about the delay! I got caught up finishing another ticket. Mm, going to be braindumping a bit. Jetty used to return null when it detected any "javax.servlet.include.*" attribute in a forward. This caused problems in http://jira.springframework.org/browse/SPR-3682 , where it appeared that Jetty would return wrong values for certain forward attributes (the returning of the null masked the actual attribute value). In the sample project submitted here, the opposite appears to be the case: returning a value for javax.servlet.include.servlet_path masks the forwarded value of the request. (The previous value of null made it fall back upon the value from request.getServletPath()). To wit: "include.servlet_path" (original): /Home.do – returned in Jetty6.1.5 request.servletPath() (forwarded): /home.jsp – returned in Jetty6.1.4 The simplest way for me seems to be to return null for key.equals("javax.servlet.include.servlet_path") within ForwardAttributes#getAttribute (unlike the previous code, it would only affect the include servlet_path, not all include attributes.) Otherwise, in ServletHandler, you could check whether the request is a forward which contains an __INCLUDE_SERVLET_PATH, and update the include.servlet_path attribute at that point. I've been trying to decide which of these two approaches works bette, but unfortunately, I haven't been able to figure out from the information given in SPR-3682 exactly what was happening, so I can't tell whether either of these two approaches would (re)introduce the bug described there....
        Hide
        Athena Yao added a comment -

        Attached is a diff that takes the second approach I mentioned in my comment. This should fix the problem that I noticed while running the test case that Jose provided. I didn't go with setting the include servlet path to null, because that could possibly affect behavior of more things, in more subtle ways.

        Jose, I did not notice any difference between setting flush="true" and not setting flush="true", at least not with index.jsp/Home.do in the test project. It is possible that the problem/fix I dug out is different from the one that you were experiencing.

        Show
        Athena Yao added a comment - Attached is a diff that takes the second approach I mentioned in my comment. This should fix the problem that I noticed while running the test case that Jose provided. I didn't go with setting the include servlet path to null, because that could possibly affect behavior of more things, in more subtle ways. Jose, I did not notice any difference between setting flush="true" and not setting flush="true", at least not with index.jsp/Home.do in the test project. It is possible that the problem/fix I dug out is different from the one that you were experiencing.
        Hide
        Greg Wilkins added a comment -

        Jan,

        can you check what athena has found out on this one.

        Show
        Greg Wilkins added a comment - Jan, can you check what athena has found out on this one.
        Hide
        Jan Bartel added a comment -

        Athena,

        Can you make a simple test webapp to verify the behaviour of include->forward, and also forward->include? Once that's done, can you turn that into a junit test for the jetty module?

        thanks
        Jan

        Show
        Jan Bartel added a comment - Athena, Can you make a simple test webapp to verify the behaviour of include->forward, and also forward->include? Once that's done, can you turn that into a junit test for the jetty module? thanks Jan
        Hide
        Athena Yao added a comment -

        Better patches against the dispatcher that take care of the core problem (include attributes should be hidden after a forward).

        Show
        Athena Yao added a comment - Better patches against the dispatcher that take care of the core problem (include attributes should be hidden after a forward).
        Hide
        Jan Bartel added a comment -

        Patches applied as rev 3191 to trunk and svn rev 3192 to jetty-6.

        Show
        Jan Bartel added a comment - Patches applied as rev 3191 to trunk and svn rev 3192 to jetty-6.

          People

          • Assignee:
            Athena Yao
            Reporter:
            jose ayala
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: