[ClusterLabs] [RFC at agents] unresolved issue with handling parameters containing whitespaces
Dejan Muhamedagic
dejanmm at fastmail.fm
Fri May 15 17:54:40 UTC 2015
Hi,
On Wed, May 13, 2015 at 11:35:01AM +0300, Vladislav Bogdanov wrote:
> 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.
Quite right. And in particular when it's user's input. If
somebody opens an "issue" at github.com, there's some chance of
this being fixed.
I wonder if there's a lint for shells...
Cheers,
Dejan
> 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
> >
>
>
> _______________________________________________
> 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