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.

Sign up for GitHub

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

Jump to bottom

Issue #11494 - PathMappingsHandler exposes PathSpec and Context based on PathSpec. #11497

Open
wants to merge 4 commits into
base: jetty-12.0.x
Choose a base branch
from
Open

Issue #11494 - PathMappingsHandler exposes PathSpec and Context based on PathSpec. #11497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6966d73
Issue #11494 - PathMappingsHandler exposes PathSpec and Context based…
joakime Mar 7, 2024
946070f
Adding RegexPathSpec example on PathMappingsHandler
joakime Mar 11, 2024
b373bd3
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/pa…
joakime Apr 4, 2024
4c512a5
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/pa…
joakime Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
107 changes: 107 additions & 0 deletions jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Context.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.io.File;
import java.util.List;
import java.util.Set;
import java.util.concurrent.Executor;

import org.eclipse.jetty.http.MimeTypes;
Expand Down Expand Up @@ -165,4 +166,110 @@ static String getPathInContext(String encodedContextPath, String encodedPath)
return null;
return encodedPath.substring(encodedContextPath.length());
}

public static class Wrapper implements Context
{
private final Context _wrapped;

public Wrapper(Context context)
{
_wrapped = context;
}

@Override
public <T> T decorate(T o)
{
return _wrapped.decorate(o);
}

@Override
public void destroy(Object o)
{
_wrapped.destroy(o);
}

@Override
public String getContextPath()
{
return _wrapped.getContextPath();
}

@Override
public ClassLoader getClassLoader()
{
return _wrapped.getClassLoader();
}

@Override
public Resource getBaseResource()
{
return _wrapped.getBaseResource();
}

@Override
public Request.Handler getErrorHandler()
{
return _wrapped.getErrorHandler();
}

@Override
public List<String> getVirtualHosts()
{
return _wrapped.getVirtualHosts();
}

@Override
public MimeTypes getMimeTypes()
{
return _wrapped.getMimeTypes();
}

@Override
public void execute(Runnable task)
{
_wrapped.execute(task);
}

@Override
public Object removeAttribute(String name)
{
return _wrapped.removeAttribute(name);
}

@Override
public Object setAttribute(String name, Object attribute)
{
return _wrapped.setAttribute(name, attribute);
}

@Override
public Object getAttribute(String name)
{
return _wrapped.getAttribute(name);
}

@Override
public Set<String> getAttributeNameSet()
{
return _wrapped.getAttributeNameSet();
}

@Override
public void run(Runnable task)
{
_wrapped.run(task);
}

@Override
public void run(Runnable task, Request request)
{
_wrapped.run(task, request);
}

@Override
public File getTempDirectory()
{
return _wrapped.getTempDirectory();
}
}
}
42 changes: 41 additions & 1 deletion ...core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/PathMappingsHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
import java.util.Objects;

import org.eclipse.jetty.http.pathmap.MappedResource;
import org.eclipse.jetty.http.pathmap.MatchedPath;
import org.eclipse.jetty.http.pathmap.MatchedResource;
import org.eclipse.jetty.http.pathmap.PathMappings;
import org.eclipse.jetty.http.pathmap.PathSpec;
import org.eclipse.jetty.server.Context;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
Expand Down Expand Up @@ -111,11 +113,49 @@ public boolean handle(Request request, Response response, Callback callback) thr
return false;
}
Handler handler = matchedResource.getResource();
PathSpec pathSpec = matchedResource.getPathSpec();
if (LOG.isDebugEnabled())
LOG.debug("Matched {} to {} -> {}", pathInContext, matchedResource.getPathSpec(), handler);
boolean handled = handler.handle(request, response, callback);

PathSpecRequest pathSpecRequest = new PathSpecRequest(request, pathSpec);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the contextualization of a request make sense for non prefix matches? Perhaps this is better in a subclass that only allows prefix patterns?

boolean handled = handler.handle(pathSpecRequest, response, callback);
Comment on lines +120 to +121
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this optional? Perhaps not all usages will want the context set... specially if the handler mapped is itself a ContextHandler. Or maybe this is in a PathMappingsHandler.Contextual subclass?

if (LOG.isDebugEnabled())
LOG.debug("Handled {} {} by {}", handled, pathInContext, handler);
return handled;
}

private static class PathSpecRequest extends Request.Wrapper
{
private final PathSpec pathSpec;
private final Context context;
private final MatchedPath matchedPath;

public PathSpecRequest(Request request, PathSpec pathSpec)
{
super(request);
this.pathSpec = pathSpec;
matchedPath = pathSpec.matched(request.getHttpURI().getCanonicalPath());
setAttribute(PathSpec.class.getName(), this.pathSpec);
this.context = new Context.Wrapper(request.getContext())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to reuse this wrapper for every mapping to the same context. ... but maybe don't do this yet... just keep it in mind whilst we work out the other details.

{
@Override
public String getContextPath()
{
return matchedPath.getPathMatch();
}

@Override
public String getPathInContext(String canonicallyEncodedPath)
{
return matchedPath.getPathInfo();
}
};
}

@Override
public Context getContext()
{
return context;
}
}
}
99 changes: 96 additions & 3 deletions .../jetty-server/src/test/java/org/eclipse/jetty/server/handler/PathMappingsHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@

package org.eclipse.jetty.server.handler;

import java.nio.charset.StandardCharsets;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;
Expand All @@ -22,13 +24,15 @@
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.pathmap.PathSpec;
import org.eclipse.jetty.http.pathmap.RegexPathSpec;
import org.eclipse.jetty.http.pathmap.ServletPathSpec;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.component.LifeCycle;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -178,6 +182,58 @@ public void testSeveralMappingAndNoWrapper(String requestPath, int expectedStatu
assertEquals(expectedResponseBody, response.getContent());
}

public static Stream<Arguments> pathInContextInput()
{
return Stream.of(
Arguments.of("/", "/", "null", ServletPathSpec.class.getSimpleName(), "/"),
Arguments.of("/foo/test", "/foo", "/test", ServletPathSpec.class.getSimpleName(), "/foo/*"),
Arguments.of("/index.html", "/index.html", "null", ServletPathSpec.class.getSimpleName(), "/index.html"),
Arguments.of("/does-not-exist", "/does-not-exist", "null", ServletPathSpec.class.getSimpleName(), "/"),
Arguments.of("/deep/path/foo.php", "/deep/path/foo.php", "null", ServletPathSpec.class.getSimpleName(), "*.php"),
Arguments.of("/re/1234/baz", "/re/1234/baz", "null", ServletPathSpec.class.getSimpleName(), "/"),
Arguments.of("/re/ABC/baz", "/re/ABC/baz", "null", RegexPathSpec.class.getSimpleName(), "/re/[A-Z]*/.*"),
Arguments.of("/rest/api/users/ver-1/groupfoo/baruser", "api/users", "groupfoo/baruser", RegexPathSpec.class.getSimpleName(), "^/rest/(?<name>.*)/ver-[0-9]+/(?<info>.*)$"),
Arguments.of("/zed/test.txt", "/zed", "/test.txt", null, null)
);
}

@ParameterizedTest
@MethodSource("pathInContextInput")
public void testPathContextResolution(String requestPath, String expectedContextPath, String expectedPathInContext,
String expectedPathSpecImpl, String expectedPathSpecDeclaration) throws Exception
{
ContextHandler contextHandler = new ContextHandler();
contextHandler.setContextPath("/");

PathMappingsHandler pathMappingsHandler = new PathMappingsHandler();
pathMappingsHandler.addMapping(new ServletPathSpec("/"), new ContextDumpHandler());
pathMappingsHandler.addMapping(new ServletPathSpec("/index.html"), new ContextDumpHandler());
pathMappingsHandler.addMapping(new ServletPathSpec("/foo/*"), new ContextDumpHandler());
pathMappingsHandler.addMapping(new ServletPathSpec("*.php"), new ContextDumpHandler());
pathMappingsHandler.addMapping(new RegexPathSpec("/re/[A-Z]*/.*"), new ContextDumpHandler());
pathMappingsHandler.addMapping(new RegexPathSpec("^/rest/(?<name>.*)/ver-[0-9]+/(?<info>.*)$"), new ContextDumpHandler());
ContextHandler zedContext = new ContextHandler("/zed");
zedContext.setHandler(new ContextDumpHandler());
pathMappingsHandler.addMapping(new ServletPathSpec("/zed/*"), zedContext);
contextHandler.setHandler(pathMappingsHandler);

startServer(contextHandler);

HttpTester.Response response = executeRequest("""
GET %s HTTP/1.1\r
Host: local\r
Connection: close\r

""".formatted(requestPath));
assertEquals(200, response.getStatus());
assertThat(response.getContent(), containsString("contextPath=[" + expectedContextPath + "]"));
assertThat(response.getContent(), containsString("pathInContext=[" + expectedPathInContext + "]"));
if (expectedPathSpecImpl != null)
assertThat(response.getContent(), containsString("pathSpec=[" + expectedPathSpecImpl + "]"));
if (expectedPathSpecDeclaration != null)
assertThat(response.getContent(), containsString("pathSpec.declaration=[" + expectedPathSpecDeclaration + "]"));
}

@Test
public void testDump() throws Exception
{
Expand Down Expand Up @@ -306,7 +362,7 @@ public boolean handle(Request request, Response response, Callback callback)
assertTrue(isStarted());
response.setStatus(HttpStatus.OK_200);
response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8");
response.write(true, BufferUtil.toBuffer(message, StandardCharsets.UTF_8), callback);
Content.Sink.write(response, true, message, callback);
return true;
}

Expand All @@ -316,4 +372,41 @@ public String toString()
return String.format("%s[msg=\"%s\"]", SimpleHandler.class.getSimpleName(), message);
}
}

private static class ContextDumpHandler extends Handler.Abstract
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
String message = null;
PathSpec pathSpec = (PathSpec)request.getAttribute(PathSpec.class.getName());
try (StringWriter stringWriter = new StringWriter();
PrintWriter out = new PrintWriter(stringWriter))
{
out.printf("contextPath=[%s]\n", Request.getContextPath(request));
out.printf("pathInContext=[%s]\n", Request.getPathInContext(request));
if (pathSpec != null)
{
out.printf("pathSpec=[%s]\n", pathSpec.getClass().getSimpleName());
out.printf("pathSpec.declaration=[%s]\n", pathSpec.getDeclaration());
}
message = stringWriter.toString();
}
catch (IOException e)
{
callback.failed(e);
return true;
}
response.setStatus(HttpStatus.OK_200);
response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8");
Content.Sink.write(response, true, message, callback);
return true;
}

@Override
public String toString()
{
return ContextDumpHandler.class.getSimpleName();
}
}
}