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

Paths with leading separator not handled correctly #142

Closed
zdenek-jonas opened this issue Jul 27, 2020 · 12 comments
Closed

Paths with leading separator not handled correctly #142

zdenek-jonas opened this issue Jul 27, 2020 · 12 comments
Assignees

Comments

@zdenek-jonas
Copy link
Contributor

Edit TM:
this issue turned out to actually be about leading separators in paths. See below


Configuration - setBaseDirectory() has no effect.

Project: Microstream-test
branch: master
test: microstream-test/intern/src/main/java/test/java/microstream/configuration/baseDirectoryTest.java

test:

class BaseDirectoryTest {

    @TempDir
    Path location;

    Path configFilePath;


    @Test
    void baseDirectoryTest() throws IOException {
        configFilePath = location.resolve("baseDirectory.ini");
        FileUtils.writeStringToFile(configFilePath.toFile(),"baseDirectory = "+ location.toString(), "UTF-8");

        Customer customer = CustomerGenerator.generateNewCustomer();

        Configuration configuration = Configuration.LoadIni(configFilePath);

        EmbeddedStorageManager storageManager = configuration.createEmbeddedStorageFoundation().createEmbeddedStorageManager(customer).start();

        List<File> files = (List<File>) FileUtils.listFiles(location.toFile(), null, true);

        assertTrue(files.size() > 2,files.toString());

        storageManager.shutdown();
    }
}

image

@zdenek-jonas
Copy link
Contributor Author

When ich write the location direct:
Path location = Paths.get("/tmp/xxxx/");

image

Bei Flo works... linux problem?

@zdenek-jonas
Copy link
Contributor Author

Ok, after some evaluation, in linux, the MS ignore the full path of file `/tmp/junitxxxx' and create folder tmp in actual directory. This is definitely bug.
image

@fh-ms
Copy link
Contributor

fh-ms commented Jul 28, 2020

So, long story short, the AFS must keep leading separators, because they are relevant on most systems.

@tm-ms
Copy link
Contributor

tm-ms commented Jul 29, 2020

This is a little more complicated in principle than requested.
(Funnily, it turned out to be even easier at the same time, but from the beginning).

A leading "/" can't just be "kept" because the abstract path representation does not hold any separators. That's the point of having an abstraction.
What can (and must) be done, however, if for the leading "/" to cause a root element with an empty string as its identifier, so the assembled physical path will have the leading "/" again.

It turned out that cutting it off wasn't even a bug in the AFS code. Instead, the JDK's StringTokenizer used to split the path String simply ignores leading separators. No null element, no "" element, no exception, no configurability, nothing.
Therefore, from this point on and beknownst to all, StringTokenizer shall be called "StringLolenizer".

I only implemented a workaround for the StringLolenizer bug in the string splitting utility method and now causes a ""-identifier root element, which, in turn, causes the leading "/" to be replicated correctly.

Also note:
All the funny leading symbols like ".", "..", "~", etc. are kept automatically. Everything that is not a separator is simply an identifier. So they simply create root elements named "." etc., which get reassembled as that for accessing the physical file.
How useful or correct that might be when changing the physical layer (e.g. windows <-> linux or local file system to cloud storage or whatever) is the user's responsibility.

@zdenek-jonas
Copy link
Contributor Author

@tm-ms I still have the same behavior.
I double-checked that I have the correct version, that the fix in question is included in the version.
I launch exact the same test as mentioned in issue. And the behavior is the same as reported.

@tm-ms
Copy link
Contributor

tm-ms commented Aug 3, 2020

I looked into this again.

There is the simple test class for testing leading a separator in the path String MainTestAfsSpecialRoots.
Even more simplified, its this:

public static void main(final String[] args)
{
	final Path filePath = Paths.get("/some/path/with/leading/separator.test");
	final AFile file = NioFileSystem.New().ensureFile(filePath);
	
	// must replicate the path string, including the leading "/".
	System.out.println(file);
}

This replicates the correct String in the output.
E.g.
one.microstream.afs.AFile$Default@3e3abc88("/some/path/with/leading/separator.test")
Everything after that is just passing the String (Path) to the JDK NIO layer.

And I tested setting a different base directory via
Storage.FileProviderBuilder().setDirectory( ... )

This also works corrects. The storage is located at the specified directory and can be used without exception.

This makes me strongly assume that the cause for the problem is not located within the core logic.
For example, the configuration layer ist used, which is not part of the core.

@tm-ms tm-ms removed their assignment Aug 3, 2020
@zdenek-jonas
Copy link
Contributor Author

zdenek-jonas commented Aug 3, 2020

image

I have tried this sample method, print correct but the file and path is not created....

@zdenek-jonas
Copy link
Contributor Author

public class MainTestAfsSpecialRoot {
    public static void main(final String[] args)
    {
        final Path filePath = Paths.get("/tmp/path/with/leading/separator.test");
        final AFile file = NioFileSystem.New().ensureFile(filePath);
        file.fileSystem().ioHandler().create(file.useWriting());

        // must replicate the path string, including the leading "/".
        System.out.println(file);
    }
}

the file is created at relative directory and not absolute.
image

@tm-ms
Copy link
Contributor

tm-ms commented Aug 3, 2020

I was confused about the specific problem.
I thought it was still about the initial description of setting the base directory having no effect at all.
But in the meantime, the specific problem changed to the file being created relatively.

Since I don't have Linux and the problem hasn't simply been debugged, I did some extensive debugging, testing and research.
Turned out that Java in general does not seem to handle the "~" symbol:
https://stackoverflow.com/questions/7163364/how-to-handle-in-file-paths

Rejected JDK bug report:
https://bugs.openjdk.java.net/browse/JDK-8152072
This one is for windows but if I understand it correctly, Alan Bateman's answer applies in general:
"~" is not a symbol of the file system even on Linux, but of the shell for the Linux file system (or in general "the application").
So handling it is not a file system handling concern, but an application logic concern.

Specifically for Linux:
https://stackoverflow.com/questions/7463545/accessing-user-home-from-java-in-linux
"The ~ notation is a shell thing."

Very similarily, on Windows path variables (%HOMEDRIVE%, %HOMEPATH%) don't work in Java on Windows, either.

@zdenek-jonas
Copy link
Contributor Author

If I understand good:
Generally i do not have a problem with '~'. I have a problem, that when i write full path,
/var/path/to/something
is file write in
actualFolder /var/path/to/something

'~' on the screen is only shortcut to home folder....

@tm-ms tm-ms changed the title Configuration - setBaseDirectory() has no effect. Paths with leading separator not handled correctly Aug 4, 2020
@tm-ms
Copy link
Contributor

tm-ms commented Aug 4, 2020

So after two stages of confusion, I searched for a bug regarding the handling of paths with a leading "/".
The direct cause is once again a hilarious JDK story.
In short: Paths#get ignores empty path elements in the passed array.
So "/var" is parsed to the two separator-independant path elements {"", "var"}, which are assembled by Paths#get just as if it was only {"var"}. This swallows the leading slash.

So once again there's quite some idiocy going on in the JDK that makes their code unusable for all cases without additional workarounds.

As a workaround, I enhanced my method XIO#Path (already compensating some other idiocy of Path#get) with an explicit check for this case: If the first element of the path is "", then prepend an explicit leading separator.
This did the trick as least as far as properly keeping the leading "/".
I don't know about how Linux handles a query of the existence of the directory "", though.

But while this would probably work on Linus, its caused another problem on Windows:
The existence of the physical pendant of a directory is checked cascadingly.
Meaning checking {"", "var", "path", "to", "something"} first checks {"", "var", "path", "to"}, this in turn checks {"", "var", "path"} and then {"", "var"}. But assembling this with a leading slash creates "/var", which is translated by the JDK NIO Windows implementation as an UNC path "//var". But only a host without shared path is not valid, so this causes an exception.
(see https://stackoverflow.com/questions/20611779/java-file-system-unc-path-is-missing-sharename)

So after much research and fiddling, I came up with the following solution:

  • empty-named root directories are assumed to "exist" implicitely since they are not really a directory but only a dummy for a leading slash. This should take care of that case regardless of the underlying physical file system.
  • directories are not existence-checked cascadingly. This works just as well, but maybe shifts a little more work to the IoHandler implementation. In case of NIO, the JDK's NIO implementation itself handles the cascading directory checking and creation.

With that change, testing the path "/some/path/with/leading/separator.test" causes "only" the FileSystemException "\some\path\with: The network path was not found.", which is correct because there is no such network path on my system.

As far as I can tell, the absolut path should work now.

@zdenek-jonas
Copy link
Contributor Author

in Linux is correct.

@fh-ms fh-ms transferred this issue from another repository Apr 27, 2021
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

3 participants