Re: RFR: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input [v2]
On Tue, 8 Dec 2020 03:00:34 GMT, Athijegannathan Sundararajan wrote: >> Safe URI encode logic adopted from UnixUriUtils. > > Athijegannathan Sundararajan has updated the pull request incrementally with > one additional commit since the last revision: > > renamed the test as per review comment. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1669
Re: RFR: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input [v2]
On Tue, 8 Dec 2020 02:55:43 GMT, Athijegannathan Sundararajan
wrote:
>> test/jdk/jdk/internal/jrtfs/Test8242258.java line 60:
>>
>>> 58: { "xyz ", "jrt:/xyz%20" },
>>> 59: { "xy z", "jrt:/xy%20z" },
>>> 60: };
>>
>> One other thing we test here is a malformed escape pair, e.g. "jrt:/%5" and
>> check that getPath(URI) throws IAE.
>
> URI.create(String) method itself checks for malformed escape pairs. Do we
> need anything additional here?
The bug report is Path -> URI but any testing here has to do round-trip (as you
have done). My comment about the malformed escaped pair is that we don't seem
to have tests for this case, or did I missed it? The example I tased was %5
which isn't a complete pair and will be rejected. In any case, I think what you
have is fine for now.
-
PR: https://git.openjdk.java.net/jdk/pull/1669
Re: RFR: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input [v2]
On Mon, 7 Dec 2020 19:59:40 GMT, Alan Bateman wrote:
>> Athijegannathan Sundararajan has updated the pull request incrementally with
>> one additional commit since the last revision:
>>
>> renamed the test as per review comment.
>
> test/jdk/jdk/internal/jrtfs/Test8242258.java line 40:
>
>> 38: import static org.testng.Assert.assertEquals;
>> 39:
>> 40: public class Test8242258 {
>
> I think it would be better to create something like UriTests that we can add
> further tests for jrtfs URIs as they arise.
I'll renamed the test
> test/jdk/jdk/internal/jrtfs/Test8242258.java line 60:
>
>> 58: { "xyz ", "jrt:/xyz%20" },
>> 59: { "xy z", "jrt:/xy%20z" },
>> 60: };
>
> One other thing we test here is a malformed escape pair, e.g. "jrt:/%5" and
> check that getPath(URI) throws IAE.
URI.create(String) method itself checks for malformed escape pairs. Do we need
anything additional here?
-
PR: https://git.openjdk.java.net/jdk/pull/1669
Re: RFR: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input [v2]
> Safe URI encode logic adopted from UnixUriUtils. Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision: renamed the test as per review comment. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1669/files - new: https://git.openjdk.java.net/jdk/pull/1669/files/b3d0a927..4d7417f8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1669=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1669=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1669.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1669/head:pull/1669 PR: https://git.openjdk.java.net/jdk/pull/1669
