Fix #34 Consider JSON arguments if only two arguments

Cover all the cases that can occur if the JSON format is used but there are only two actual arguments written for ADD and COPY instructions. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Apr 4, 2018 · b7155b7 · b7155b7
1 parent 7486e54
commit b7155b7
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 0 deletions.
diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts
@@ -594,6 +594,14 @@ export class Validator {
                             const jsonStrings = add.getJSONStrings();
                             if (jsonStrings.length === 0) {
                                 problems.push(Validator.createADDRequiresAtLeastTwoArguments(instruction.getArgumentsRange()));
+                            } else if (jsonStrings.length === 1) {
+                                problems.push(Validator.createADDRequiresAtLeastTwoArguments(jsonStrings[0].getJSONRange()));
+                            } else if (jsonStrings.length > 2) {
+                                const addDestination = jsonStrings[jsonStrings.length - 1].getValue();
+                                const lastChar = addDestination.charAt(addDestination.length - 2);
+                                if (lastChar !== '\\' && lastChar !== '/') {
+                                    problems.push(Validator.createADDDestinationNotDirectory(jsonStrings[jsonStrings.length - 1].getJSONRange()));
+                                }
                             }
                         }
                     } else {
@@ -680,6 +688,14 @@ export class Validator {
                             const jsonStrings = copy.getJSONStrings();
                             if (jsonStrings.length === 0) {
                                 problems.push(Validator.createCOPYRequiresAtLeastTwoArguments(instruction.getArgumentsRange()));
+                            } else if (jsonStrings.length === 1) {
+                                problems.push(Validator.createCOPYRequiresAtLeastTwoArguments(jsonStrings[0].getJSONRange()));
+                            } else 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].getJSONRange()));
+                                }
                             }
                         }
                     } else {

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -1605,6 +1605,12 @@ describe("Docker Validator Tests", function() {
 
                 diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nADD [\"Dockerfile\",\"Dockerfile2\",\"C:\\tmp\\\\\"]");
                 assert.equal(diagnostics.length, 0);
+
+                diagnostics = validateDockerfile("FROM alpine\nADD [\"Dockerfile\",\"Dockerfile2\",\"/root/\" ]");
+                assert.equal(diagnostics.length, 0);
+
+                diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nADD [\"Dockerfile\",\"Dockerfile2\",\"C:\\tmp\\\\\" ]");
+                assert.equal(diagnostics.length, 0);
             });
 
             it("requires at least two", function() {
@@ -1628,6 +1634,10 @@ describe("Docker Validator Tests", function() {
                 assert.equal(diagnostics.length, 1);
                 assertADDRequiresAtLeastTwoArguments(diagnostics[0], 1, 6, 1, 14);
 
+                diagnostics = validateDockerfile("FROM alpine\nADD [\"test.txt\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertADDRequiresAtLeastTwoArguments(diagnostics[0], 1, 6, 1, 14);
+
                 diagnostics = validateDockerfile("FROM alpine\nADD --chown=root:root");
                 assert.equal(diagnostics.length, 1);
                 assertADDRequiresAtLeastTwoArguments(diagnostics[0], 1, 0, 1, 3);
@@ -1665,6 +1675,14 @@ describe("Docker Validator Tests", function() {
                 diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nADD [\"Dockerfile\",\"Dockerfile2\",\"C:\\tmp\"]");
                 assert.equal(diagnostics.length, 1);
                 assertADDDestinationNotDirectory(diagnostics[0], 2, 33, 2, 39);
+
+                diagnostics = validateDockerfile("FROM alpine\nADD [\"Dockerfile\",\"Dockerfile2\",\"/root\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertADDDestinationNotDirectory(diagnostics[0], 1, 33, 1, 38);
+
+                diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nADD [\"Dockerfile\",\"Dockerfile2\",\"C:\\tmp\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertADDDestinationNotDirectory(diagnostics[0], 2, 33, 2, 39);
             });
         });
 
@@ -1819,6 +1837,12 @@ 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\",\"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() {
@@ -1842,6 +1866,10 @@ describe("Docker Validator Tests", function() {
                 assert.equal(diagnostics.length, 1);
                 assertCOPYRequiresAtLeastTwoArguments(diagnostics[0], 1, 7, 1, 15);
 
+                diagnostics = validateDockerfile("FROM alpine\nCOPY [\"test.txt\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertCOPYRequiresAtLeastTwoArguments(diagnostics[0], 1, 7, 1, 15);
+
                 diagnostics = validateDockerfile("FROM alpine\nCOPY --from=busybox");
                 assert.equal(diagnostics.length, 1);
                 assertCOPYRequiresAtLeastTwoArguments(diagnostics[0], 1, 0, 1, 4);
@@ -1883,6 +1911,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, 34, 2, 40);
+
+                diagnostics = validateDockerfile("FROM alpine\nCOPY [\"Dockerfile\",\"Dockerfile2\",\"/root\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertCOPYDestinationNotDirectory(diagnostics[0], 1, 34, 1, 39);
+
+                diagnostics = validateDockerfile("#escape=`\nFROM microsoft/nanoserver\nCOPY [\"Dockerfile\",\"Dockerfile2\",\"C:\\tmp\" ]");
+                assert.equal(diagnostics.length, 1);
+                assertCOPYDestinationNotDirectory(diagnostics[0], 2, 34, 2, 40);
             });
         });