Fix #29 Fix COPY validation error with JSON folders

If a COPY instruction is written in the JSON format and has more than two string arguments, the last string argument will be incorrectly flagged as not being a folder even if it is. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Mar 31, 2018 · 55fa76b · 55fa76b
1 parent ea40650
commit 55fa76b
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,9 @@ All notable changes to this project will be documented in this file.
 - warn if two or more decimals found in a unit of time in HEALTHCHECK duration flags ([#25](https://github.com/rcjsuen/dockerfile-utils/issues/25))
 - warn if two hyphens are found in HEALTHCHECK duration flags ([#26](https://github.com/rcjsuen/dockerfile-utils/issues/26))
 
+### Fixed
+- fix incorrect validation error if a COPY uses JSON arguments and its last string argument is correctly defined as a folder ([#29](https://github.com/rcjsuen/dockerfile-utils/issues/29))
+
 ## [0.0.7] - 2018-03-01
 ### Fixed
 - use a non-zero range for the diagnostic if FROM's base image's digest is the empty string ([#21](https://github.com/rcjsuen/dockerfile-utils/issues/21))

diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts
@@ -627,10 +627,21 @@ export class Validator {
                     } else if (copyArgs.length === 0) {
                         problems.push(Validator.createCOPYRequiresAtLeastTwoArguments(instruction.getInstructionRange()));
                     } else if (copyArgs.length > 2) {
-                        let copyDestination = copyArgs[copyArgs.length - 1].getValue();
-                        let lastChar = copyDestination.charAt(copyDestination.length - 1);
-                        if (lastChar !== '\\' && lastChar !== '/') {
-                            problems.push(Validator.createCOPYDestinationNotDirectory(copyArgs[copyArgs.length - 1].getRange()));
+                        if (copy.getClosingBracket()) {
+                            let jsonStrings = copy.getJSONStrings();
+                            if (jsonStrings.length > 2) {
+                                let copyDestination = jsonStrings[jsonStrings.length - 1].getValue();
+                                let lastChar = copyDestination.charAt(copyDestination.length - 2);
+                                if (lastChar !== '\\' && lastChar !== '/') {
+                                    problems.push(Validator.createCOPYDestinationNotDirectory(jsonStrings[jsonStrings.length - 1].getRange()));
+                                }
+                            }
+                        } else {
+                            let copyDestination = copyArgs[copyArgs.length - 1].getValue();
+                            let lastChar = copyDestination.charAt(copyDestination.length - 1);
+                            if (lastChar !== '\\' && lastChar !== '/') {
+                                problems.push(Validator.createCOPYDestinationNotDirectory(copyArgs[copyArgs.length - 1].getRange()));
+                            }
                         }
                     }
                     this.checkFlagValue(flags, ["chown", "from"], problems);

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -1727,6 +1727,18 @@ describe("Docker Validator Tests", function() {
 
                 diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nCOPY Dockerfile Dockerfile2 C:\\tmp\\");
                 assert.equal(diagnostics.length, 0);
+
+                diagnostics = validateDockerfile("FROM alpine\nCOPY [ \"Dockerfile\", \"/root\" ]");
+                assert.equal(diagnostics.length, 0);
+
+                diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nCOPY [ \"Dockerfile\", \"C:\\tmp\" ]");
+                assert.equal(diagnostics.length, 0);
+
+                diagnostics = validateDockerfile("FROM alpine\nCOPY [ \"Dockerfile\", \"Dockerfile2\", \"/root/\" ]");
+                assert.equal(diagnostics.length, 0);
+
+                diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nCOPY [ \"Dockerfile\", \"Dockerfile2\", \"C:\\tmp\\\\\" ]");
+                assert.equal(diagnostics.length, 0);
             });
 
             it("requires at least two", function() {
@@ -1755,6 +1767,14 @@ describe("Docker Validator Tests", function() {
                 diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nCOPY Dockerfile Dockerfile2 C:\\tmp");
                 assert.equal(diagnostics.length, 1);
                 assertCOPYDestinationNotDirectory(diagnostics[0], 2, 28, 2, 34);
+
+                diagnostics = validateDockerfile("FROM alpine\nCOPY [ \"Dockerfile\", \"Dockerfile2\", \"/root\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertCOPYDestinationNotDirectory(diagnostics[0], 1, 36, 1, 43);
+
+                diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nCOPY [ \"Dockerfile\", \"Dockerfile2\", \"C:\\tmp\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertCOPYDestinationNotDirectory(diagnostics[0], 2, 36, 2, 44);
             });
         });