[ClusterLabs] [RFC at agents] unresolved issue with handling parameters containing whitespaces
Vladislav Bogdanov
bubble at hoster-ok.com
Wed May 13 08:35:01 UTC 2015
Hi
what you see is a bug in mysql-common.sh
It uses shell tests on unquoted variables:
if [ ! -f $OCF_RESKEY_config ]; then
ocf_exit_reason "Config $OCF_RESKEY_config doesn't exist";
return $OCF_ERR_INSTALLED;
fi
if [ ! -d $OCF_RESKEY_datadir ]; then
ocf_exit_reason "Datadir $OCF_RESKEY_datadir doesn't exist";
return $OCF_ERR_INSTALLED;
fi
That should look like
if [ ! -f "$OCF_RESKEY_config" ]; then
ocf_exit_reason "Config $OCF_RESKEY_config doesn't exist";
return $OCF_ERR_INSTALLED;
fi
if [ ! -d "$OCF_RESKEY_datadir" ]; then
ocf_exit_reason "Datadir $OCF_RESKEY_datadir doesn't exist";
return $OCF_ERR_INSTALLED;
fi
May be the same should be done in other places too.
It is the safest position to quote variables every time you use them
unless you know for sure that in the specific case var should be used
unquoted.
Best,
Vladislav
12.05.2015 16:03, Jan Pokorný wrote:
> Aloha,
>
> recently I realized one of those rough edges that probably were never
> run into (per quick ML seach) but are lurking to bite anyone that will
> leave the safe, well-tested paths.
>
> What I have in mind in particular is unability to propagate parameters
> participating directly in underlying resource execution as command's
> arguments[*] (referred to as "argv items" for clarity) when they contain
> (white)spaces (per IFS variable?) due to (intentionally?) preserving them
> as-are in the majority of shell scripts constituting resource agents
> (will leave fence agents for a follow-up now):
>
> 1. no wrapping them with quotes just for the sake of protecting against
> word-splitting/preserving a _single_ argv item (eventually handled
> in unquoted form, see the next point) to cover cases with space
> contained within
>
> 2. no unwrapping in case the user-provided parameter already is wrapped
> with {single,double} quotes so as to denote "this is to be treated
> as a single argv item" explicitly from user's input (somewhat
> intuitive workaround attempt if one is familiar with shell and/or
> quoting rules in some configuration files)
>
> [*] not exclusively, but these parameters are most exposed to the problem
> at hand
>
>
> To demonstrate the problem in a real-world scenario:
>
> - I want to run ocf:heartbeat:mysql resource in my cluster
>
> - I am a rebel going against best practices, hence my configuration
> file for this mysql instance is "/mnt/foo/mysql/config final.cnf"
> (notice the space)
>
> - so I go ahead and type:
>
> # pcs resource create mysql-test1 ocf:heartbeat:mysql \
> 'config=/mnt/foo/mysql/config final.cnf'
> (quotes present so as to pass the whole config=... argument at once,
> otherwise the same word-splitting problem as mentioned would be
> present)
>
> # pcs resource debug-start --full mysql-test1 \
> | grep /mnt/foo/mysql/config
> > stderr: ++ 06:47:57: 52: : /mnt/foo/mysql/config final.cnf
> > stderr: + 06:47:57: mysql_common_validate:108: \
> > '[' '!' -f /mnt/foo/mysql/config final.cnf ']'
> ^^^
> OOPS#1a
> > stderr: /usr/lib/ocf/lib/heartbeat/mysql-common.sh: line 108: \
> > [: /mnt/foo/mysql/config: binary operator expected
> ^^^
> OOPS#1b
> > stderr: + 06:47:58: mysql_common_start:209: \
> > /usr/bin/mysqld_safe \
> > --defaults-file=/mnt/foo/mysql/config final.cnf \
> ^^^
> OOPS#2a
> > --pid-file=/var/run/mysql/mysqld.pid \
> > --socket=/var/lib/mysql/mysql.sock --datadir=/var/lib/mysql
> > --log-error=/var/log/mysqld.log --user=mysql
>
> # grep '/mnt/foo/mysql/config' /var/log/mysqld.log
> Could not open required defaults file: /mnt/foo/mysql/config
> ^^^
> OOPS#2b
>
> - well, no good, let's try a workaround sketched in point 2. above:
>
> # pcs resource create mysql-test2 ocf:heartbeat:mysql \
> 'config="/mnt/foo/mysql/config final.cnf"'
> (outer singlequotes so as to preserve inner doublequotes)
>
> # pcs resource debug-start --full mysql-test2 \
> | grep '/mnt/foo/mysql/config'
> > stderr: ++ 07:06:44: 52: : '"/mnt/foo/mysql/config' 'final.cnf"'
> > stderr: + 07:06:44: mysql_common_validate:108: \
> '[' '!' -f '"/mnt/foo/mysql/config' 'final.cnf"' ']'
> ^^^
> OOPS#1a
> > stderr: /usr/lib/ocf/lib/heartbeat/mysql-common.sh: line 108: \
> [: "/mnt/foo/mysql/config: binary operator expected
> ^^^
> OOPS#1b ???
> > stderr: + 07:06:45: mysql_common_start:209: /usr/bin/mysqld_safe \
> '--defaults-file="/mnt/foo/mysql/config' 'final.cnf"' \
> ^^^
> OOPS#2a
> --pid-file=/var/run/mysql/mysqld.pid \
> --socket=/var/lib/mysql/mysql.sock --datadir=/var/lib/mysql \
> --log-error=/var/log/mysqld.log --user=mysql
>
> # grep '/mnt/foo/mysql/config' /var/log/mysqld.log
> Could not open required defaults file: /root/"/mnt/foo/mysql/config
> ^^^
> OOPS#2b
>
>
> Possible solutions:
>
> A. claim whitespace as invalid character in agents' parameters
> o seems pretty dull solution
> - we shouldn't limit the domain that hasn't been limited like that
> from the beginning
>
> B. smart quoting logic per 1. on the Pacemaker side to accommodate for
> shell scripts deficiencies causing the stated issues
> (like whitespace as a proper radix of parameter -> add quotes)
> o non-systemic
> - no guarantee all shell-based agents are broken in the same way
> - would almost certainly break non-shell-based agents
>
> C. fix the shell-based resource agents
>
> o C0. use doublequotes whenever suitable (like the tests denoted with
> OOPS#1 above) in the shell code
>
> o C1. (optionally) smart quoting logic per 1. directly in the agents
> - otherwise, a convention would have to be introduced that user
> has to quote the parameters on her own when configuring the
> cluster -- suboptimal as there would be artificial distinction
> betweeb shell-based RAs and the rest (if any)
>
> o C2. instead of invoking the underlying commands:
> $exec $param1 $param2
> use:
> eval $exec $param1 $param2
> to ensure that the quoting from C1. will be taken into account
> properly and not passed as part of argv items
> - together with C1., this would fix OOPS#2
> - possibly, there could also be OCF helper's level function to unify
> the approach
>
>
> What the community thinks? I hope this is not too crazy :-)
> Let's get rid of these remaining rough edges.
>
>
> Some related bugs, these are against pcs but other management tools may
> be similarly affected (or be using [influential] approaches already):
> - https://bugzilla.redhat.com/show_bug.cgi?id=1219574
> [gui] resource optional arguments: quoted strings missing
> - https://bugzilla.redhat.com/show_bug.cgi?id=1220778
> [RFC] should pcs escape double-quotes to prevent clashes/irregularity with
> string-denoting ones in parameter printouts?
>
>
> P.S. OCF list CC'd only for posterity and to remind its audience that the
> other lists (developers and users at ClusterLabs) have become
> preferred venues to discuss these things (this in particular
> is a better fit for the former, admittedly).
>
>
>
> _______________________________________________
> Users mailing list: Users at clusterlabs.org
> http://clusterlabs.org/mailman/listinfo/users
>
> Project Home: http://www.clusterlabs.org
> Getting started: http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> Bugs: http://bugs.clusterlabs.org
>
More information about the Users
mailing list