Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dispatcher.include() with welcome files lead to stack overflow error #5025

Closed
grgrzybek opened this issue Jul 4, 2020 · 7 comments
Closed
Assignees

Comments

@grgrzybek
Copy link
Contributor

Jetty version
10.0.0-SNAPSHOT

Java version
11.0.6

OS type/version
Linux everfree.forest 5.6.19-300.fc32.x86_64 #1 SMP Wed Jun 17 16:10:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Description
I'm working on Pax Web 8 and welcome file handling with default servlets.

I have org.eclipse.jetty.servlet.DefaultServlet mapped to /alt/* only (no mapping of org.eclipse.jetty.servlet.DefaultServlet to / path) with pathInfoOnly=true init parameter (because I want resources to be located using ... path info only, because this default servlet is not mapped to /) and another servlet mapped to /gateway/*.
Then context has welcome files configured.

"gateway" servlet does this:

req.getRequestDispatcher("/alt/").include(req, resp);

"default" servlet (the one mapped to /alt/* is properly called with include (has all the javax.servlet.include.* attributes, but it eventually calls org.eclipse.jetty.server.ResourceService#sendWelcome() because the /alt/ resource is a directory.
The problem is that sendWelcome() method creates welcome file /gateway/index.html instead of /alt/index.html, because it (using pathInfoOnly=true has to restore full request URI.

However, when resource servlet is included, org.eclipse.jetty.server.ResourceService#sendWelcome() should get the servlet path differently (using javax.servlet.include.servlet_path attribute).

grgrzybek added a commit to grgrzybek/jetty.project that referenced this issue Jul 4, 2020
…e() and non-default mapping

Signed-off-by: Grzegorz Grzybek <gr.grzybek@gmail.com>
grgrzybek added a commit to grgrzybek/jetty.project that referenced this issue Jul 4, 2020
…e() and non-default mapping

Signed-off-by: Grzegorz Grzybek <gr.grzybek@gmail.com>
@joakime
Copy link
Contributor

joakime commented Jul 4, 2020

Are you using ServletContextHandler? or WebAppContext? (either way, you'd be surprised, there is always some servlet mapped to / default after startup, there has to be)

@joakime
Copy link
Contributor

joakime commented Jul 4, 2020

Also, you should never have a named servlet "default" ever mapped to something that isn't the "/" default url-pattern.
And the ServletContextHandler (or WebAppContext) must have a valid ResourceBase declared, otherwise the ServletContext is essentially broken.
And you know you can have multiple DefaultServlet's declared, right? (with a named called "alt", i suspect you already read that stackoverflow answer)

grgrzybek added a commit to grgrzybek/jetty.project that referenced this issue Jul 4, 2020
…e() and non-default mapping

Signed-off-by: Grzegorz Grzybek <gr.grzybek@gmail.com>
@grgrzybek
Copy link
Contributor Author

I'm looking from the perspective of ServletContextHandler, not the WebAppContext one. I'm mostly interested in embedded Jetty usage (I'm refactoring Pax Web 8) and definitely I'm interested in multiple registrations of "default servlet" under different paths (according to OSGi CMPN Http and Whiteboard specifications).
And yes - I'm aware of org.eclipse.jetty.servlet.ServletHandler#_ensureDefaultServlet. I don't want to distinguish any particular resource servlet and also (for OSGi purposes) I'm doing some tricks related to "resource base" for the resource servlets.

@grgrzybek
Copy link
Contributor Author

I wanted to thank you first for responding ;)
I'm not really sure I need "default" servlet. Even 12.1 "Use of URL Paths" of Servlet (4.0) spec says:

If neither of the previous three rules result in a servlet match, the container will attempt to serve content appropriate for the resource requested. If a "default" servlet is defined for the application, it will be used. Many containers provide an implicit default servlet for serving content

Which means (IMO) that "default" servlet is not a requirement.

OSGi CMPN Http Service specification defines org.osgi.service.http.HttpService#registerResources() method, where some "resource base" (by default - root of the Bundle) is mounted under an alias (which is mapped to /alias/* URL pattern of Servlet spec).

FYI, here's my unit test where I try to check all the combinations of parameters, redirects, forwards, includes and welcome file scenarios.

@grgrzybek
Copy link
Contributor Author

grgrzybek commented Jul 4, 2020

BTW, Jetty again seems the most flexible implementation ;)

In Tomcat, welcome files are handled in org.apache.catalina.mapper.Mapper#internalMapWrapper() as "rule 4" - after checking exact, prefix and extension mappings. So if a "resource servlet" is mapped to e.g., /res/*, we won't get welcome file handling with GET /res/ HTTP/1.1 request.

In Undertow, welcome files are handled by io.undertow.servlet.handlers.ServletInitialHandler#handleRequest() using io.undertow.servlet.handlers.ServletPathMatches#getServletHandlerByPath() and as in Tomcat, if "resource servlet" (io.undertow.servlet.handlers.DefaultServlet) is mapped to e.g., prefix URI pattern, it won't leverage context-wide welcome files.

In Jetty, except this include-stack-overflow problem, I had no problems mapping different org.eclipse.jetty.servlet.DefaultServlet instances to different URIs (with or without / default mapping).

@janbartel
Copy link
Contributor

@grgrzybek I've reproduced the problem, am pondering the solution.

@janbartel janbartel self-assigned this Jul 6, 2020
grgrzybek added a commit to grgrzybek/jetty.project that referenced this issue Jul 10, 2020
…e() and non-default mapping

Signed-off-by: Grzegorz Grzybek <gr.grzybek@gmail.com>
grgrzybek added a commit to grgrzybek/jetty.project that referenced this issue Jul 10, 2020
…e() and non-default mapping

Signed-off-by: Grzegorz Grzybek <gr.grzybek@gmail.com>
janbartel pushed a commit that referenced this issue Jul 15, 2020
…nd non-default mapping (#5026)

Signed-off-by: Grzegorz Grzybek <gr.grzybek@gmail.com>
@janbartel
Copy link
Contributor

Applied the PR provided by @grgrzybek - thanks!

janbartel added a commit that referenced this issue Jul 16, 2020
…nd non-default mapping (#5026)

Signed-off-by: Grzegorz Grzybek <gr.grzybek@gmail.com>
Signed-off-by: Jan Bartel <janb@webtide.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants