Skip to content

Commit

Permalink
feat: Implement S3Path.normalize() method. (#383)
Browse files Browse the repository at this point in the history
Co-authored-by: Enno Ruijters <eruijters@betterbe.com>
Co-authored-by: Marcel Hekman <mhekman@betterbe.com>
  • Loading branch information
3 people committed Jul 29, 2023
1 parent 51983e6 commit 2be9515
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 14 deletions.
95 changes: 83 additions & 12 deletions src/main/java/org/carlspring/cloud/storage/s3fs/S3Path.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
import java.nio.file.WatchEvent;
import java.nio.file.WatchKey;
import java.nio.file.WatchService;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Iterator;
import java.util.List;

import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -42,12 +45,12 @@ public class S3Path
* URI not encoded
* Is the key for S3Client
*/
private String uri;
private final String uri;

/**
* actual filesystem
*/
private S3FileSystem fileSystem;
private final S3FileSystem fileSystem;

/**
* S3BasicFileAttributes cache
Expand Down Expand Up @@ -109,18 +112,16 @@ public S3Path(S3FileSystem fileSystem, String first, String... more)
uriBuilder.append(path + PATH_SEPARATOR);
}
}

this.uri = normalizeURI(uriBuilder.toString());
String localUri = normalizeURI(uriBuilder.toString());
// remove last PATH_SEPARATOR
if (!first.isEmpty() &&
// only first param and not ended with PATH_SEPARATOR
((!first.endsWith(PATH_SEPARATOR) && (more == null || more.length == 0))
// we have more param and not ended with PATH_SEPARATOR
|| more != null && more.length > 0 && !more[more.length - 1].endsWith(PATH_SEPARATOR)))
{
this.uri = this.uri.substring(0, this.uri.length() - 1);
// only first param and not ended with PATH_SEPARATOR
((!first.endsWith(PATH_SEPARATOR) && (more == null || more.length == 0))
// we have more param and not ended with PATH_SEPARATOR
|| more != null && more.length > 0 && !more[more.length-1].endsWith(PATH_SEPARATOR))) {
localUri = localUri.substring(0, localUri.length() - 1);
}

this.uri = localUri;
this.fileSystem = fileSystem;
}

Expand Down Expand Up @@ -435,10 +436,80 @@ public boolean endsWith(String other)
return this.endsWith(new S3Path(this.fileSystem, other));
}

// Inspired by "JimfsPath.isNormal()"
@Override
public Path normalize()
{
return this;
List<String> pathParts = uriToList();
if (isNormal(pathParts))
return this;

Deque<String> newPathParts = new ArrayDeque<>();
for (String pathPart : pathParts)
{
if (pathPart.equals(".."))
{
String lastPathPart = newPathParts.peekLast();
if (lastPathPart != null && !lastPathPart.equals(".."))
{
newPathParts.removeLast();
}
else if (!isAbsolute())
{
// if there's a root and we have an extra ".." that would go
// up above the root, ignore it
newPathParts.add(pathPart);
}
}
else if (!pathPart.equals("."))
{
newPathParts.add(pathPart);
}
}
StringBuilder pathBuilder = new StringBuilder();
if (this.isAbsolute())
{
pathBuilder.append(PATH_SEPARATOR).append(this.fileStore.name()).append(PATH_SEPARATOR);
}
pathBuilder.append(Joiner.on(PATH_SEPARATOR).join(newPathParts));
if (newPathParts.size() > 0 && uri.endsWith(PATH_SEPARATOR))
{
pathBuilder.append(PATH_SEPARATOR);
}
return new S3Path(this.fileSystem, pathBuilder.toString());
}

// Inspired by "JimfsPath.isNormal()"
private boolean isNormal(List<String> pathParts)
{
if (getNameCount() == 0 || getNameCount() == 1 && !isAbsolute())
{
return true;
}
boolean foundNonParentName = isAbsolute(); // if there's a root, the
// path doesn't start with ..
boolean normal = true;
for (String pathPart : pathParts)
{
if (pathPart.equals(".."))
{
if (foundNonParentName)
{
normal = false;
break;
}
}
else
{
if (pathPart.equals("."))
{
normal = false;
break;
}
foundNonParentName = true;
}
}
return normal;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,31 @@ void getParent()
assertNull(forPath("/bucket").getParent());
}

@Test
public void normalize()
{
assertEquals(forPath("/bucket"), forPath("/bucket").normalize());
assertEquals(forPath("/bucket/"), forPath("/bucket/").normalize());
assertEquals(forPath("/bucket/"), forPath("/bucket/.").normalize());

// We can't normalize to outside of the bucket
assertEquals(forPath("/bucket/"), forPath("/bucket/..").normalize());
assertEquals(forPath("/bucket/path"), forPath("/bucket/../path").normalize());

// Various different spellings of the same path
assertEquals(forPath("/bucket/path/to"), forPath("/bucket/path/to").normalize());
assertEquals(forPath("/bucket/path/to/"), forPath("/bucket/path/to/").normalize());
assertEquals(forPath("/bucket/path/to/"), forPath("/bucket/path/to/file/../").normalize());
assertEquals(forPath("/bucket/path/to/"), forPath("/bucket/path/to/./").normalize());
assertEquals(forPath("/bucket/path/to/"), forPath("/bucket/./path/to/").normalize());
assertEquals(forPath("/bucket/path/to/"), forPath("/bucket/foo/./../bar/../path/to/").normalize());
assertEquals(forPath("/bucket/path/to/"), forPath("/bucket/path/to/foo/bar/../../").normalize());
assertEquals(forPath("/bucket/path/to/"), forPath("/bucket/././././././foo/./././../././bar/./././../path/./to/././").normalize());

S3Path path = forPath("../bucket/path/to");
assertTrue(path == path.normalize());
}

@Test
void nameCount()
{
Expand Down Expand Up @@ -144,7 +169,7 @@ void registerWatcherWithEventsAndModifierShouldThrowUnsupportedOperationExceptio
{
// We're expecting an exception here to be thrown
Exception exception = assertThrows(UnsupportedOperationException.class, () -> {
forPath("file1").register(null, null, null);
forPath("file1").register(null);
});

assertNotNull(exception);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void startsWithNotNormalize()
void startsWithNormalize()
{
// in this implementation not exists .. or . special paths
assertFalse(getPath("/bucket/file1/file2").startsWith(getPath("/bucket/file1/../").normalize()));
assertTrue(getPath("/bucket/file1/file2").startsWith(getPath("/bucket/file1/../").normalize()));
}

@Test
Expand Down

0 comments on commit 2be9515

Please sign in to comment.