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

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

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

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

Show file tree
Hide file tree
Changes from 1 commit
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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 @@ -142,4 +143,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();
}
}
}
41 changes: 40 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,48 @@ 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);
gregw marked this conversation as resolved.
Show resolved Hide resolved
boolean handled = handler.handle(pathSpecRequest, response, callback);
gregw marked this conversation as resolved.
Show resolved Hide resolved
gregw marked this conversation as resolved.
Show resolved Hide resolved
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;

public PathSpecRequest(Request request, PathSpec pathSpec)
{
super(request);
this.pathSpec = pathSpec;
setAttribute(PathSpec.class.getName(), this.pathSpec);
this.context = new Context.Wrapper(request.getContext())
gregw marked this conversation as resolved.
Show resolved Hide resolved
{
@Override
public String getContextPath()
{
return pathSpec.getPrefix();
}

@Override
public String getPathInContext(String canonicallyEncodedPath)
{
MatchedPath matchedPath = pathSpec.matched(canonicallyEncodedPath);
return matchedPath.getPathInfo();
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I'm not sure we can just return that pathInfo. I think we have to do:

Suggested change
return matchedPath.getPathInfo();
return Context.getPathInContext(getWrapped().getContext().getContextPath(), canonicallyEncodedPath);

}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct. I think you can just use the default method.

};
}

@Override
public Context getContext()
{
return context;
}
}
}
97 changes: 94 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,56 @@ public void testSeveralMappingAndNoWrapper(String requestPath, int expectedStatu
assertEquals(expectedResponseBody, response.getContent());
}

public static Stream<Arguments> pathInContextInput()
{
return Stream.of(
Arguments.of("/", "null", "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", "null", "null", ServletPathSpec.class.getSimpleName(), "/"),
Arguments.of("/deep/path/foo.php", "null", "null", ServletPathSpec.class.getSimpleName(), "*.php"),
Arguments.of("/re/1234/baz", "null", "null", ServletPathSpec.class.getSimpleName(), "/"),
Arguments.of("/re/ABC/baz", "null", "null", RegexPathSpec.class.getSimpleName(), "/re/[A-Z]*/.*"),
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());
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 +360,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 +370,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();
}
}
}
Loading