Apply review feedback

This commit is contained in:
Volkan Yazıcı 2026-02-02 21:15:36 +01:00
parent 9d647f288c
commit e55e605046
No known key found for this signature in database
GPG Key ID: D37D4387C9BD368E
5 changed files with 37 additions and 29 deletions

View File

@ -284,8 +284,8 @@ public abstract class HttpServer {
* then some implementations may use <em>string prefix matching</em> where
* this context path matches request paths {@code /foo},
* {@code /foo/bar}, or {@code /foobar}. Others may use <em>path prefix
* matching</em> where {@code /foo} matches only request paths
* {@code /foo} and {@code /foo/bar}, but not {@code /foobar}.
* matching</em> where {@code /foo} matches request paths {@code /foo} and
* {@code /foo/bar}, but not {@code /foobar}.
*
* @implNote
* By default, the JDK built-in implementation uses path prefix matching.

View File

@ -107,7 +107,7 @@ import com.sun.net.httpserver.*;
* {@code pathPrefix})<br/>
*
* The path matching scheme used to route requests to context handlers.
* Following list of values are allowed by this property.</p>
* The property can be configured with one of the following values:</p>
*
* <blockquote>
* <dl>
@ -117,7 +117,7 @@ import com.sun.net.httpserver.*;
* would match request paths {@code /foo}, {@code /foo/}, and {@code /foo/bar},
* but not {@code /foobar}.</dd>
* <dt>{@code stringPrefix}</dt>
* <dd>Request path string must begin with the context path string. For
* <dd>The request path string must begin with the context path string. For
* instance, the context path {@code /foo} would match request paths
* {@code /foo}, {@code /foo/}, {@code /foo/bar}, and {@code /foobar}.
* </dd>

View File

@ -40,6 +40,8 @@ class ContextList {
// expects the protocol to be lower-cased using ROOT locale, hence:
assert ctx.getProtocol().equals(ctx.getProtocol().toLowerCase(Locale.ROOT));
assert ctx.getPath() != null;
// `ContextPathMatcher` expects context paths to be non-empty:
assert !ctx.getPath().isEmpty();
if (contains(ctx)) {
throw new IllegalArgumentException("cannot add context to list");
}
@ -199,10 +201,19 @@ class ContextList {
}
// Is it a path-prefix match?
if (contextPath.charAt(contextPathLength - 1) == '/'
|| requestPath.charAt(contextPathLength) == '/') {
return true;
}
assert contextPathLength > 0;
return
// Case 1: The request path starts with the context
// path, but the context path has an extra path
// separator suffix. For instance, the context path is
// `/foo/` and the request path is `/foo/bar`.
contextPath.charAt(contextPathLength - 1) == '/' ||
// Case 2: The request path starts with the
// context path, but the request path has an
// extra path separator suffix. For instance,
// context path is `/foo` and the request path
// is `/foo/` or `/foo/bar`.
requestPath.charAt(contextPathLength) == '/';
}

View File

@ -37,6 +37,7 @@ import static java.net.http.HttpClient.Builder.NO_PROXY;
import org.junit.jupiter.api.AfterAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import org.junit.jupiter.api.Test;
@ -72,7 +73,7 @@ import org.junit.jupiter.api.Test;
public class ContextPathMatcherPathPrefixTest {
private static final HttpClient CLIENT =
protected static final HttpClient CLIENT =
HttpClient.newBuilder().proxy(NO_PROXY).build();
@AfterAll
@ -80,6 +81,12 @@ public class ContextPathMatcherPathPrefixTest {
CLIENT.shutdownNow();
}
@Test
void testContextPathOfEmptyString() {
var iae = assertThrows(IllegalArgumentException.class, () -> new Infra(""));
assertEquals("Illegal value for path or protocol", iae.getMessage());
}
@Test
void testContextPathAtRoot() throws Exception {
try (var infra = new Infra("/")) {
@ -103,7 +110,7 @@ public class ContextPathMatcherPathPrefixTest {
}
}
public static final class Infra implements AutoCloseable {
protected static final class Infra implements AutoCloseable {
private static final InetSocketAddress LO_SA_0 =
new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
@ -114,14 +121,14 @@ public class ContextPathMatcherPathPrefixTest {
private final String contextPath;
public Infra(String contextPath) throws IOException {
protected Infra(String contextPath) throws IOException {
this.server = HttpServer.create(LO_SA_0, 10);
server.createContext(contextPath, HANDLER);
server.start();
this.contextPath = contextPath;
}
public void expect(int statusCode, String... requestPaths) throws Exception {
protected void expect(int statusCode, String... requestPaths) throws Exception {
for (String requestPath : requestPaths) {
var requestURI = URI.create("http://%s:%s%s".formatted(
server.getAddress().getHostString(),

View File

@ -21,13 +21,8 @@
* questions.
*/
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;
import java.net.http.HttpClient;
import static java.net.http.HttpClient.Builder.NO_PROXY;
/*
* @test
* @bug 8272758
@ -39,33 +34,28 @@ import static java.net.http.HttpClient.Builder.NO_PROXY;
* ${test.main.class}
*/
class ContextPathMatcherStringPrefixTest {
private static final HttpClient CLIENT =
HttpClient.newBuilder().proxy(NO_PROXY).build();
@AfterAll
static void stopClient() {
CLIENT.shutdownNow();
}
class ContextPathMatcherStringPrefixTest extends ContextPathMatcherPathPrefixTest {
@Test
@Override
void testContextPathAtRoot() throws Exception {
try (var infra = new ContextPathMatcherPathPrefixTest.Infra("/")) {
try (var infra = new Infra("/")) {
infra.expect(200, "/foo", "/foo/", "/foo/bar", "/foobar");
}
}
@Test
@Override
void testContextPathAtSubDir() throws Exception {
try (var infra = new ContextPathMatcherPathPrefixTest.Infra("/foo")) {
try (var infra = new Infra("/foo")) {
infra.expect(200, "/foo", "/foo/", "/foo/bar", "/foobar");
}
}
@Test
@Override
void testContextPathAtSubDirWithTrailingSlash() throws Exception {
try (var infra = new ContextPathMatcherPathPrefixTest.Infra("/foo/")) {
try (var infra = new Infra("/foo/")) {
infra.expect(200, "/foo/", "/foo/bar");
infra.expect(404, "/foo", "/foobar");
}