ENV escaping behavior is confusing

[elibarzilay]

(This came up when I tried to construct dockerfile programmatically.)

It looks like the parsing of ENV lines is a rough hack. Specifically, it seems that:

• If the line ends, no error is thrown if a variable is mid-value: ENV VAR="1234 results in VAR being set to 1234.

• Backslashes are implemented as half-quote-chars, applicable only for double quotes and dollar signs? What I see is that

  • ENV VAR="12\"34" works as expected,
  • ENV VAR="12\34" has a backslash between the 2 and the 3,
  • ENV VAR="12\\34" has two backslashes.

• As a result, you cannot use ENV to have a backslash before a $variable or at the end of the string. For dockerfile-generation tools, the implication is usually more severe, they should just avoid backslashes completely (or become very complicated).

• In addition, it looks like there is no way to have a newline in the string value?

Is there a way to get some consistent quoting scheme? (I expected some json-like syntax, but looks like there isn't one?)

Given all of these, it would be nice if the parsing got cleaned up to make backslashes be proper escape characters similarly to how they're treated in (double-quoted) shell strings, which would imply throwing an error for \x with an unknown x, which in turn would make it possible to parse \n as a newline.

But I'm sure that you'd worry about backward compatibility so the minimum that could be done with this is clarify these limitations in the reference page.

[justincormack]

We should have a JSON form.

…

On 27 Mar 2017 15:09, "Eli Barzilay" ***@***.***> wrote: (This came up when I tried to construct dockerfile programmatically.) It looks like the parsing of ENV lines is a rough hack. Specifically, it seems that: - If the line ends, no error is thrown if a variable is mid-value: ENV VAR="1234 results in VAR being set to 1234. - Backslashes are implemented as half-quote-chars, applicable only for double quotes and dollar signs? What I see is that - ENV VAR="12\"34" works as expected, - ENV VAR="12\34" has a backslash between the 2 and the 3, - ENV VAR="12\\34" has two backslashes. - As a result, you cannot use ENV to have a backslash before a $variable or at the end of the string. For dockerfile-generation tools, the implication is usually more severe, they should just avoid backslashes completely (or become very complicated). - In addition, it looks like there is no way to have a newline in the string value? Is there a way to get some consistent quoting scheme? (I expected some json-like syntax, but looks like there isn't one?) Given all of these, it would be nice if the parsing got cleaned up to make backslashes be proper escape characters similarly to how they're treated in (double-quoted) shell strings, which would imply throwing an error for \x with an unknown x, which in turn would make it possible to parse \n as a newline. But I'm sure that you'd worry about backward compatibility so the minimum that could be done with this is clarify these limitations in the reference page. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#32140>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAdcPEaLudYJCgtVKEYwg7KEFXwRuZtKks5rp8MngaJpZM4MqUej> .

[vdemeester]

/cc @tonistiigi @tiborvass @duglin @simonferquel

[cpuguy83]

Tried to add a JSON form some time ago, but it was rejected since we didn't want to have more json in the Dockerfile.

[dnephin]

it looks like there is no way to have a newline in the string value?

I think you can use single quotes.

Tried to add a JSON form some time ago, but it was rejected

Maybe we can try again? I would LGTM that PR.

[vdemeester]

@elibarzilay I renamed the issue to be more clear about the actual issue, which is on ENV 👼

[elibarzilay]

@dnephin: I tried single quotes -- looks like they're more predictable, but still no newlines. Using

FROM ubuntu
ENV FOO='x
y'

I get Error response from daemon: Unknown instruction: Y'. I even tried the bash-like $'x\ny' thing, and that didn't work also (with more questionable behavior: no complaint about the $).

Is there something else I'm missing?

@vdemeester: well, "confusing" sounds fine -- but in general I was referring to the fact that such parsing results are something you get from a naive "regexp-based parsing"...

[dnephin]

@elibarzilay it's not regex-based parsing. The implementation is in builder/dockerfile/shell_parser.go.

When this was written there may not have been any good options for shell parsing libraries. Now there are a few, and we even use a couple already. It might be a good idea to use a more complete library, which could fix some of these issues. Although I suspect we'll have to fork them to implement the escapeToken hack that was added for windows Dockerfiles.

[duglin]

The env var processing was based on what we see from the shell. For example:

$ foo="12\34" env | grep foo
foo=12\34

is what I see from bash and sh.

And for \n:

$ foo="12\n34" env | grep foo
foo=12\n34

so I'm not sure there's an issue.

[duglin]

I take that back, there might be an issue on double-\ :

$ foo="12\\34" env | grep foo
foo=12\34

I can take a look at that after CNCFcon this week.

[dnephin]

@duglin I think the "newline in single quotes" example in #32140 (comment) is relevant as well?

[elibarzilay]

@duglin: yes, backslash in a double-quoted string is left alone if it's followed by something that it doesn't know about. The relevant part in the man page is:

The backslash retains its special meaning only when followed by one of the following characters: $, `, ", \, or <newline>.

And the docker parser doesn't do that. Worse, if a double-quoted string is not closed, there is no complaint which can make it much more confusing (bash does throw an error in such cases).

Fixing this is probably relatively easy, but if mimicking a shell is the target, then it should also do the proper newline thing and allow newlines in quoted strings. That might be more difficult if that parsing code receives the whole string -- but without that there's no way to have a newline in a string, which is why newer shells added the $'...' thing. Adding that is probably not too difficult: AFAICT, you do almost the same thing as '...', except that \s do quote what comes after them (including a '), known \x sequences are converted as listed in the man page, and other \xs are left as-is.

[duglin]

I'd like to address these issues slowly to ensure we don't go too far too fast. So, PR #32328 addresses part of this - the double-backslash in double-quotes and will now generate an error if the trailing " or ' is missing.

[duglin]

Newlines are just a pain and may never work correctly until we don't need to worry about backwards compatibility. The biggest issue I see with them is that we try to parse things on a line-by-line basis, and we don't really know what type of Dockerfile command we have until later. This means that the logic to split things up based on quotes isn't done until after we have what we think is "a line". The biggest exception to that is the continuation char at the end - which, obviously, will try to wrap lines. But even then its not great since \\ at the end should not be treated as a \ but it is and fixing that involves a more char-by-char analysis of the line.

@elibarzilay where in the man pages does it talk about \x expansion? I see it for echo and printf but its not clear to me those apply to env vars.

[macropin]

I believe the behaviour has changed between recent versions.
Previously this was supported:

ENV "phpopts_suhosin.get.max_value_length=2048"

Now in docker 17.06.0-ce and presumably other versions this raises a parsing error and it is impossible to set environment variables that contain a period.

[rcjsuen]

@macropin If you wrap the key in quotes then it seems to work.

FROM scratch
ENV "phpopts_suhosin.get.max_value_length"=2048

$ docker build .

Sending build context to Docker daemon  1.693MB
Step 1/2 : FROM scratch
 --->
Step 2/2 : ENV "phpopts_suhosin.get.max_value_length"=2048
 ---> Running in a8db2ccd3d35
Removing intermediate container a8db2ccd3d35
 ---> eb8b6069ff42
Successfully built eb8b6069ff42

$ docker inspect eb8b6069ff42 --format='{{.ContainerConfig.Env}}'

[PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin phpopts_suhosin.get.max_value_length=2048]

[rcjsuen]

Actually, looks like you don't even need quotes.

FROM scratch
ENV phpopts_suhosin.get.max_value_length=2048

$ docker build .
Sending build context to Docker daemon  1.693MB
Step 1/2 : FROM scratch
 --->
Step 2/2 : ENV phpopts_suhosin.get.max_value_length=2048
 ---> Running in adeb8478b6d4
Removing intermediate container adeb8478b6d4
 ---> 0d328c2faa23
Successfully built 0d328c2faa23

[cooperbrown9]

YEah I've got
ENV REACT_APP_API=http://abcdefg.com/api
its not working... why dont they just make it JSON!