[ClusterLabs] [OCF] [RFC at agents] unresolved issue with handling parameters containing whitespaces

[Alan Robertson]

This is not a problem in the OCF API, but in the pcs command.

There is no issue with having spaces in environment variables.

It's the pcs command that's doing you dirty here...

    -- Alan Robertson
       alanr at unix.sh

On 05/12/2015 07:03 AM, 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).
>
>
>
> _______________________________________________
> OCF mailing list
> OCF at lists.community.tummy.com
> http://lists.community.tummy.com/mailman/listinfo/ocf

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://clusterlabs.org/pipermail/users/attachments/20150512/358a2fb5/attachment.html>