This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-vfs.git
The following commit(s) were added to refs/heads/master by this push:
new cb45c945 Support URLencoding during normalization (#396)
cb45c945 is described below
commit cb45c9453669eb2570f582f491cb0e06b0e63466
Author: Arnout Engelen <arn...@engelen.eu>
AuthorDate: Tue Jan 9 21:07:15 2024 +0100
Support URLencoding during normalization (#396)
* Support URLencoding during normalization
Earlier, the path would be URL-decoded after normalization.
This meant some opportunities for normalization would be missed
when dots or slashes were URL-encoded.
One way to solve this would be to change the ordering of operations,
but that seems risky as it would change the meaning of some of the
abstractions. Because of that I opted for a more conservative
approach, where normalization will take into account URL-encoded
characters, but otherwise leave the input string intact.
* Fix binary compatibility
* Fix DefaultFileSystemManagerTest
Might need some more reviewing to make sure this logic is correct
* Make PMD happy
* A few additional testcases
---------
Co-authored-by: Gary Gregory <garydgreg...@users.noreply.github.com>
---
.../apache/commons/vfs2/provider/UriParser.java | 230 ++++++++++++++++-----
.../java/org/apache/commons/vfs2/NamingTests.java | 6 +
2 files changed, 182 insertions(+), 54 deletions(-)
diff --git
a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/UriParser.java
b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/UriParser.java
index 327f6049..2f494b95 100644
--- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/UriParser.java
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/UriParser.java
@@ -47,6 +47,9 @@ public final class UriParser {
private static final char LOW_MASK = 0x0F;
+ private static final String URLENCODED_SLASH_LC = "%2f";
+ private static final String URLENCODED_SLASH_UC = "%2F";
+
/**
* Encodes and appends a string to a StringBuilder.
*
@@ -460,6 +463,168 @@ public final class UriParser {
return changed;
}
+ private static class PathNormalizer {
+ private final StringBuilder path;
+
+ private int cursor;
+ private int lastSeparator;
+ private int end;
+
+ PathNormalizer(StringBuilder path) {
+ this.path = path;
+ this.end = path.length();
+ }
+
+ void run() throws FileSystemException {
+ lastSeparator = cursor;
+ readSeparator();
+ while (cursor < end) {
+ consumeSeparators();
+ if (readDot()) {
+ if (readDot()) {
+ int beforeNextSeparator = cursor;
+ if (readSeparator() || cursor == end) {
+ // '/../'
+ removePreviousElement(beforeNextSeparator);
+ } else {
+ // '/..other'
+ readNonSeparators();
+ lastSeparator = cursor;
+ readSeparator();
+ }
+ } else {
+ int beforeNextSeparator = cursor;
+ if (readSeparator() || cursor == end) {
+ // '/./'
+ path.delete(lastSeparator, beforeNextSeparator);
+ cursor = lastSeparator + (cursor -
beforeNextSeparator);
+ this.end = path.length();
+ } else {
+ // '/.other'
+ readNonSeparators();
+ lastSeparator = cursor;
+ readSeparator();
+ }
+ }
+ } else {
+ readToNextSeparator();
+ lastSeparator = cursor;
+ readSeparator();
+ }
+ }
+ }
+
+ private void consumeSeparators() {
+ boolean consuming = true;
+ while (consuming) {
+ consuming = consumeSeparator();
+ }
+ }
+
+ private void readNonSeparators() {
+ boolean reading = true;
+ while (reading) {
+ reading = readNonSeparator();
+ }
+ }
+
+ private void removePreviousElement(int to) throws FileSystemException {
+ if (lastSeparator == 0) {
+ // Previous element is missing
+ throw new
FileSystemException("vfs.provider/invalid-relative-path.error");
+ }
+ cursor = lastSeparator - 1;
+ while (readNonSeparator()) {
+ cursor = cursor - 2;
+ if (cursor < 0) {
+ // Previous element is the first element
+ cursor = 0;
+ break;
+ }
+ }
+ path.delete(cursor, to);
+ lastSeparator = cursor;
+ this.end = path.length();
+ readSeparator();
+ }
+
+ private void readToNextSeparator() {
+ boolean reading = true;
+ while (reading) {
+ reading = readNonSeparator();
+ }
+ }
+
+ private boolean readSeparator() {
+ if (cursor == end) {
+ return false;
+ }
+ if (path.charAt(cursor) == SEPARATOR_CHAR) {
+ cursor++;
+ return true;
+ }
+ if (cursor + 2 >= end) {
+ return false;
+ }
+ String sub = path.substring(cursor, cursor + 3);
+ if (sub.equals(URLENCODED_SLASH_LC) ||
sub.equals(URLENCODED_SLASH_UC)) {
+ cursor = cursor + 3;
+ return true;
+ }
+ return false;
+ }
+
+ private boolean readDot() {
+ if (cursor == end) {
+ return false;
+ }
+ if (path.charAt(cursor) == '.') {
+ cursor++;
+ return true;
+ }
+ if (cursor + 2 >= end) {
+ return false;
+ }
+ String sub = path.substring(cursor, cursor + 3);
+ if (sub.equals("%2e") || sub.equals("%2E")) {
+ cursor = cursor + 3;
+ return true;
+ }
+ return false;
+ }
+
+ private boolean consumeSeparator() {
+ int from = cursor;
+ if (readSeparator()) {
+ path.delete(from, cursor);
+ cursor = from;
+ this.end = path.length();
+ return true;
+ }
+ return false;
+ }
+
+ private boolean readNonSeparator() {
+ if (cursor == end) {
+ return false;
+ }
+ if (path.charAt(cursor) == SEPARATOR_CHAR) {
+ return false;
+ }
+ if (cursor + 2 >= end) {
+ cursor++;
+ return true;
+ }
+ String sub = path.substring(cursor + 1, cursor + 3);
+ if (sub.equals(URLENCODED_SLASH_UC) ||
sub.equals(URLENCODED_SLASH_LC)) {
+ return false;
+ }
+ cursor++;
+ return true;
+ }
+
+ }
+
/**
* Normalises a path. Does the following:
* <ul>
@@ -495,64 +660,21 @@ public final class UriParser {
// Adjust separators
// fixSeparators(path);
- // Determine the start of the first element
- int startFirstElem = 0;
- if (path.charAt(0) == SEPARATOR_CHAR) {
- if (path.length() == 1) {
- return fileType;
- }
- startFirstElem = 1;
- }
+ // Resolve double separators, '/./' and '/../':
+ new PathNormalizer(path).run();
- // Iterate over each element
- int startElem = startFirstElem;
- int maxlen = path.length();
- while (startElem < maxlen) {
- // Find the end of the element
- int endElem = startElem;
- while (endElem < maxlen && path.charAt(endElem) != SEPARATOR_CHAR)
{
- endElem++;
- }
-
- final int elemLen = endElem - startElem;
- if (elemLen == 0) {
- // An empty element - axe it
- path.delete(endElem, endElem + 1);
- maxlen = path.length();
- continue;
- }
- if (elemLen == 1 && path.charAt(startElem) == '.') {
- // A '.' element - axe it
- path.delete(startElem, endElem + 1);
- maxlen = path.length();
- continue;
+ // Remove trailing separator
+ if (!VFS.isUriStyle()) {
+ int maxlen = path.length();
+ if (maxlen > 1 && path.charAt(maxlen - 1) == SEPARATOR_CHAR) {
+ path.delete(maxlen - 1, maxlen);
}
- if (elemLen == 2 && path.charAt(startElem) == '.' &&
path.charAt(startElem + 1) == '.') {
- // A '..' element - remove the previous element
- if (startElem == startFirstElem) {
- // Previous element is missing
- throw new
FileSystemException("vfs.provider/invalid-relative-path.error");
+ if (maxlen > 3) {
+ String sub = path.substring(maxlen - 3);
+ if (sub.equals(URLENCODED_SLASH_UC) ||
sub.equals(URLENCODED_SLASH_LC)) {
+ path.delete(maxlen - 3, maxlen);
}
-
- // Find start of previous element
- int pos = startElem - 2;
- while (pos >= 0 && path.charAt(pos) != SEPARATOR_CHAR) {
- pos--;
- }
- startElem = pos + 1;
-
- path.delete(startElem, endElem + 1);
- maxlen = path.length();
- continue;
}
-
- // A regular element
- startElem = endElem + 1;
- }
-
- // Remove trailing separator
- if (!VFS.isUriStyle() && maxlen > 1 && path.charAt(maxlen - 1) ==
SEPARATOR_CHAR) {
- path.delete(maxlen - 1, maxlen);
}
return fileType;
diff --git
a/commons-vfs2/src/test/java/org/apache/commons/vfs2/NamingTests.java
b/commons-vfs2/src/test/java/org/apache/commons/vfs2/NamingTests.java
index 69df17b4..8e8b827b 100644
--- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/NamingTests.java
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/NamingTests.java
@@ -103,6 +103,10 @@ public class NamingTests extends AbstractProviderTestCase {
final String path = name.getPath() + "/a";
assertSameName(path, name, path, scope);
assertSameName(path, name, "../" + name.getBaseName() + "/a", scope);
+ assertSameName(path, name, "./a", scope);
+ assertSameName(path, name, "./a/foo/..", scope);
+ assertSameName(path, name, "foo/../a", scope);
+ assertSameName(path, name, "foo%2f..%2fa", scope);
// Test an empty name
assertBadName(name, "", scope);
@@ -116,6 +120,8 @@ public class NamingTests extends AbstractProviderTestCase {
assertBadName(name, "../a", scope);
assertBadName(name, "../" + name.getBaseName() + "a", scope);
assertBadName(name, "a/..", scope);
+ assertBadName(name, "%2e%2e/ab", scope);
+ assertBadName(name, "..%2f../ab", scope);
// Test absolute names
assertBadName(name, "/", scope);
