Copilot commented on code in PR #773:
URL: https://github.com/apache/commons-vfs/pull/773#discussion_r3580895897
##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/UriParser.java:
##########
@@ -161,12 +161,29 @@ public static void decode(final StringBuilder buffer,
final int offset, final in
int index = offset;
int count = length;
boolean ipv6Host = false;
+ boolean authorityDetected = false;
+ boolean inAuthority = false;
+ int consecutiveSlashes = 0;
for (; count > 0; count--, index++) {
final char ch = buffer.charAt(index);
- if (ch == '[') {
- ipv6Host = true;
- }
- if (ch == ']') {
+ if (!authorityDetected) {
+ if (ch == '/') {
+ consecutiveSlashes++;
+ if (consecutiveSlashes == 2) {
+ inAuthority = true;
+ authorityDetected = true;
+ }
Review Comment:
Authority detection currently triggers on any "//" sequence anywhere in the
string. For inputs that have no authority but contain "//" later in the path
(e.g., "file:/a//[inside%25text]" or other non-hierarchical URIs), this will
incorrectly treat subsequent "[...]" as an IPv6 host and skip percent-decoding
inside the brackets, reintroducing the original bug for those cases. Limit
authority detection to a leading "//" (network-path reference) or to "://"
immediately after a scheme delimiter.
##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/local/UrlTests.java:
##########
@@ -65,4 +70,31 @@ public void testHashURL() throws Exception {
assertEquals(file.toString(),
UriParser.decode(file.getURL().toString()));
}
+ /**
+ * Tests that getURL() round-trips correctly when a directory name
contains brackets.
+ */
+ @Test
+ public void testGetUrlRoundTripWithBracketsInPath() throws Exception {
+ final Path tmp = Files.createTempDirectory("vfs-roundtrip");
+ try {
+ final Path child = tmp.resolve("outside%text[inside%text]tail");
+ Files.createDirectories(child);
+
+ final FileSystemManager mgr = VFS.getManager();
+ final FileObject a = mgr.resolveFile(child.toUri().toString());
+ final FileObject b = mgr.resolveFile(a.getURL().toString());
+
+ assertEquals(a.getName().getPath(), b.getName().getPath());
+ } finally {
+ Files.walk(tmp)
+ .sorted(Comparator.reverseOrder())
+ .forEach(p -> {
+ try {
+ Files.delete(p);
+ } catch (final IOException ignore) { // NOPMD
+ }
+ });
Review Comment:
Files.walk(tmp) returns a Stream that must be closed. Not closing it can
leak file handles and (notably on Windows) can prevent the subsequent delete
operations from succeeding reliably. Wrap the walk in a try-with-resources.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
