RFR: JDK-8212035 : merge jdk.test.lib.util.SimpleHttpServer with jaxp.library.SimpleHttpServer [v4]

Daniel Fuchs dfuchs at openjdk.java.net
Mon Dec 14 19:47:59 UTC 2020


On Fri, 11 Dec 2020 21:23:12 GMT, Mahendra Chhipa <github.com+34924738+mahendrachhipa at openjdk.org> wrote:

>> jaxp.library.SimpleHttpServer
>> https://bugs.openjdk.java.net/browse/JDK-8212035
>
> Mahendra Chhipa has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Merge branch 'JDK-8212035' of https://github.com/mahendrachhipa/jdk into JDK-8212035
>  - Implemnted the review comments.

Changes requested by dfuchs (Reviewer).

test/lib/jdk/test/lib/net/SimpleHttpServer.java line 105:

> 103:                     Path fPath;
> 104:                     try {
> 105:                         fPath = Paths.get(docRoot, path);

Hi Mahendra,

I am not comfortable with this. You cannot just concatenate a file system path with a URI path in the general case (think windows file system). I would suggest to transform docRoot into a root URI using `Path.of(docRoot).toURI().normalize()` - then you can probably concatenate the string representation of this root URI with the *raw path* extracted from the request URI, normalize that again (to remove potential double slashes) and convert that back into a Path using Path.of(URI). 
Some additional conditions might need to be asserted:
 - `t.getRequestURI().getRawPath()` should either be empty or start with `"/"` (and even that may be a bit lax)
 - the resulting URI you obtain (after normalization and before converting back to Path) should either be equal to the root URI or start with the root URI.
`(rootURI.toString().equals(uri.toString()) || uri.toString().startsWith(rootUri.toString() + "/") )`

best regards,

-- daniel

-------------

PR: https://git.openjdk.java.net/jdk/pull/1632


More information about the core-libs-dev mailing list