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

handle new lines in docker environment variables correctly #1081

Merged
merged 1 commit into from Jul 21, 2016

Conversation

Projects
None yet
3 participants
@ssalinas
Member

ssalinas commented Jun 10, 2016

@jonathanwgoodwin was a bit trickier than I thought, but this should work

echo "Creating continer with: sudo -E -H -u {{{ runContext.user }}} docker create $DOCKER_OPTIONS $DOCKER_IMAGE {{{ runContext.cmd }}}"
cid=`sudo -E -H -u {{{ runContext.user }}} docker create $DOCKER_OPTIONS $DOCKER_IMAGE {{{runContext.cmd }}}`
echo "Creating continer with: sudo -E -H -u {{{ runContext.user }}} docker create $DOCKER_OPTIONS {{#each envContext.env}}{{#ifHasNewLines value}}-e {{{name}}}={{#escapedNewLines value}}{{/escapedNewLines}}{{/ifHasNewLines}}{{/each}} $DOCKER_IMAGE {{{ runContext.cmd }}}"
cid=`sudo -E -H -u {{{ runContext.user }}} docker create $DOCKER_OPTIONS {{#each envContext.env}}{{#ifHasNewLines value}}-e {{{name}}}={{#escapedNewLines value}}{{/escapedNewLines}}{{/ifHasNewLines}}{{/each}} $DOCKER_IMAGE {{{runContext.cmd }}}`

This comment has been minimized.

@tpetr

tpetr Jun 17, 2016

Member

i'm not sure you need closing blocks for escapedNewLines

@tpetr

tpetr Jun 17, 2016

Member

i'm not sure you need closing blocks for escapedNewLines

@tpetr

This comment has been minimized.

Show comment
Hide comment
@tpetr

tpetr Jun 17, 2016

Member

For posterity, can you explain what this PR exactly does? Seems like it adds explicit -e args for docker env vars with newlines?

Member

tpetr commented Jun 17, 2016

For posterity, can you explain what this PR exactly does? Seems like it adds explicit -e args for docker env vars with newlines?

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Jun 21, 2016

Member

Explanation for this:

When docker reads a .env file, it is not interpreted like bash as one would expect. Docker has its own parser that reads the variables in. This parser does not handle values that span multiple lines, even if they are quoted. So, any lines following a value with new lines could be messed up when read, and the value with new lines will only have the first line.

When instead using explicit -e options versus an env file, as long as they are escaped, new line values are acceptable and interpreted correctly.

What this PR does is separates out any environment variables that may contain new lines and sets them using -e instead of putting them in the env file

Member

ssalinas commented Jun 21, 2016

Explanation for this:

When docker reads a .env file, it is not interpreted like bash as one would expect. Docker has its own parser that reads the variables in. This parser does not handle values that span multiple lines, even if they are quoted. So, any lines following a value with new lines could be messed up when read, and the value with new lines will only have the first line.

When instead using explicit -e options versus an env file, as long as they are escaped, new line values are acceptable and interpreted correctly.

What this PR does is separates out any environment variables that may contain new lines and sets them using -e instead of putting them in the env file

@jonathanwgoodwin

This comment has been minimized.

Show comment
Hide comment
@jonathanwgoodwin
Member

jonathanwgoodwin commented on 6cf5f17 Jul 20, 2016

🚢

@ssalinas ssalinas added the hs_stable label Jul 20, 2016

@ssalinas ssalinas merged commit 835b49e into master Jul 21, 2016

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details

@ssalinas ssalinas deleted the new_lines branch Jul 21, 2016

@ssalinas ssalinas added this to the 0.9.0 milestone Jul 29, 2016

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