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

[Jan Pokorný]

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).

-- 
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.clusterlabs.org/pipermail/developers/attachments/20150512/5040a5b3/attachment-0003.sig>