Fix for spaces in data path doesn't work on windows

[Blanco27]

Trying to create an embedded DB like this:

        final var db = DB.newEmbeddedDB( DBConfigurationBuilder.newBuilder()
            .setDeletingTemporaryBaseAndDataDirsOnShutdown( false )
            .setDataDir( "D:\\contains spaces" )
            .build() );

creates two folders:

The one that ends up being used is the one with the ``%20` for spaces, the other one is left empty.

If you append another folder behind the one with spaces, you get an exception instead:

15:50:51,468 ERROR LoggingExecuteResultHandler:55 - Program [C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base\bin\mysql_install_db.exe, --datadir=D:\contains%20spaces\data] (in working directory C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base) failed unexpectedly
org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404)
	at org.apache.commons.exec.DefaultExecutor.access$200(DefaultExecutor.java:48)
	at org.apache.commons.exec.DefaultExecutor$1.run(DefaultExecutor.java:200)
	at java.base/java.lang.Thread.run(Thread.java:829)
15:50:51,520 ERROR ManagedProcess:308 - Program [C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base\bin\mysql_install_db.exe, --datadir=D:\contains%20spaces\data] (in working directory C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base) failed
15:50:51,544 ERROR SpringBootstrap:1000 - Konnte APM Server Bootstrap nicht starten.
ch.vorburger.exec.ManagedProcessException: An error occurred while installing the database
	at ch.vorburger.mariadb4j.DB.install(DB.java:143)
	at ch.vorburger.mariadb4j.DB.newEmbeddedDB(DB.java:83)
	at com.isp.lea.service.ioc.SpringBootstrap.startServer(SpringBootstrap.java:684)
	at com.isp.lea.service.ioc.SpringBootstrap.main(SpringBootstrap.java:230)
Caused by: ch.vorburger.exec.ManagedProcessException: Program [C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base\bin\mysql_install_db.exe, --datadir=D:\contains%20spaces\data] (in working directory C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base) failed, exitValue=1, last 100 lines of console:
Running bootstrap
FATAL ERROR: Cannot set current directory to 'D:\contains%20spaces\data'

https://mariadb.com/kb/en/installation-issues-on-windows contains some help
for solving the most common problems.  If this doesn't help you, please
leave a comment in the Knowledge Base or file a bug report at
https://jira.mariadb.org
	at ch.vorburger.exec.ManagedProcess.checkResult(ManagedProcess.java:310)
	at ch.vorburger.exec.ManagedProcess.waitForExitMaxMsWithoutLog(ManagedProcess.java:448)
	at ch.vorburger.exec.ManagedProcess.waitForExit(ManagedProcess.java:416)
	at ch.vorburger.mariadb4j.DB.install(DB.java:141)
	... 3 more
Caused by: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404)
	at org.apache.commons.exec.DefaultExecutor.access$200(DefaultExecutor.java:48)
	at org.apache.commons.exec.DefaultExecutor$1.run(DefaultExecutor.java:200)
	at java.base/java.lang.Thread.run(Thread.java:829)
An error occurred while installing the database  Fehler: Der APM Server wird sich nun beenden.15:50:51,548 ERROR SpringBootstrap:1242 - An error occurred while installing the database  Fehler: Der APM Server wird sich nun beenden.

We are using v2.5.0 and are on Windows 10 with Java 11.

[vorburger]

#helpwanted from 🪟 users 😄 I won't be able to work on this issue myself, but do welcome and will review and merge PRs from the community re. this.

Perhaps this could be fixed (worked around?) in https://github.com/vorburger/ch.vorburger.exec (the library which MariaDB4j uses to launch mysqld, it's built on top of and extends org.apache.commons.exec) instead of MariaDB4j (this project) itself.

[vorburger]

The root cause of this bug seems to be that something somewhere in ch.vorburger.mariadb4j, or in ch.vorburger.exec or in org.apache.commons.exec seems to have decided that setDataDir("D:\\contains spaces") needs to be turned into an argument with "escaping", note the --datadir=D:\contains%20spaces\data. That's obviously wrong (it's a not a URL). Someone would have to have some fun to do some step through debugging and finger point to where this happens.

[vorburger]

Tip to anyone interested in investigating this further and raising a PR: There's an existing dataDirWithSpace() in MariaDB4jSampleOtherTest that's related to this. It passes on Linux, reproducing the problem described above with this test on Windows - and then fixing it, would be the way to go here for anyone wanting to contribute.

There's also some related history about this kind of stuff e.g. in old #30 and #35 as well (although back when that was about using quotes around arguments to prevent problems with spaces on Windows, whereas this issue is slightly different with the %20.

[vorburger]

Hah, I've found it! This comes from the toWindowsPath() method in class DB - which was added by @jtmuhone in #234 to fix windows datadir with spaces... it's easy enough to get rid of that again. The more interesting question is why someone 3 years ago felt it was needed, yet here it seems it's actually not needed. Calling all Windows 🪟 users of MariaDB4j to chime in here what works for you on Windows, and raise a PR...

[Blanco27]

Thanks for the useful tips. As you said, it is very easy to remove the %20 again. Unfortunately, this does not fix the problem. When the ManagerProcess tries to start the database with the following arguments

 Starting Program [C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base\bin\mysqld.exe, 
--no-defaults, --console, --skip-grant-tables, --max_allowed_packet=64M,
--basedir=C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base, --datadir="D:\contains spaces", --port=3306]

it results in the following error

C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base\bin\mysqld.exe: Too many arguments (first extra is 'spaces').
2021-12-01 16:48:25 17132 [ERROR] Aborting

Although the datadir argument was escaped correctly --datadir="D:\contains spaces"

If you execute the command above directly via console, it works just fine

mysqld.exe --no-defaults --console --skip-grant-tables --max_allowed_packet=64M
--basedir=C:\Users\nwe\AppData\Local\Temp\MariaDB4j\base --datadir="D:\contains spaces" --port=3306

Therefore, I suspect that the ManagerProcess somehow misinterprets the arguments even though they are escaped.
I'll take a closer look tomorrow

[Blanco27]

We've pinpointed the issue to behaviour from Runtime.exec. The problem is that the JDK code reescapes the already quoted key-value pair, due to the fact that it's not two arguments, but one.

A workaround is applicable in ManagedProcessBuilder.addArgument:

    protected ManagedProcessBuilder addArgument(String argPart1, String separator, String argPart2) {
        // @see MariaDB4j Issue #30 why 'quoting' (https://github.com/vorburger/MariaDB4j/issues/30)
        final StringBuilder sb = new StringBuilder();

        final String arg;
        
        //Internally Runtime.exec is being used, which says that an argument such
        //as --name="escaped value" isn't escaped, since there's no leading quote
        //and it contains a space, so it reescapes it, causing applications to split
        //it into multiple pieces, which is why we quote the whole arg (key and value) instead.
        if("=".equals(separator)) {            
            sb.append(argPart1);
            sb.append(separator);
            sb.append(argPart2);
            arg =StringUtils.quoteArgument(sb.toString());
        } else {
            sb.append(StringUtils.quoteArgument(argPart1));
            sb.append(separator);
            sb.append(StringUtils.quoteArgument(argPart2));
            arg = sb.toString();
        }
        
        // @see https://issues.apache.org/jira/browse/EXEC-93 why we have to use 'false' here
        // TODO Remove the false when commons-exec has a release including EXEC-93 fixed.
        addArgument(arg, false);
        
        return this;
    }

Would this be an acceptable solution?

[vorburger]

@Blanco27 From my side, if you have tested this and can confirm it does the trick for you on Windows (and the tests on Linux still pass, which I will verify if you cannot), then yes I'm happy to merge a PR from you proposing something like this. Please include a link to this issue inline at the top of the new code. Note that you will have to raise this as a PR in https://github.com/vorburger/ch.vorburger.exec, and not this repo (MariaDB4j). I would then release a new version of that library, and bump the dependency of this project to that project.

If anyone else reading along here (specifically e.g. @robin-xyzt-ai who recently has stepped up in this project, thank you) has objections or better ideas about this, please do chime in by commenting here.

[Blanco27]

I have now created a PR for both projects. Unfortunately the testing for Linux has to be done by you or someone else.

[vorburger]

testing for Linux has to be done by you or someone else.

The MariaDB4jSampleOtherTest.dataDirWithSpace() fails to run on Linux with an ch.vorburger.exec:exec:3.1.3-SNAPSHOT that includes vorburger/ch.vorburger.exec#58, with the following error:

[main] INFO ch.vorburger.mariadb4j.DB - Installing a new embedded database to: /tmp/MariaDB4j/base
[main] INFO ch.vorburger.exec.ManagedProcess - Starting Program [/tmp/MariaDB4j/base/scripts/mysql_install_db, "--datadir=/tmp/MariaDB4j/ch.vorburger.mariadb4j.tests.MariaDB4jSampleOtherTest dataDirWithSpace", --basedir=/tmp/MariaDB4j/base, --no-defaults, --force, --skip-name-resolve] (in working directory /tmp/MariaDB4j/base)
[Exec Stream Pumper] INFO ch.vorburger.exec.ManagedProcess - mysql_install_db: Installing MariaDB/MySQL system tables in './data' ...
[main] INFO ch.vorburger.exec.ManagedProcess - Thread is now going to wait for this process to terminate itself: Program [/tmp/MariaDB4j/base/scripts/mysql_install_db, "--datadir=/tmp/MariaDB4j/ch.vorburger.mariadb4j.tests.MariaDB4jSampleOtherTest dataDirWithSpace", --basedir=/tmp/MariaDB4j/base, --no-defaults, --force, --skip-name-resolve] (in working directory /tmp/MariaDB4j/base)
[Exec Stream Pumper] INFO ch.vorburger.exec.ManagedProcess - mysql_install_db: /tmp/MariaDB4j/base/bin/mysqld: Too many arguments (first extra is '"--datadir=/tmp/MariaDB4j/ch.vorburger.mariadb4j.tests.MariaDB4jSampleOtherTest').