strip newline from all passed variables

[OndrejHome]

This prevents weird errors when attributes get shifted to newline and are executed by docker-entrypoint.sh. This PR removes the newline and when newline is stripped a 'Warning' is printed.

Short version: docker-entrypoint.sh scripts breaks (as seen later in examples of this PR) when 'newline' is present with rather ugly error that is hard to analyze without checking the code of script.
Better error message or automated removal of 'newline' would be more desirable. This PR proposes automated 'newline' removal.

Long version:
I have run into this when trying to get image working with kubernetes and erroneously generated 'kubernetes secret' that contained the newline at the end. From looking at the logs the error that I have made required me to do debugging of image as there was no good message on what is going on. I believe there is no practical use case for having 'newline' in any of variables (if this is not true please correct me).

After the change proposed here the mysql image would progress with only printing warning when 'newline' is included which would make this work in most cases. With kubernetes the newline is preserved at the end of string which makes this invisible to user if such mistake is done.
Alternative approach to this would be to "hard fail" with error that 'newline' was encountered in some variable as that will most probably never be desired and user should be aware of it.

Main idea for creating this PR is to avoid unnecessary image debugging when 'newline' is accidentally included in any of the variables.

Please let me know what you think about this change and also let me know there are any changes needed for this to be merged. Thank you!

How to reproduce issue:

# a="aa
bb"
# docker run --name t56 -e MYSQL_ROOT_PASSWORD="$a" -d mysql:5.6 --character-set-server=utf8mb4 --collation-server=utf8mb4_unicode_ci
xxxxxxxx
# docker logs -f xxxxxxxx

mysql:5.6

...
2019-12-08 08:18:54+00:00 [Note] [Entrypoint]: Temporary server started.
Warning: Unable to load '/usr/share/zoneinfo/iso3166.tab' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/leap-seconds.list' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/zone.tab' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/zone1970.tab' as time zone. Skipping it.
2019-12-08 08:18:56 96 [Warning] 'proxies_priv' entry '@ root@0b2ef1dce921' ignored in --skip-name-resolve mode.

2019-12-08 08:18:56+00:00 [Note] [Entrypoint]: Stopping temporary server
mysqladmin: unknown option '--bb"'
2019-12-08 08:18:56+00:00 [ERROR] [Entrypoint]: Unable to shut down server.

mysql:5.7

...
2019-12-08 08:20:09+00:00 [Note] [Entrypoint]: Temporary server started.
Warning: Unable to load '/usr/share/zoneinfo/iso3166.tab' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/leap-seconds.list' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/zone.tab' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/zone1970.tab' as time zone. Skipping it.

2019-12-08 08:20:17+00:00 [Note] [Entrypoint]: Stopping temporary server
mysqladmin: [ERROR] unknown option '--bb"'
2019-12-08 08:20:17+00:00 [ERROR] [Entrypoint]: Unable to shut down server.

mysql:8.0

...
2019-12-08 08:22:48+00:00 [Note] [Entrypoint]: Temporary server started.
2019-12-08T08:22:48.929209Z 0 [System] [MY-011323] [Server] X Plugin ready for connections. Socket: '/var/run/mysqld/mysqlx.sock'
Warning: Unable to load '/usr/share/zoneinfo/iso3166.tab' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/leap-seconds.list' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/zone.tab' as time zone. Skipping it.
Warning: Unable to load '/usr/share/zoneinfo/zone1970.tab' as time zone. Skipping it.

2019-12-08 08:22:54+00:00 [Note] [Entrypoint]: Stopping temporary server
mysqladmin: [ERROR] unknown option '--bb"'.
2019-12-08 08:22:54+00:00 [ERROR] [Entrypoint]: Unable to shut down server.

After fix the mysql can start properly and following can be seen in logs if there was any newline stripped (including the name of variable from which it was stripped so user can fix it):

2019-12-08 08:38:22+00:00 [Warn] [Entrypoint]: Stripping newline from MYSQL_ROOT_PASSWORD.

[yosifkit]

Seems to be related to these issues: MariaDB/mariadb-docker#183 and #571

While this is one possible way to handle "bad" characters and is targeted at fixing the "Option File" created by docker_process_sql (and _mysql_passfile), I'd rather not "silently" change what the user passed in. Many won't notice the log message and then wonder why they cannot connect.

I see two main alternatives to address escaping (passwords specifically*): fail on bad values or escape everything. Escaping everything will probably lead us to things like implementing mysql_real_escape_string in bash (and we'd rather not maintain that). On the other hand, early failing on any input that just needs an escape hurts user experience. I think we can meet somewhere in the middle that will allow most character sequences, while failing on more complex ones without maintaining a full mysql_real_escape_string in bash.

⬆️ WDYT @tianon and @ltangvald? Maybe something like wordpress uses when escaping things for sed?

---

Side note: MariaDB/mariadb-docker#183 discussed printf %q, but that was only in the context of SQL escaping and may not be enough since the password in the "option file" is interpreted differently than in SQL (https://dev.mysql.com/doc/refman/8.0/en/option-files.html#option-file-syntax).

* I say passwords specifically since most other environment variables like user or database are unlikely to contain characters that need escaping (and have accidental "features" #338).

[OndrejHome]

Hi @yosifkit, Thanks for feedback.

I see two main alternatives to address escaping (passwords specifically*): fail on bad values or escape everything.

For simplicity I would suggest 'failing on bad values' and letting the user know what bad values are. If the container fails to start and last message in its logs looks like [ERROR] [Entrypoint]: variable MYSQL_ROOT_PASSWORD contains restricted character(s) like '$/\J\n...' then I would expect user to either "brute-force" what they can/cannot put into variable or complain that their favourite special thing is not allowed (which can lead into issue that would eventually add character if considered viable). In both of these cases it would lead to improvement in user experience as more things will get allowed. For backward-compatibility this can be done for example only for one type of variables (either ones passed via 'env' or ones from 'file').
If interested let me know and I can draft code example for above idea.

[ltangvald]

I was going to suggest printf %q (we used it in e.g. the upstream debian packages to escape input during the installation), but if you give it a multiline input it wraps it in $'', which causes a syntax error in SQL

[ltangvald]

I like the idea of doing input validation, i.e. fail as it does now, but with a more helpful error message.

[tianon]

That sounds like fun -- is there somewhere we can reference for what the "canonically supported" set of characters / restrictions are? 😄

[ltangvald]

There is a list of special characters here https://dev.mysql.com/doc/refman/8.0/en/string-literals.html
But detecting them reliably might be a bit tricky. We could maybe use printf %q to escape the "normal" ones, and try to just detect tricker cases like newlines?