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

Dockerfile seems to allow some unformatted JSON arrays #32074

Closed
rcjsuen opened this issue Mar 24, 2017 · 4 comments
Closed

Dockerfile seems to allow some unformatted JSON arrays #32074

rcjsuen opened this issue Mar 24, 2017 · 4 comments

Comments

@rcjsuen
Copy link
Contributor

rcjsuen commented Mar 24, 2017

Description
Many Dockerfile instructions such as VOLUME or RUN take a JSON array. However, the syntax checking is not very strict.

Steps to reproduce the issue:

  1. Create two Dockerfiles.
FROM alpine:3.4
VOLUME [ "/data"
EXPOSE 8080
FROM alpine:3.4
VOLUME [ "/data
EXPOSE 8080
  1. Build them.

Describe the results you received:
The build succeeds!

$ docker build -t rcjsuen/test .
Sending build context to Docker daemon 53.25 kB
Step 1 : FROM alpine:3.4
3.4: Pulling from library/alpine
Status: Downloaded newer image for alpine:3.4
 ---> 245f7a86c576
Step 2 : VOLUME [ "/data"
 ---> Running in 5a629770e8b3
 ---> a070c2c95c88
Removing intermediate container 5a629770e8b3
Step 3 : EXPOSE 8080
 ---> Running in f8d3b1a8723f
 ---> cbfdb74134bb
Removing intermediate container f8d3b1a8723f
Successfully built cbfdb74134bb
$ docker build -t rcjsuen/test .
Sending build context to Docker daemon 53.25 kB
Step 1 : FROM alpine:3.4
3.4: Pulling from library/alpine
Status: Downloaded newer image for alpine:3.4
 ---> 245f7a86c576
Step 2 : VOLUME [ "/data
 ---> Running in 33a7c3bad606
 ---> f08248f7b109
Removing intermediate container 33a7c3bad606
Step 3 : EXPOSE 8080
 ---> Running in b0404ac94f94
 ---> 194743a43722
Removing intermediate container b0404ac94f94
Successfully built 194743a43722

Describe the results you expected:
Was kind of expecting it to fail with a parser error...?

Additional information you deem important (e.g. issue happens only occasionally):

Output of docker version:

docker version
Client:
 Version:      1.12.3
 API version:  1.24
 Go version:   go1.6.3
 Git commit:   6b644ec
 Built:        Wed Oct 26 21:44:32 2016
 OS/Arch:      linux/amd64
Server:
 Version:      1.12.3
 API version:  1.24
 Go version:   go1.6.3
 Git commit:   6b644ec
 Built:        Wed Oct 26 21:44:32 2016
 OS/Arch:      linux/amd64

Additional environment details (AWS, VirtualBox, physical, etc.):
Ubuntu 14.04.5 LTS
Linux 4.4.0-51-generic

@thaJeztah
Copy link
Member

Yes, this is a known limitation, but unfortunately not something that can be fixed. The reason is that both JSON and shell syntax can be provided, and while it may be invalid JSON, it could be valid shell syntax.

So, what happens is that;

  • docker checks if it's a valid JSON array
  • if so, proceed using the JSON syntax
  • if not, proceed, and use the shell syntax (run it as a shell command as-is)

In the last case, it may be an invalid shell command, but docker cannot verify that, because custom shells can be used, and verifying would mean implementing the shell in Docker itself, so handling of those errors is up to the shell that's run (and produce an error)

I'll close this issue because of the above, but hope my explanation helps understanding the reason this works like this.

Feel free to continue the discussion though!

@thaJeztah
Copy link
Member

For the VOLUME example; [ and "/data" are valid names for directories in Linux; you'll probably see that after building that Dockerfile, two volumes are defined at those locations.

@ijc
Copy link
Contributor

ijc commented Mar 24, 2017

Both the example in the report say VOLUME, but I assume one was supposed to say RUN so it is worth mentioning that there would also be no errors executing [ "/data" via a standard shell because /usr/bin/[ is an actual command (test(1)) and treats a bare string argument as -n string, where -n returns true/success if the length of the string is non-zero, which is the case for "/data" here so it succeeds.

@rcjsuen
Copy link
Contributor Author

rcjsuen commented Mar 24, 2017

Thank you two for the detailed explanation. This certainly keeps things interesting for people writing Dockerfile editors and linters. :P

Both were supposed to say VOLUME but it ended up being a great opportunity to learn about /usr/bin/[. Haha! :)

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