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

second-instance event process arguments rip apart "name-value" arguments #20322

Closed
3 tasks done
siebertm opened this issue Sep 23, 2019 · 7 comments
Closed
3 tasks done

Comments

@siebertm
Copy link

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

When I run an electron app with requestSingleInstanceLock() and some command line arguments, in electron 6 (and maybe even 5), it seems that the --allow-file-access-from-files and --original-process-start-time=13213718723637733 arguments are inserted anywhere in the argv array. The original arguments are preserved but when we have applications which expect key-value type arguments, these arguments will be split, resulting in invalid values.

A simple test case is actually contained in the electron repository: https://github.com/electron/electron/blob/6-0-x/spec/fixtures/api/singleton/main.js

const { app } = require('electron')

app.once('ready', () => {
  console.log('started') // ping parent
})

const gotTheLock = app.requestSingleInstanceLock()

app.on('second-instance', (event, args) => {
  setImmediate(() => {
    console.log(JSON.stringify(args))
    app.exit(0)
  })
})

if (!gotTheLock) {
  app.exit(1)
}

When we now start the second instance with 2 arguments: ["--param", "value"], the second-instance event will fire with the following arguments:

[
  "path\to\electron.exe",
  "--param",
  "--allow-file-access-from-files",
  "--original-process-start-time=13213718723637733",
  "value"
]

The expected behaviour would be to keep the arguments together, like:

[
  "path\to\electron.exe",
  "--allow-file-access-from-files",
  "--original-process-start-time=13213718723637733",
  "--param",
  "value"
]

In the spec spec-main/api-app-spec.ts:222, the test successfully validates exactly that behaviour, but that may be wrong. the arguments could be appended or prepended, but should not be inserted somewhere in the middle...

  • Electron Version:

    • v6.0.9
  • Operating System:

    • Windows 10 (insiders, Version 10.0.18975.1000)
  • Last Known Working Electron version:

    • 4.x
@nornagon
Copy link
Member

This does seem surprising, but I think it's intended behavior from Chromium's command-line parsing logic. Chromium treats "switches" (things like --foo or --foo=bar) and "arguments" (things that don't begin with --) separately. When it assembles a command line, it always moves the switches to the beginning and the arguments to the end. See https://chromium.googlesource.com/chromium/src/+/12915a6957ebe9ca20ef30c0e5141ad1dee87a7d/base/command_line.cc#88. I imagine you'll find that if you pass "--param1 value1 --param2 value2" you'll receive "--param1 --param2 value1 value2".

I'm not sure it's worth messing with the way Chromium handles command-line parsing in order to change this behavior. Would it be possible for you to use the --param=value syntax instead?

@siebertm
Copy link
Author

Thanks for the reply and clarification (and especially the chromium link, I couldn't find where that happened)

I'm not sure it's worth messing with the way Chromium handles command-line parsing in order to change this behavior

Definitely not worth it! It'll be just a brittle hack and will break all the time, causing issues regularly.

Would it be possible for you to use the --param=value syntax instead?

Not without seriously upsetting our customers 😥

Is there some other way to send kind of an argument to the second instance instead? Technically, I just need one or two flags passed from the second instance to the "main" one...

To me this looks like a bug in chromium, I'll take a closer look tomorrow

@paul-marechal
Copy link

paul-marechal commented Nov 4, 2019

@siebertm Just shimming in, but would it be worth it to wrap the CLI to your electron-app with something that would do the conversion of arguments? Anything like --key value would be passed as --key=value to your app.

@nornagon
Copy link
Member

nornagon commented Nov 6, 2019

If you wrote a wrapper script, you could also just pass --args-from-cli="--whatever you --received-on the-command-line" and extract those in the main process.

I'm closing this for now since there's no action for us to take here, but feel free to continue discussion in this issue.

@nornagon nornagon closed this as completed Nov 6, 2019
@xmedeko
Copy link

xmedeko commented Apr 22, 2020

A workaround (hack) is to exploit app.commandLine:

const { app } = require("electron");
let argv = process.argv;
app.commandLine.appendSwitch("second-instance" , JSON.stringify(process.argv)); 
if (!app.requestSingleInstanceLock()) {
    return;
} else {
    app.on("second-instance", function(event, argvChromium, cwd) {
        // argv = JSON.parse(event.sender.commandLine.getSwitchValue("second-instance")); // does not work ...
        // parse from argvChromium
        const argStart = "--second-instance=";
        let argv = argvChromium.filter(a => a.startsWith(argStart))[0];
        if (argv) {
            argv = argv.substr(argStart.length);
            argv = JSON.parse(argv);
        }
        // and here we go with argv ...
}

@alexNecroJack
Copy link

Another workaround, using additionalData:

//Get your args and pass them when you call app.requestSingleInstanceLock
var argv = require("minimist")(process.argv.slice(2));
var isFirstInstance = app.requestSingleInstanceLock({
  argv: argv,
});

Then consume argv in your first instance

if (!isFirstInstance) {
  app.quit(); //Application already running ?
} else {
  //First instance detects second instance
  app.on("second-instance", (event, commandLine, workingDirectory, additionalData) => {
    console.log("additionalData.argv: ", additionalData.argv);
    // ...
  }
  // ...
}

Also, thanks xmedeko for your snippet!

@xmedeko
Copy link

xmedeko commented Jun 20, 2022

@alexNecroJack This issue (and my workaround) was about problems in Electron 15 and lower when "second-instance" event has no additionalData param. You are using additionalData already, so your workaround is about a bit different topic.

mifi added a commit to mifi/electron that referenced this issue Dec 23, 2022
codebytere pushed a commit that referenced this issue Jan 5, 2023
Add note about argv getting modified

See #20322
trop bot added a commit that referenced this issue Jan 5, 2023
See #20322

Co-authored-by: Mikael Finstad <finstaden@gmail.com>
jkleinsc pushed a commit that referenced this issue Jan 5, 2023
Add note about argv getting modified

See #20322

Co-authored-by: Mikael Finstad <finstaden@gmail.com>

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Mikael Finstad <finstaden@gmail.com>
khalwa pushed a commit to solarwindscloud/electron that referenced this issue Feb 22, 2023
gecko19 pushed a commit to brightsign/electron that referenced this issue Feb 28, 2023
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

5 participants