On Wed, 9 Apr 2025 14:18:20 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Apply review suggestions
>
> test/jdk/java/net/httpclient/EmptyAuthenticate.java line 75:
>
>> 73: void test(Version version, boolean secure) throws Exception {
>> 74: String uriPath =
>> "/%s/%s/%s".formatted(EmptyAuthenticate.class.getSimpleName(), version,
>> secure ? 's' : 'c');
>> 75: HttpTestServer server = createServer(version, secure, uriPath);
>
> It's actually better practice to have uri looks like "/rootpath/morechars"
> and register the handler for "/rootpath/"
> => make sure the path you pass to addHandler ends with "/".
>
> Suggestion:
>
> String handlerPath =
> "/%s/%s/".formatted(EmptyAuthenticate.class.getSimpleName(), version)
> String uriPath = handlerPath + (secure ? 's' : 'c');
> HttpTestServer server = createServer(version, secure, handlerPath);
Rationale: if you add a handler for "/foo" it will match "/foo/x" but also
"/foobar" which is not always expected by the developper. The recommendation is
therefore to use "/foo/" instead of "/foo" if you don't want "/foobar" to be
matched.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24542#discussion_r2035489732
