Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3Path: relativize can return an absolute path #293

Closed
thsandu opened this issue Nov 9, 2023 · 5 comments
Closed

S3Path: relativize can return an absolute path #293

thsandu opened this issue Nov 9, 2023 · 5 comments

Comments

@thsandu
Copy link

thsandu commented Nov 9, 2023

We are relying in a part of our implementation on the behavior of the Path.relativize() to return a relative path. I discovered that the S3Path gives an absolute path back, if the relativize is executed on the root path. I checked again against windows and jimfs implementation:

@Test
public void relativizeAbsolutePathOnRoot() {
    Path path = Paths.get("C:\\");
    Path path2 = Paths.get("C:\\foo\\bar\\");
    assertFalse(path.relativize(path2).isAbsolute());
    
    FileSystem linux = Jimfs.newFileSystem("testme", Configuration.unix());
    Path pathJim = linux.getPath("/");
    Path pathJim2 = linux.getPath("/foo/bar/");
    assertFalse(pathJim.relativize(pathJim2).isAbsolute());
}

Here again a matching unit test: https://github.com/thsandu/aws-java-nio-spi-for-s3-issues-with-s3x/blob/2cbb6d27b8adde8499f49a146eedce870dec1ec6/src/test/java/software/amazon/nio/spi/s3/S3PathTest.java#L293

@markjschreiber
Copy link
Contributor

markjschreiber commented Nov 9, 2023

So does

    Path pathJim = linux.getPath("/");
    Path pathJim2 = linux.getPath("/foo/bar/");

    pathJim.relativize(pathJim2).isAbsolute();

result in a path equivalent to "./foo/bar/"?

@thsandu
Copy link
Author

thsandu commented Nov 9, 2023

No, it results in a path equivalent to "foo/bar/". This I interpret as the relative path from "/foo/bar/" to "/".

stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue Mar 3, 2024
stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue Mar 3, 2024
stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue Mar 3, 2024
stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue Mar 3, 2024
stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue Mar 3, 2024
stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue Mar 3, 2024
stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue Mar 3, 2024
@markjschreiber
Copy link
Contributor

Can you check if PR #394 has fixed this?

@markjschreiber
Copy link
Contributor

This test seems to pass with PR #394.

    @Test
    void testRelativizeAgainstRoot() {
        var s3Path = S3Path.getPath(fileSystem, "/");
        var relativePath = s3Path.relativize(S3Path.getPath(fileSystem, "/a/b/c/d"));
        assertEquals("a/b/c/d", relativePath.toString());
        assertFalse(relativePath.isAbsolute());

        var emptyS3Path = S3Path.getPath(fileSystem, "");
        relativePath = emptyS3Path.relativize(S3Path.getPath(fileSystem, "a/b/c/d"));
        assertEquals("a/b/c/d", relativePath.toString());
        assertFalse(relativePath.isAbsolute());
    }

@markjschreiber
Copy link
Contributor

I have added some additional tests to prevent a regression. Feel free to open a new issue if this is still a problem.

stefanofornari added a commit to stefanofornari/aws-java-nio-spi-for-s3 that referenced this issue May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants