Fix ENV directive quoting, use Fmt.str

[MisterDA]

The environment variables were not correctly quoted which introduce unnecessary and sometimes buggy escape sequences in the variable value.
Piggy-back a switch from Fmt.strf to Fmt.str as the former is a deprecated alias to the later.

[avsm]

I'm not sure how to evaluate this PR. Could I get an example of what broke, and an explanation of how this fixes it please?

[MisterDA]

Sure. Currently, environment variables are quoted with %S:

https://ocaml.org/releases/4.12/api/Printf.html

S: convert a string argument to OCaml syntax (double quotes, escapes).

Docker ENV states:

The value will be interpreted for other environment variables, so quote characters will be removed if they are not escaped. Like command line parsing, quotes and backslashes can be used to include spaces within values.

However the Docker documentation is wrong: the character used to escape the value is not always a backslash, it's the character set by the escape parser directive which on Windows is usually set to ` (backtick).

My solution is to fetch the escape character, always enclose values in double-quotes, and escape inside the value the escape character and double-quotes.

Without the patch applied:

Dockerfile.(parser_directive (`Escape '`') @@
            env [("VAL1", {|quick "fox"|}); ("VAL2", {|la`zy \dog|})] |> string_of_t) |> print_string;;
(*
# escape=`
ENV VAL1="quick \"fox\"" VAL2="la`zy \\dog"
*)
Dockerfile.(parser_directive (`Escape '\\') @@
            env [("VAL1", {|quick "fox"|}); ("VAL2", {|la`zy \dog|})] |> string_of_t) |> print_string;;
(*
# escape=\
ENV VAL1="quick \"fox\"" VAL2="la`zy \\dog"
*)

With the patch applied:

Dockerfile.(parser_directive (`Escape '`') @@
            env [("VAL1", {|quick "fox"|}); ("VAL2", {|la`zy \dog|})] |> string_of_t) |> print_string;;
(*
# escape=`
ENV VAL1="quick `"fox`"" VAL2="la``zy \dog"
*)
Dockerfile.(parser_directive (`Escape '\\') @@
            env [("VAL1", {|quick "fox"|}); ("VAL2", {|la`zy \dog|})] |> string_of_t) |> print_string;;
(*
# escape=\
ENV VAL1="quick \"fox\"" VAL2="la`zy \\dog"
*)

Without the patch, on Windows with #escape=`:

Sending build context to Docker daemon  2.048kB
Step 1/3 : FROM mcr.microsoft.com/windows/nanoserver:20H2
 ---> 67ae078d1460
Step 2/3 : ENV VAL1="quick \"fox\"" VAL2="la`zy \\dog"
 ---> Using cache
 ---> 3b5b781e04c9
Step 3/3 : RUN echo %VAL1% && echo %VAL2%
 ---> Running in d3992da5a7b7
quick \fox\
la`zy \\dog
Removing intermediate container d3992da5a7b7
 ---> 3d5be9509bd7
Successfully built 3d5be9509bd7

With the patch, on Windows with #escape=`:

Sending build context to Docker daemon  2.048kB
Step 1/3 : FROM mcr.microsoft.com/windows/nanoserver:20H2
 ---> 67ae078d1460
Step 2/3 : ENV VAL1="quick `"fox`"" VAL2="la``zy \dog"
 ---> Running in 71774ece9441
Removing intermediate container 71774ece9441
 ---> 00b5151e0109
Step 3/3 : RUN echo %VAL1% && echo %VAL2%
 ---> Running in 2b3d038b10d0
quick "fox"
la`zy \dog
Removing intermediate container 2b3d038b10d0
 ---> 0ac57d847e36
Successfully built 0ac57d847e36

You get the original values only with the patch.

[MisterDA]

The force-push is a little simplification in the code.
I've tested this PR yesterday and it works as expected.

[avsm]

Thanks for the explanation! @djs55 @justincormack, just copying you about the incorrect Docker documentation referenced above in case you want to get that fixed: #53 (comment)

[MisterDA]

The escape character problem can also be seen with this, when escaping a single value containing a space without double-quotes:

$ cat << 'EOF' | docker build --no-cache -
# escape=`
FROM mcr.microsoft.com/windows/nanoserver:20H2
ENV VAR=foo\ bar
RUN echo %VAR%
EOF
Sending build context to Docker daemon  2.048kB
Error response from daemon: failed to parse Dockerfile: Syntax error - can't find = in "bar". Must be of the form: name=value

$ cat << 'EOF' | docker build --no-cache -
# escape=`
FROM mcr.microsoft.com/windows/nanoserver:20H2
ENV VAR=foo` bar
RUN echo %VAR%
EOF
Sending build context to Docker daemon  2.048kB
Step 1/3 : FROM mcr.microsoft.com/windows/nanoserver:20H2
 ---> 67ae078d1460
Step 2/3 : ENV VAR=foo` bar
 ---> Running in 4769f1cc508c
Removing intermediate container 4769f1cc508c
 ---> e5f3bcb89cab
Step 3/3 : RUN echo %VAR%
 ---> Running in 8912b40a1146
foo bar
Removing intermediate container 8912b40a1146
 ---> c9c5ac5359df
Successfully built c9c5ac5359df

[avsm]

Looks good, merging!