------------------------------------------------------------------------------ To reply, visit https://hellosplat.com/s/beanbag/tickets/4845/ ------------------------------------------------------------------------------
New ticket #4845 by amiryal For Beanbag, Inc. > Review Board Status: New Tags: Priority:Medium, Type:Defect ------------------------------------------------------------------------------ Can’t pass arguments to rbt alias in a safe manner ============================================================================== # What version are you running? RBTools 1.0.2 # What's the URL of the page containing the problem? https://github.com/reviewboard/rbtools/commit/fb7e8d33182797bc681b17040d36cdc9a202c504 # What steps will reproduce the problem? 1. Define these aliases in `.reviewboardrc`: ```python ALIASES = { 'shell': "!rbt test '--summary=Hello World'", 'no-shell': "test '--summary=Hello World'", } ``` Note: `'--summary=Hello World'` is a surrogate for `shlex.quote("--summary={}".format(user_controlled_data))`. 2. Dry-run the aliases: ``` $ rbt alias --dry-run shell | sed -e 's/ //g' -e 's/""/ /g' rbt test --summary=Hello World $ rbt alias --dry-run no-shell rbt test "'--summary=Hello World'" ``` # What is the expected output? What do you see instead? The expected behaviour is to pass the summary as exactly one argument to the spawned process, and that the spawned process will not see the quotation marks surrounding the argument. This is how the expected behaviour shold look like: ``` $ rbt alias --dry-run shell | sed -e 's/ //g' -e 's/""/ /g' rbt test '--summary=Hello World' $ rbt alias --dry-run no-shell rbt test '--summary=Hello World' ``` # What operating system are you using? Linux, Python 3.7 # Please provide any additional information below. In the first example (`rbt shell`), the arguments effectively go through: ```python ' '.join(shlex.split(alias, posix=True) ``` This is incorrect because the inverse of `shlex.split` is `' '.join(map(shlex.quote))`: ```python ' '.join(map(shlex.quote, shlex.split(alias, posix=True))) ``` In the second example (`rbt no-shell`), the arguments effectively go through: ```python [RB_MAIN] + shlex.split(alias, posix=False) ``` The problem here is that `shlex.split(posix=False)` treats quotation marks in a counterproductive way. I honestly don’t understand what the use case for this mode is, and its documentation also fails to explain it. Maybe someone can dig it up from a PEP, or something. The default `$EFS` in the shells that I tested with is `" \t\n"`, so I can probably use the `no-shell` variant with something like: ```python def shell_quote(s): return (s .replace(' ', r'\ ') .replace('\t', r'\t') .replace('\n', r'\n')) ``` However, I would really like to _not_ reinvent anything and just use `shlex.quote` on my user-controlled data. ------------------------------------------------------------------------------ -- You received this message because you are subscribed to the Google Groups "reviewboard-issues" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard-issues/20190926081407.3737.63228%40ip-10-1-54-209.ec2.internal.
