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

Between 0.46.0-beta1 and 0.46.0 a change to Windows parsing of "chromium-args" in package.json truncates it to a length of MAX_PATH (260 characters). #7670

Open
sirisian opened this issue Feb 4, 2021 · 3 comments

Comments

@sirisian
Copy link

sirisian commented Feb 4, 2021

NWJS Version : 0.46.0+
Operating System: Only Windows

Expected behavior

"chromium-args" of any length should be parsed.

"chromium-args": "--user-data-dir='./chromium-config' --use-fake-ui-for-media-stream --disable-site-isolation-trials --autoplay-policy=no-user-gesture-required --enable-file-cookies --ignore-certificate-errors --allow-file-access-from-files --allow-http-background-page --allow-external-pages --allow-running-insecure-content --allow-hidden-media-playback --disable-user-media-security --register-pepper-plugins='./node_modules/mpv.js/build/Release/mpvjs.node;application/x-mpvjs'"

(I know some of those flags are no longer valid, just an example).

Actual behavior

The "chromium-args" truncates and only seems to process the first 260 characters silently ignoring the rest. This can cause very unexpected and subtle bugs when upgrading past 0.46.0-beta1.

How to reproduce

Add in a bunch of flags like I have and it'll ignore the ones after 260 characters.

Refer to the changes made to "src/nw_package.cc" in this diff:

nw-v0.46.0-beta1...nw-v0.46.0

The following code was added that I think caused this:

  // Expand Windows psudovars (ex; %APPDATA%) passed in chromium-args in the same way as when 
  // passed as command line parameters.
  #if defined(OS_WIN)
  TCHAR szEnvPath[MAX_PATH];
  std::wstring ws; 
  ws.assign( args.begin(), args.end());
  if (ExpandEnvironmentStrings(ws.c_str(), szEnvPath,MAX_PATH) != 0) {
    std::wstring ws_out = szEnvPath;
    args = std::string(ws_out.begin(), ws_out.end());
  } else {
    ReportError("Failed to expand chromium args",args.c_str());
  }
  #endif

One could just naively replace MAX_PATH to a larger number like 10000 or correctly handle this by growing an array with the value returned by ExpandEnvironmentStrings?

@TheRealDannyyy
Copy link

TheRealDannyyy commented Feb 10, 2021

This is also affecting some projects of mine. Would appreciate a review and merge of the PR @rogerwang.

@ayushmanchhabra
Copy link
Contributor

@sirisian I looked at the PR related to this. Should we instead have a flag which dynamically sets the character limit higher than 260 with an upper limit of 32,000? Let's say the user's chromium-args character length is 268 which is only 8 above the limit. It probably wouldn't make sense to allocate all that extra memory if it isn't going to be used (forgive me if I misunderstood the way memory is handled)? Or we could instead keep the limit of 260 characters and add a note about it in the documentation.

cc @rogerwang

@Arzenar
Copy link

Arzenar commented Oct 18, 2022

It would be great to increase it dynamically as many use nw.js for games or other GPU and media related things. There are many Chromium args that support this type of use case. Now if it stays at a size of 260, that would be extremely obstructive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants