Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escaped double-quotes in addArguments don't get de-escaped #194

Open
mfreed7 opened this issue Sep 20, 2020 · 2 comments
Open

Escaped double-quotes in addArguments don't get de-escaped #194

mfreed7 opened this issue Sep 20, 2020 · 2 comments

Comments

@mfreed7
Copy link

mfreed7 commented Sep 20, 2020

It appears that double-quotes in the command line aren't being properly de-escaped when passed to the chrome process. See the example below. Using escaped double quote (\") seems to pass exactly that (\") to the chrome process, rather than just the raw double-quote ("). I've tried swapping to single quotes (') for the outer set around the arguments, but that is a syntax error.

  "benchmarks": [
    {
      "name": "DARK MODE",
      "url": "http://google.com",
      "measurement": "fcp",
      "browser": {
        "name": "chrome",
        "addArguments": ["--blink-settings=forceDarkModeEnabled=true"],
        "windowSize": {"width": 800,"height": 600}
      }
    },
    {
      "name": "SHOULD BE DARK MODE BUT ISN'T",
      "url": "http://google.com",
      "measurement": "fcp",
      "browser": {
        "name": "chrome",
        "addArguments": ["--blink-settings=\"forceDarkModeEnabled=true\""],
        "windowSize": {"width": 800,"height": 600}
      }
    },
    {
      "name": "SYNTAX ERROR",
      "url": "http://google.com",
      "measurement": "fcp",
      "browser": {
        "name": "chrome",
        "addArguments": ['--blink-settings="forceDarkModeEnabled=true"'],
        "windowSize": {"width": 800,"height": 600}
      }
    }
  ]
@aomarks
Copy link
Member

aomarks commented Sep 21, 2020

This might be WAI? I think there's no shell involved here, so each item in the array is just passed through directly to the process as its own argv item. So for example, if you want to pass an argument with spaces in it, there should be no quoting required.

This is actually handled by the webdriver library (not to say that means it's correct, just FYI this is how using webdriver directly would work too).

Is there something you can't achieve with the current behavior, or is it more that it's confusing that you can't always copy/paste to/from a commandline shell?

@mfreed7
Copy link
Author

mfreed7 commented Sep 28, 2020

This might be WAI? I think there's no shell involved here, so each item in the array is just passed through directly to the process as its own argv item. So for example, if you want to pass an argument with spaces in it, there should be no quoting required.

This is actually handled by the webdriver library (not to say that means it's correct, just FYI this is how using webdriver directly would work too).

Is there something you can't achieve with the current behavior, or is it more that it's confusing that you can't always copy/paste to/from a commandline shell?

Thanks! The problem is that it seems impossible to pass arguments that contain a " in them. E.g. if I want to call Chrome like this:

chrome --blink-settings="forceDarkModeEnabled=true"

...I don't seem to be able to do that. I can't use a naked " in the JSON file, because that ends the value. And if I use an escaped \" then the escape backslash gets passed through, and not understood by Chrome:

chrome --blink-settings=\"forceDarkModeEnabled=true\"

If I wanted to pass that as the actual command line, I would have expected to need to double-escape it, like this, in the JSON:

        "addArguments": ["--blink-settings=\\\"forceDarkModeEnabled=true\\\""],

Note that most command line arguments don't need quotes, because the stuff inside the quotes contains no spaces. But some documentation (e.g. here) shows command line parameters with the quotes, and when escaped, they are silently ignored. I believe those can be replaced with single quotes (e.g. --renderer-cmd-prefix='xterm -e gdb --args'), but the main point is the silent ignoring of anything with \". The reason for this bug is that this situation is not great to discover after leaving your experiment running overnight, as I did, and then find that all experiments have identical results because all command lines were ignored.

Perhaps there should just be a warning or something? Feel free to disregard this bug, it is clearly low priority.

@aomarks aomarks moved this to Todo in Lit Project Board Jan 24, 2022
@aomarks aomarks moved this from 🔥 Front Burner to 📋 Triaged in Lit Project Board Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants