Skip to content

Commit

Permalink
Simplify UriParser
Browse files Browse the repository at this point in the history 
The reverts #396 and related
changes and implements the same in a simpler way by replacing the
encoded characters already in `fixSeparators`.

This approach has a slightly higher risk at breaking existing behaviour,
but a lower risk of remaining problems in this part of the codebase. All
testcases still succeed.

This PR is intended to replace #543 and #555. It includes the testcases
from #543, adapted to the behaviour before #396.

This reverts commit cb45c94.
This reverts commit 5399c76.

Co-Authored-By: Anthony Goubard <[email protected]>
  • Loading branch information
@raboof @japplis
raboof and japplis committed Jun 26, 2024
1 parent 9d7a7d4 commit 4b02af2
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 177 deletions.
245 changes: 68 additions & 177 deletions commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/UriParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,168 +29,6 @@
*/
public final class UriParser {

private static class PathNormalizer {
private final StringBuilder path;

private int cursor;
private int lastSeparator;
private int end;

PathNormalizer(final StringBuilder path) {
this.path = path;
this.end = path.length();
}

private boolean consumeSeparator() {
final int from = cursor;
if (readSeparator()) {
path.delete(from, cursor);
cursor = from;
this.end = path.length();
return true;
}
return false;
}

private void consumeSeparators() {
boolean consuming = true;
while (consuming) {
consuming = consumeSeparator();
}
}

private boolean readDot() {
if (cursor == end) {
return false;
}
if (path.charAt(cursor) == '.') {
cursor++;
return true;
}
if (cursor + 2 >= end) {
return false;
}
final String sub = path.substring(cursor, cursor + 3);
if (sub.equals("%2e") || sub.equals("%2E")) {
cursor += 3;
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;
}
final String sub = path.substring(cursor + 1, cursor + 3);
if (sub.equals(URLENCODED_SLASH_UC) || sub.equals(URLENCODED_SLASH_LC)) {
return false;
}
cursor++;
return true;
}

private void readNonSeparators() {
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;
}
final String sub = path.substring(cursor, cursor + 3);
if (sub.equals(URLENCODED_SLASH_LC) || sub.equals(URLENCODED_SLASH_UC)) {
cursor += 3;
return true;
}
return false;
}

private void readToNextSeparator() {
boolean reading = true;
while (reading) {
reading = readNonSeparator();
}
}

private void removePreviousElement(final 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 -= 2;
if (cursor < 0) {
// Previous element is the first element
cursor = 0;
break;
}
}
path.delete(cursor, to);
lastSeparator = cursor;
this.end = path.length();
readSeparator();
}

void run() throws FileSystemException {
lastSeparator = cursor;
readSeparator();
while (cursor < end) {
consumeSeparators();
if (readDot()) {
if (readDot()) {
final int beforeNextSeparator = cursor;
if (readSeparator() || cursor == end) {
// '/../'
removePreviousElement(beforeNextSeparator);
} else {
// '/..other'
readNonSeparators();
lastSeparator = cursor;
readSeparator();
}
} else {
final 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();
}
}
}

}

/**
* The set of valid separators. These are all converted to the normalized one. Does <i>not</i> contain the
* normalized separator
Expand All @@ -208,9 +46,6 @@ void run() throws FileSystemException {
private static final int BITS_IN_HALF_BYTE = 4;

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.
Expand Down Expand Up @@ -614,13 +449,26 @@ public static String extractScheme(final String[] schemes, final String uri, fin
*/
public static boolean fixSeparators(final StringBuilder name) {
boolean changed = false;
final int maxlen = name.length();
int maxlen = name.length();
for (int i = 0; i < maxlen; i++) {
final char ch = name.charAt(i);
if (ch == TRANS_SEPARATOR) {
name.setCharAt(i, SEPARATOR_CHAR);
changed = true;
}
if (i < maxlen - 2 && name.charAt(i) == '%' && name.charAt(i + 1) == '2') {
if (name.charAt(i + 2) == 'f' || name.charAt(i + 2) == 'F') {
name.setCharAt(i, SEPARATOR_CHAR);
name.delete(i + 1, i + 3);
maxlen -= 2;
changed = true;
} else if (name.charAt(i + 2) == 'e' || name.charAt(i + 2) == 'E') {
name.setCharAt(i, '.');
name.delete(i + 1, i + 3);
maxlen -= 2;
changed = true;
}
}
}
return changed;
}
Expand Down Expand Up @@ -660,21 +508,64 @@ public static FileType normalisePath(final StringBuilder path) throws FileSystem
// Adjust separators
// fixSeparators(path);

// Resolve double separators, '/./' and '/../':
new PathNormalizer(path).run();
// Determine the start of the first element
int startFirstElem = 0;
if (path.charAt(0) == SEPARATOR_CHAR) {
if (path.length() == 1) {
return fileType;
}
startFirstElem = 1;
}

// Remove trailing separator
if (!VFS.isUriStyle()) {
final int maxlen = path.length();
if (maxlen > 1 && path.charAt(maxlen - 1) == SEPARATOR_CHAR) {
path.delete(maxlen - 1, maxlen);
// 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;
}
if (maxlen > 3) {
final String sub = path.substring(maxlen - 3);
if (sub.equals(URLENCODED_SLASH_UC) || sub.equals(URLENCODED_SLASH_LC)) {
path.delete(maxlen - 3, 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");
}

// 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;
Expand Down
4 changes: 4 additions & 0 deletions commons-vfs2/src/test/java/org/apache/commons/vfs2/NamingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ private void checkDescendentNames(final FileName name, final NameScope scope) th
assertSameName(path, name, "./a/foo/..", scope);
assertSameName(path, name, "foo/../a", scope);
assertSameName(path, name, "foo%2f..%2fa", scope);
assertSameName(path, name, "foo%2f/..%2fa", scope);
assertSameName(path, name, "foo/%2f..%2fa", scope);

// Test an empty name
assertBadName(name, "", scope);
Expand All @@ -123,6 +125,8 @@ private void checkDescendentNames(final FileName name, final NameScope scope) th
assertBadName(name, "a/..", scope);
assertBadName(name, "%2e%2e/ab", scope);
assertBadName(name, "..%2f../ab", scope);
assertBadName(name, "foo%2f..%2f..%2fa", scope);
assertBadName(name, "foo%2f..%2f..%2fnone-child%2fa", scope);

// Test absolute names
assertBadName(name, "/", scope);
Expand Down
17 changes: 17 additions & 0 deletions commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/UriParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,21 @@ public void testTypeOfNormalizedPath() {
fail(e);
}
}

@Test
public void testPathOfNormalizedPath() throws FileSystemException {
checkNormalizedPath("./Sub Folder/", "Sub Folder");
checkNormalizedPath("./Sub Folder/../", "");
checkNormalizedPath("./Sub Folder%2f..%2f", "");
checkNormalizedPath("File.txt", "File.txt");
checkNormalizedPath("./Sub Folder/./File.txt", "Sub Folder/File.txt");
checkNormalizedPath("./Sub Folder%2F.%2FFile.txt", "Sub Folder/File.txt");
}

private void checkNormalizedPath(String path, String normalized) throws FileSystemException {
final StringBuilder pathBuilder = new StringBuilder(path);
UriParser.fixSeparators(pathBuilder);
UriParser.normalisePath(pathBuilder);
assertEquals(normalized, pathBuilder.toString());
}
}

0 comments on commit 4b02af2

Please sign in to comment.