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

cli issue - Valid devcontainer.metadata can corrupt compose override file #904 #928

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gauravsaini04
Copy link
Contributor

@gauravsaini04 gauravsaini04 commented Nov 6, 2024

Ref: #904

Description

Changelog

  • Changed src/spec-node/dockerCompose.ts file's Line number 560, wrapped the key value pairs in double quotes so they be considered single entity, rather the : separating them into two entities earlier as discussed in the issue at
    ROSCONSOLE_FORMAT: "[$${severity}] [$${walltime:%Y-%m-%d %H:%M:%S}] [$${node}]: $${message}"

Checklist

  • Checked that applied changes work as expected

@gauravsaini04 gauravsaini04 requested a review from a team as a code owner November 6, 2024 13:04
@Mathiyarasy Mathiyarasy reopened this Nov 12, 2024
@@ -557,7 +557,7 @@ while sleep 1 & wait $$!; do :; done", "-"${userEntrypoint.map(a => `, ${JSON.st
init: true` : ''}${user ? `
user: ${user}` : ''}${Object.keys(env).length ? `
environment:${Object.keys(env).map(key => `
- ${key}=${env[key]}`).join('')}` : ''}${mergedConfig.privileged ? `
- "${key}=${env[key].replace(/"/g, '\\"')}"`).join('')}` : ''}${mergedConfig.privileged ? `
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the resulting value? Should we put the env[key] in quotes if the quotes the in the values are escaped.

Is it possible to add tests for this scenario?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are additional characters to escape in a double quoted string. I might be easier to use a single quoted string, though I'm not sure how a newline would be represented: https://www.yaml.info/learn/quote.html

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.

4 participants