Skip to content

Conversation

Bujupah
Copy link
Contributor

@Bujupah Bujupah commented Apr 28, 2025

The current code is bluntly splitting the environment variable by comma for RENDERING_ARGS which can fail if we want to pass an argument with a comma in it, which as a result will cause the chromium to crash on run.

const argsList = args.split(',');

With this PR, I have added support to consider \n instead, so in case if the argument itself contains a comma, the user will have the ability to use \n as a separator instead of , in his deployment config.

if (args.includes('\n')) {
  // New style: split by newlines
  argsList = args.split('\n').map(arg => arg.trim()).filter(arg => arg.length > 0);
} else {
  // Old style: split by commas
  argsList = args.split(',').map(arg => arg.trim()).filter(arg => arg.length > 0);
}

At that time the splitting will work fine, and chromium will accept the arguments without crashing. This is backward compatible and will work only if \n is used in the environment variables as shown below

env:
  - name: RENDERING_ARGS
    value: |
      --no-sandbox
      --disable-setuid-sandbox
      --host-resolver-rules=MAP fonts.googleapis.com 127.0.0.1, MAP fonts.gstatic.com 127.0.0.1

Regards ✨

@Proximyst
Copy link
Member

Is this still required?

If so, could you please add some tests?

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

Successfully merging this pull request may close these issues.

2 participants