[BUGFIX] escape database-username and -password in options-file created by mysql-command (branch latest)

[brandung-sjorek]

This PR applies to your latest-branch, other PRs for your other active branches follow.

Issue: Database-credentials are not properly escaped when writing the --defaults-extra-file. Usernames or passwords with backslashes (\) and double-quotes (") won't work.

Solution: use addslashes() to escape username and password.

• @see https://dev.mysql.com/doc/refman/5.7/en/option-files.html
• @see https://dev.mysql.com/doc/refman/8.0/en/option-files.html

Leading and trailing spaces are automatically deleted from option names and values.

You can use the escape sequences \b, \t, \n, \r, \\, and \s in option values to represent the backspace, tab, newline, carriage return, backslash, and space characters. In option files, these escaping rules apply:

A backslash followed by a valid escape sequence character is converted to the character represented by the sequence. For example, \s is converted to a space.

A backslash not followed by a valid escape sequence character remains unchanged. For example, \S is retained as is.

The preceding rules mean that a literal backslash can be given as \\, or as \ if it is not followed by a valid escape sequence character.

Hence, the MySQL option-files escaping-rules are not 100% addslashes() compatible, but using addslashes() solves 99% of the escaping-issues for now.

Cheers & Thanks,
Stephan

[mbrodala]

Hm, evidently the backtick is not covered:

echo addslashes('`'); // `

Since this is one of the usecases you mentioned, it won't be fixed with this change alone ...

[brandung-sjorek]

@mbrodala The single tick does not need to be escaped … I wrote double-tick (``), but meant double-quote ("). I updated the description …

[brandung-sjorek]

@mbrodala Maybe we should simply str_replace(['\\', '"'], ['\\\\', '\\"'], $value) ?

[helhum]

Thanks for the PRs!

Hm, evidently the backtick is not covered:

Since this is one of the usecases you mentioned, it won't be fixed with this change alone ...

This PR applies to your latest-branch, other PRs for your other active branches follow.

Issue: Database-credentials are not properly escaped when writing the --defaults-extra-file. Usernames or passwords with backslashes () and double-ticks (") won't work.

In this little thread there already is confusion which characters need to be escaped and which not.

If I get the docs right, then only backslashes need to be escaped, because they might start a vaild escaping sequence and quotes must be escaped, because the value can be enclosed in single or double quotes.

Since we enclose the value with double quotes already, we must encode backslashes and double quotes
and only those two. Using addslashes is therefore wrong, because it would encode single quotes and therefore break any password using single quotes.

Correct would be:

$userDefinition = sprintf('user="%s"', addcslashes($this->dbConfig['user'], '\\"'));

Hence, the MySQL option-files escaping-rules are not 100% addslashes() compatible, but using addslashes() solves 99% of the escaping-issues for now.

As argued above: If encoding is applied then it must be 100% correct, otherwise it is as buggy as before or sometimes even more harmful

[helhum]

See comments in the PR how this needs to be changed

[brandung-sjorek]

@helhum I got something wrong, and you're right. It's not about proper support for escape-sequences - addcslashes()would make it even worse, as it encodes much more, than mysql supports here. The solution is just to escape the double-quote and the backslash.

[brandung-sjorek]

I'll update my PR this afternoon …

[brandung-sjorek]

After reading the docs regarding addcslashes()'s second parameter I finally went for @helhum 's proposed solution using sprintf('user="%s"', addcslashes($this->dbConfig['user'], '\\"'));. My assumption that using addcslashes() encodes more than we need, was not correct, @helhum was right from the beginning … I updated the PR accordingly.

[brandung-sjorek]

One more question: Are going to accept the other two related PR's too?

• [BUGFIX] escape database-password in arguments for mysql-related commands (branch 4.x) #968
• [BUGFIX] escape database-username and -password in options-file created by mysql-command (branch 5.x) #967

I would appreciate this very much …

[helhum]

Thanks!

[brandung-sjorek]

Thank you! …