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

Wrong value of javax.servlet.forward.context_path attribute #9119

Closed
grgrzybek opened this issue Jan 3, 2023 · 3 comments · Fixed by #9123
Closed

Wrong value of javax.servlet.forward.context_path attribute #9119

grgrzybek opened this issue Jan 3, 2023 · 3 comments · Fixed by #9123
Assignees
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)
Milestone

Comments

@grgrzybek
Copy link
Contributor

Jetty version(s)
10.0.13

Java version/vendor (use: java -version)
11.0.7

OS type/version
Linux/Fedora 37

Description
I work on Pax Web project and it unifies 3 embedded web servers (Jetty, Tomcat and Undertow) in OSGi runtime.
I try hard to make the tests consistent and ensure similar behavior between the runtimes.

According to Chapter 9.4.2 Forwarded Request Parameters of the Servlets 4 specification:

The following request attributes must be set:
javax.servlet.forward.mapping
javax.servlet.forward.request_uri
javax.servlet.forward.context_path
javax.servlet.forward.servlet_path
javax.servlet.forward.path_info
javax.servlet.forward.query_string
The values of these attributes must be equal to the return values of the
HttpServletRequest methods getRequestURI, getContextPath, getServletPath,
getPathInfo, getQueryString respectively, invoked on the request object passed to
the first servlet object in the call chain that received the request from the client.

When using Jetty 9.4.50, I see correct (in org.eclipse.jetty.server.Dispatcher#forward()):

final String old_context_path = baseRequest.getContextPath();
...
ForwardAttributes attr = new ForwardAttributes(old_attr);
...
attr._contextPath = old_context_path;

However in Jetty 10, it's:

if (old_attr.getAttribute(FORWARD_REQUEST_URI) == null)
    baseRequest.setAttributes(new ForwardAttributes(old_attr,
        old_uri.getPath(),
        old_context == null ? null : old_context.getContextHandler().getContextPathEncoded(),
        baseRequest.getPathInContext(),
        source_mapping,
        old_uri.getQuery()));

And the forwarded context path is "/" instead of "" (empty string).

How to reproduce?

I have a "/gateway/*" mapped servlet with:

protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
	String what = req.getParameter("what");
	String where = req.getParameter("where");
	switch (what) {
		case "redirect":
			resp.sendRedirect(where);
			return;
		case "forward":
			// we can't send anything when forwarding
			req.getRequestDispatcher(where).forward(req, resp);
			return;
		case "include":
			resp.getWriter().print(">>>");
			req.getRequestDispatcher(where).include(req, resp);
			resp.getWriter().print("<<<");
			return;
		default:
	}
}

And I call it with this URI:

"/gateway/x?what=forward&where=/"

The test is https://github.com/ops4j/org.ops4j.pax.web/blob/web-9.0.4/pax-web-jetty/src/test/java/org/ops4j/pax/web/service/jetty/internal/UnifiedJettyTest.java#L505-L516

@grgrzybek grgrzybek added the Bug For general bugs on Jetty side label Jan 3, 2023
@grgrzybek
Copy link
Contributor Author

The include attributes are fine - there, org.eclipse.jetty.server.handler.ContextHandler#getRequestContextPath() is used, while in forward, the method is org.eclipse.jetty.server.handler.ContextHandler#getContextPathEncoded().

@joakime joakime added the Specification For all industry Specifications (IETF / Servlet / etc) label Jan 3, 2023
@joakime
Copy link
Contributor

joakime commented Jan 3, 2023

I have a test case that confirms report.
Working on a fix and PR.

@joakime joakime self-assigned this Jan 3, 2023
@joakime joakime added this to the 10.0.x milestone Jan 3, 2023
joakime added a commit that referenced this issue Jan 3, 2023
* Fixes #9119 - uses proper context path that
  satisfies the root context rules of the servlet
  spec

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor

joakime commented Jan 3, 2023

Opened PR #9123 to address this

joakime added a commit that referenced this issue Jan 20, 2023
…root context (#9123)

* Wrong value of RequestDispatcher.FORWARD_CONTEXT_PATH on root context

* Fixes #9119 - uses proper context path that
  satisfies the root context rules of the servlet
  spec

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
grgrzybek added a commit to ops4j/org.ops4j.pax.web that referenced this issue Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants