Jetty
  1. Jetty
  2. JETTY-936

DefaultServlet doesn't dispatch to welcome files that are servlets rather than static resources

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.1.14
    • Fix Version/s: 6.1.17
    • Component/s: Servlet
    • Labels:
      None
    • Number of attachments :
      1

      Description

      DefaultServlet seems to handle everything about welcome files. However it only checks if the path appended with the welcome file name is a static resource, not if its a handled servlet mapping.

      I discovered this when I compiled all the jsps in an app, including index.jsp, and deleted the source jsps..... the welcome files stopped working.

      One possible solution would be to check the pathMap in ServletHandler for the appended path and dispatch to it if present. However I'm not sure how to get to the ServletHandler from the DefaultServlet. Maybe there's a better way?

      I imagine this is a problem in 7 also but haven't checked.

        Activity

        Hide
        David Yu added a comment -

        See http://docs.codehaus.org/display/JETTY/Welcome+files+not+working
        I did not see anything in the servlet-api that a welcome-file should map to a servlet-path.
        It denotes that the welcome-file need to be an existing file.

        <welcome-path> could be proposed for servlet-3.x though.

        Cheers

        Show
        David Yu added a comment - See http://docs.codehaus.org/display/JETTY/Welcome+files+not+working I did not see anything in the servlet-api that a welcome-file should map to a servlet-path. It denotes that the welcome-file need to be an existing file. <welcome-path> could be proposed for servlet-3.x though. Cheers
        Hide
        David Jencks added a comment -

        I checked the spec before I opened the issue. Servlet 3.0 pr says in section 10.10 4th paragraph:

        The Web server must
        append each welcome file in the order specified in the deployment descriptor to the
        partial request and check whether a static resource or servlet in the WAR is mapped
        to that request URI.

        note the "or servlet" which makes it really clear that without any extra configuration if you have a servlet mapping for the welcome "file" location it should get used.

        The same text appears in servlet 2.4 fr in section 9.10.

        BTW tomcat does not have this problem.

        Show
        David Jencks added a comment - I checked the spec before I opened the issue. Servlet 3.0 pr says in section 10.10 4th paragraph: The Web server must append each welcome file in the order specified in the deployment descriptor to the partial request and check whether a static resource or servlet in the WAR is mapped to that request URI. note the "or servlet" which makes it really clear that without any extra configuration if you have a servlet mapping for the welcome "file" location it should get used. The same text appears in servlet 2.4 fr in section 9.10. BTW tomcat does not have this problem.
        Hide
        Greg Wilkins added a comment -

        This is one of the most stupid things they added in the sevlet spec!

        The problem with "matching" servlets is that if you set your welcome files to be:

        index.jsp
        index.html

        So if there is no index.jsp file, but an index.html file, then the first will always match the *.jsp pattern
        of the JSP servlet and will be dispatched to the JSP servlet, which will then 404 error because there
        is no index.jsp. Thus the index.html will not be served as it correctly should be.

        I've been fighting this for years now... so I guess I'm going to have to give up and let welcome FILES
        be activated even when there is no FILE!

        Argh the insanity of it all.

        I think we should add an option to the default servlet to support sane welcome files vs insane welcome
        things that could be files but might not be.

        Show
        Greg Wilkins added a comment - This is one of the most stupid things they added in the sevlet spec! The problem with "matching" servlets is that if you set your welcome files to be: index.jsp index.html So if there is no index.jsp file, but an index.html file, then the first will always match the *.jsp pattern of the JSP servlet and will be dispatched to the JSP servlet, which will then 404 error because there is no index.jsp. Thus the index.html will not be served as it correctly should be. I've been fighting this for years now... so I guess I'm going to have to give up and let welcome FILES be activated even when there is no FILE! Argh the insanity of it all. I think we should add an option to the default servlet to support sane welcome files vs insane welcome things that could be files but might not be.
        Hide
        David Yu added a comment -

        Yep. It will always map to index.jsp, you will always get 404. (jsp is preconfigured by default in webdefault.xml)
        And this will cause issues for people who expect an index of the files (dirAllowed is true by default) but getting a 404 page instead.

        Cheers

        Show
        David Yu added a comment - Yep. It will always map to index.jsp, you will always get 404. (jsp is preconfigured by default in webdefault.xml) And this will cause issues for people who expect an index of the files (dirAllowed is true by default) but getting a 404 page instead. Cheers
        Hide
        Athena Yao added a comment -

        Here's a patch that looks up servlet mappings, but only if enabled in the default servlet, and only if there are no matching static files. It will behave the same as before, by default, but will switch to the new behavior if you make welcomeServlets true.

        Show
        Athena Yao added a comment - Here's a patch that looks up servlet mappings, but only if enabled in the default servlet, and only if there are no matching static files. It will behave the same as before, by default, but will switch to the new behavior if you make welcomeServlets true.
        Hide
        Brian Smith added a comment -

        As David said, the servlet specification has mandated this behavior since 2.4, so it is not a new requirement.

        This behavior is required in many applications: the application needs to serve the home page with a servlet, but it wants to use the default mechanism for serving static resources under the same domain. If the user maps the servlet to "/" then it becomes the default servlet and the static resources will not be served by default any longer. This is exactly the case in my application, which is how I stumbled upon this bug report.

        The specification says "...whether a static resource or servlet in the WAR is mapped to that request URI..." The default mapping for *.jsp is not in the WAR, and neither is the default list of welcome files. Jetty needs to search for welcome-files using the specification mechanism first, and then fall back to the defaults from webdefaults.xml. In other words, Jetty doesn't need to search for servlets that match the welcome-file entries from webdefaults.xml since those defaults are an implementation detail of "If no matching welcome file is found in the manner described, the container may handle the request in a manner it finds suitable."

        Athena, the main problem with your patch is that it causes non-compliant behavior to be the default. The defaults should always comply with the specification. But, it looks to me that changing the default setting of welcomeServlets to true leads to the unwanted and non-compliant behavior described by David and Greg. So, a more complex solution is needed.

        In the meantime, the easiest way to work aronud this is to make a jsp (e.g. index.jsp) that forwards and/or redirects to the servlet, and make that index.jsp the first welcome-file.

        Show
        Brian Smith added a comment - As David said, the servlet specification has mandated this behavior since 2.4, so it is not a new requirement. This behavior is required in many applications: the application needs to serve the home page with a servlet, but it wants to use the default mechanism for serving static resources under the same domain. If the user maps the servlet to "/" then it becomes the default servlet and the static resources will not be served by default any longer. This is exactly the case in my application, which is how I stumbled upon this bug report. The specification says "...whether a static resource or servlet in the WAR is mapped to that request URI..." The default mapping for *.jsp is not in the WAR , and neither is the default list of welcome files. Jetty needs to search for welcome-files using the specification mechanism first, and then fall back to the defaults from webdefaults.xml. In other words, Jetty doesn't need to search for servlets that match the welcome-file entries from webdefaults.xml since those defaults are an implementation detail of "If no matching welcome file is found in the manner described, the container may handle the request in a manner it finds suitable." Athena, the main problem with your patch is that it causes non-compliant behavior to be the default. The defaults should always comply with the specification. But, it looks to me that changing the default setting of welcomeServlets to true leads to the unwanted and non-compliant behavior described by David and Greg. So, a more complex solution is needed. In the meantime, the easiest way to work aronud this is to make a jsp (e.g. index.jsp) that forwards and/or redirects to the servlet, and make that index.jsp the first welcome-file.
        Hide
        Jan Bartel added a comment -

        Assigning to Greg for a comment.

        Show
        Jan Bartel added a comment - Assigning to Greg for a comment.
        Hide
        Greg Wilkins added a comment -

        I've raised this again with the servlet EG and there is general agreement that the wording is bad.

        It appears that other containers implement the following approach: scan the entire welcome list for matching files
        and only if no matching files are file then scan the list looking for matching servlet mappings.

        So I think we should follow suite.

        Athena, can you do a patch for this behaviour for jetty 6 and jetty 7

        Show
        Greg Wilkins added a comment - I've raised this again with the servlet EG and there is general agreement that the wording is bad. It appears that other containers implement the following approach: scan the entire welcome list for matching files and only if no matching files are file then scan the list looking for matching servlet mappings. So I think we should follow suite. Athena, can you do a patch for this behaviour for jetty 6 and jetty 7
        Hide
        Greg Wilkins added a comment -

        Athena, the patch is good, but can you add an init parameter to turn off matching to servlets without a matching file.

        Currently with this patch if index.jsp is in the welcome file list, we never get a directory listing because there is always a JSP servlet.

        Show
        Greg Wilkins added a comment - Athena, the patch is good, but can you add an init parameter to turn off matching to servlets without a matching file. Currently with this patch if index.jsp is in the welcome file list, we never get a directory listing because there is always a JSP servlet.
        Hide
        Athena Yao added a comment -

        Done for Jetty 6. Still needs porting to Jetty 7 https://bugs.eclipse.org/bugs/show_bug.cgi?id=274251

        Show
        Athena Yao added a comment - Done for Jetty 6. Still needs porting to Jetty 7 https://bugs.eclipse.org/bugs/show_bug.cgi?id=274251
        Hide
        David Jencks added a comment -

        Changes were ported to jetty7 in rev 240.

        Show
        David Jencks added a comment - Changes were ported to jetty7 in rev 240.
        Hide
        Athena Yao added a comment -

        Thanks, David. I forgot to update this bug when I resolved the other one

        Show
        Athena Yao added a comment - Thanks, David. I forgot to update this bug when I resolved the other one
        Hide
        David Jencks added a comment -

        I don't think this is an adequate solution. If there is an exact match to a servlet mapping, that should be used, you shouldn't need a flag to get this. The flag should only apply for *.jsp type servlet mappings.

        Show
        David Jencks added a comment - I don't think this is an adequate solution. If there is an exact match to a servlet mapping, that should be used, you shouldn't need a flag to get this. The flag should only apply for *.jsp type servlet mappings.
        Hide
        Jan Bartel added a comment -

        David,

        I can see the motivation for your proposed solution, but it looks a bit hacky to me.

        Greg, can you please comment?

        regards
        Jan

        Show
        Jan Bartel added a comment - David, I can see the motivation for your proposed solution, but it looks a bit hacky to me. Greg, can you please comment? regards Jan
        Hide
        Greg Wilkins added a comment -

        David,

        Jetty-6 has always required that the welcome FILE exists before triggering. With the addition of the welcomeServlets flag, we can now match welcome servlets even if the file does not exist.

        I think having the flag apply to servlets as a whole makes a lot more sense rather than to suffix matching or to a specific *.jsp pattern. It means the flag is a switch between traditional Jetty behaviour and the vagueness of the servlet spec behaviour.

        I really think the only debate should be what is the default value for this flag. My preference is for jetty-6, the default is welcomeServlets=false, but for jetty-7 onwards we change to welcomeServlets=true as the default?

        Show
        Greg Wilkins added a comment - David, Jetty-6 has always required that the welcome FILE exists before triggering. With the addition of the welcomeServlets flag, we can now match welcome servlets even if the file does not exist. I think having the flag apply to servlets as a whole makes a lot more sense rather than to suffix matching or to a specific *.jsp pattern. It means the flag is a switch between traditional Jetty behaviour and the vagueness of the servlet spec behaviour. I really think the only debate should be what is the default value for this flag. My preference is for jetty-6, the default is welcomeServlets=false, but for jetty-7 onwards we change to welcomeServlets=true as the default?
        Hide
        Greg Wilkins added a comment -

        Actually there is a definite bug with this.

        The servlet matching should be done on the complete path in context and not just the welcome fragment.

        Show
        Greg Wilkins added a comment - Actually there is a definite bug with this. The servlet matching should be done on the complete path in context and not just the welcome fragment.
        Hide
        Greg Wilkins added a comment -

        r5522 fixes the servlet matching problem, adds a test harness and optimized so that:
        + servlet handler was only looked up once
        + servlet entries are scoped with path in context
        + servlet entries are checked that they are not the default servlet
        + only a single iteration through the welcome files - so path add done only once per pattern
        + the path in context is returned, so the caller does not need to add the context path again.

        Show
        Greg Wilkins added a comment - r5522 fixes the servlet matching problem, adds a test harness and optimized so that: + servlet handler was only looked up once + servlet entries are scoped with path in context + servlet entries are checked that they are not the default servlet + only a single iteration through the welcome files - so path add done only once per pattern + the path in context is returned, so the caller does not need to add the context path again.

          People

          • Assignee:
            Greg Wilkins
            Reporter:
            David Jencks
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: