1499511 - [mozdevice] Support single-quotes in adb command line

Status: RESOLVED FIXED

(Testing :: Mozbase, enhancement)

Description

[Geoff Brown [:gbrown]]

In bug 1475648, we've found that the jittest command line may have single-quotes in it; currently it looks like they are lost. They should probably be escaped by _escape_command_line().

Comment 1

[Geoff Brown [:gbrown]]

My first attempt was not successful:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=fc79d067e129e83dbb152304289c0d7056d274a8

Comment 2

[Bob Clary [:bc] (inactive)]

I got it running locally and see the syntax errors but my first attempt to fix it didn't work either. Dealing with passing these commands around and getting them work in the shell may be tricky. If you get to home plate first, you win the pennant.

Comment 3

[Bob Clary [:bc] (inactive)]

Attachment: bug-1499511-escape-command-line.patch

- don't quote already quoted elements of cmd
- expand the list of quotable characters and use the regular expression
  [ ()\"&'\]]
- use " to quote

before and after android jit look good on try. doing a full android test now

<https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&author=bclary%40mozilla.com&group_state=expanded&fromchange=933c28ccafc2d094fc856739cd81ebdf72837d0c&tochange=65be8c13e7535eeacecf6b6183ad74b6d0d3bf7a>

Comment 4

[Geoff Brown [:gbrown]]

Comment on attachment 9017823 [details] [diff] [review]
bug-1499511-escape-command-line.patch

Review of attachment 9017823 [details] [diff] [review]:
-----------------------------------------------------------------

Nice. I was heading toward this type of solution last night, but couldn't quite get it to work. Your try run looks great!

I have a simple unit test for _escape_command_line() on the go; I'll attach a patch later today.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/02d89c011037
[mozdevice] Support single-quotes in adb command line, r=gbrown.

Comment 6

[Geoff Brown [:gbrown]]

Attachment: simple unit test

I'm happy to add more cases if you like.

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/02d89c011037

Comment 8

[Bob Clary [:bc] (inactive)]

Comment on attachment 9017930 [details] [diff] [review]
simple unit test

Review of attachment 9017930 [details] [diff] [review]:
-----------------------------------------------------------------

r+, lgtm.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73466d27dd00
Add unit test for adb.py _escape_command_line; r=bc

Comment 10

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/73466d27dd00