Fix #40 Flag incorrectly quoted arguments

Signed-off-by: Remy Suen <[email protected]>
rcjsuen · May 29, 2018 · bbceb96 · bbceb96
1 parent 0e90f69
commit bbceb96
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 80 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.
 - flag FROM instructions that refer to an invalid image digest in a private registry with a port as an error ([#42](https://github.com/rcjsuen/dockerfile-utils/issues/42))
 - flag variables that have an invalid modifier set ([#38](https://github.com/rcjsuen/dockerfile-utils/issues/38))
 - warn if ARG instruction does not define a name for the variable ([#45](https://github.com/rcjsuen/dockerfile-utils/issues/45))
+- flag incorrectly quoted arguments for ARG, ENV, and LABEL ([#40](https://github.com/rcjsuen/dockerfile-utils/issues/40))
 
 ### Fixed
 - fix incorrect validaiton warning in ARG, ENV, and LABEL instructions caused by quotes being used in variable replacements ([#36](https://github.com/rcjsuen/dockerfile-utils/issues/36))

diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts
@@ -5,7 +5,7 @@
 import {
     TextDocument, Range, Position, Diagnostic, DiagnosticSeverity
 } from 'vscode-languageserver-types';
-import { Dockerfile, Flag, Instruction, JSONInstruction, Add, Arg, Cmd, Copy, Entrypoint, From, Healthcheck, Onbuild, ModifiableInstruction, PropertyInstruction, DockerfileParser, Directive } from 'dockerfile-ast';
+import { Dockerfile, Flag, Instruction, JSONInstruction, Add, Arg, Cmd, Copy, Entrypoint, From, Healthcheck, Onbuild, ModifiableInstruction, PropertyInstruction, Property, DockerfileParser, Directive } from 'dockerfile-ast';
 import { ValidationCode, ValidationSeverity, ValidatorSettings } from './main';
 
 export const KEYWORDS = [
@@ -141,6 +141,58 @@ export class Validator {
         }
     }
 
+    private checkProperty(document: TextDocument, escapeChar: string, keyword: string, property: Property, firstProperty: boolean, optionalValue: boolean, problems: Diagnostic[]): void {
+        let name = property.getName();
+        if (name === "") {
+            let range = property.getRange();
+            problems.push(Validator.createSyntaxMissingNames(range.start, range.end, keyword));
+        } else if (name.indexOf('=') !== -1) {
+            let nameRange = property.getNameRange();
+            let unescapedName = document.getText(nameRange);
+            let index = unescapedName.indexOf('=');
+            if (unescapedName.charAt(0) === '\'') {
+                problems.push(Validator.createSyntaxMissingSingleQuote(nameRange.start, document.positionAt(document.offsetAt(nameRange.start) + index), unescapedName.substring(0, unescapedName.indexOf('='))));
+            } else if (unescapedName.charAt(0) === '"') {
+                problems.push(Validator.createSyntaxMissingDoubleQuote(nameRange.start, document.positionAt(document.offsetAt(nameRange.start) + index), unescapedName.substring(0, unescapedName.indexOf('='))));
+            }
+            return;
+        }
+
+        let value = property.getValue();
+        if (value === null) {
+            if (!optionalValue) {
+                let range = property.getNameRange();
+                if (firstProperty) {
+                    problems.push(Validator.createENVRequiresTwoArguments(range.start, range.end));
+                } else {
+                    problems.push(Validator.createSyntaxMissingEquals(range.start, range.end, name));
+                }
+            }
+        } else if (value.charAt(0) === '"') {
+            let found = false;
+            for (let i = 1; i < value.length; i++) {
+                switch (value.charAt(i)) {
+                    case escapeChar:
+                        i++;
+                        break;
+                    case '"':
+                        if (i === value.length - 1) {
+                            found = true;
+                        }
+                        break;
+                }
+            }
+
+            if (!found) {
+                let range = property.getValueRange();
+                problems.push(Validator.createSyntaxMissingDoubleQuote(range.start, range.end, property.getUnescapedValue()));
+            }
+        } else if (value.charAt(0) === '\'' && value.charAt(value.length - 1) !== '\'') {
+            let range = property.getValueRange();
+            problems.push(Validator.createSyntaxMissingSingleQuote(range.start, range.end, value));
+        }
+    }
+
     validate(document: TextDocument): Diagnostic[] {
         this.document = document;
         let problems: Diagnostic[] = [];
@@ -326,10 +378,10 @@ export class Validator {
                         }
                         return null;
                     }, Validator.createARGRequiresOneArgument);
-                    let argProperty = (instruction as Arg).getProperty();
-                    if (argProperty && argProperty.getName() === "") {
-                        let range = argProperty.getRange();
-                        problems.push(Validator.createSyntaxMissingNames(range.start, range.end, keyword));
+                    let arg = instruction as Arg;
+                    let argProperty = arg.getProperty();
+                    if (argProperty) {
+                        this.checkProperty(document, escapeChar, keyword, argProperty, true, true, problems);
                     }
                     break;
                 case "ENV":
@@ -338,43 +390,11 @@ export class Validator {
                         return null;
                     });
                     let properties = (instruction as PropertyInstruction).getProperties();
-                    if (properties.length === 1 && properties[0].getValue() === null) {
-                        let range = properties[0].getNameRange();
-                        problems.push(Validator.createENVRequiresTwoArguments(range.start, range.end));
+                    if (properties.length === 1) {
+                        this.checkProperty(document, escapeChar, keyword, properties[0], true, false, problems);
                     } else if (properties.length !== 0) {
                         for (let property of properties) {
-                            if (property.getName() === "") {
-                                let range = property.getRange();
-                                problems.push(Validator.createSyntaxMissingNames(range.start, range.end, keyword));
-                            }
-
-                            let value = property.getValue();
-                            if (value === null) {
-                                let range = property.getNameRange();
-                                problems.push(Validator.createSyntaxMissingEquals(range.start, range.end, property.getName()));
-                            } else if (value.charAt(0) === '"') {
-                                let found = false;
-                                for (let i = 1; i < value.length; i++) {
-                                    switch (value.charAt(i)) {
-                                        case escapeChar:
-                                            i++;
-                                            break;
-                                        case '"':
-                                            if (i === value.length - 1) {
-                                                found = true;
-                                            }
-                                            break;
-                                    }
-                                }
-
-                                if (!found) {
-                                    let range = property.getValueRange();
-                                    problems.push(Validator.createSyntaxMissingDoubleQuote(range.start, range.end, property.getUnescapedValue()));
-                                }
-                            } else if (value.charAt(0) === '\'' && value.charAt(value.length - 1) !== '\'') {
-                                let range = property.getValueRange();
-                                problems.push(Validator.createSyntaxMissingSingleQuote(range.start, range.end, value));
-                            }
+                            this.checkProperty(document, escapeChar, keyword, property, false, false, problems);
                         }
                     }
                     break;

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -1993,6 +1993,62 @@ describe("Docker Validator Tests", function() {
             assert.equal(diagnostics.length, 1);
             assertSyntaxMissingNames(diagnostics[0], instruction, 1, instructionLength + 1, 1, instructionLength + 2);
         });
+
+        it("syntax missing single quote", function() {
+            let diagnostics = validateDockerfile("FROM node\n" + instruction + " var='value");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingSingleQuote(diagnostics[0], "'value", 1, instructionLength + 5, 1, instructionLength + 11);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " var='val\\\nue");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingSingleQuote(diagnostics[0], "'value", 1, instructionLength + 5, 2, 2);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " 'abc=xyz");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingSingleQuote(diagnostics[0], "'abc", 1, instructionLength + 1, 1, instructionLength + 5);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " 'abc=xyz'");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingSingleQuote(diagnostics[0], "'abc", 1, instructionLength + 1, 1, instructionLength + 5);
+        });
+
+        it("syntax missing double quote", function() {
+            let diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"value");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value", 1, instructionLength + 5, 1, instructionLength + 11);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"value\\");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value\\", 1, instructionLength + 5, 1, instructionLength + 12);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"value\\\"");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value\\\"", 1, instructionLength + 5, 1, instructionLength + 13);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"a\\  ");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"a\\", 1, instructionLength + 5, 1, instructionLength + 8);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"a\\  b");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"a\\  b", 1, instructionLength + 5, 1, instructionLength + 11);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"val\\\nue");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value", 1, instructionLength + 5, 2, 2);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"val\\\r\nue");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value", 1, instructionLength + 5, 2, 2);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " \"abc=xyz");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"abc", 1, instructionLength + 1, 1, instructionLength + 5);
+
+            diagnostics = validateDockerfile("FROM node\n" + instruction + " \"abc=xyz\"");
+            assert.equal(diagnostics.length, 1);
+            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"abc", 1, instructionLength + 1, 1, instructionLength + 5);
+        });
     }
 
     describe("ARG", function() {
@@ -2341,46 +2397,6 @@ describe("Docker Validator Tests", function() {
             assertSyntaxMissingEquals(diagnostics[0], "c", 1, instructionLength + 5, 1, instructionLength + 6);
         });
 
-        it("syntax missing single quote", function() {
-            let diagnostics = validateDockerfile("FROM node\n" + instruction + " var='value");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingSingleQuote(diagnostics[0], "'value", 1, instructionLength + 5, 1, instructionLength + 11);
-
-            diagnostics = validateDockerfile("FROM node\n" + instruction + " var='val\\\nue");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingSingleQuote(diagnostics[0], "'value", 1, instructionLength + 5, 2, 2);
-        });
-
-        it("syntax missing double quote", function() {
-            let diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"value");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value", 1, instructionLength + 5, 1, instructionLength + 11);
-
-            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"value\\");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value\\", 1, instructionLength + 5, 1, instructionLength + 12);
-
-            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"value\\\"");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value\\\"", 1, instructionLength + 5, 1, instructionLength + 13);
-
-            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"a\\  ");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"a\\", 1, instructionLength + 5, 1, instructionLength + 8);
-
-            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"a\\  b");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"a\\  b", 1, instructionLength + 5, 1, instructionLength + 11);
-
-            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"val\\\nue");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value", 1, instructionLength + 5, 2, 2);
-
-            diagnostics = validateDockerfile("FROM node\n" + instruction + " var=\"val\\\r\nue");
-            assert.equal(diagnostics.length, 1);
-            assertSyntaxMissingDoubleQuote(diagnostics[0], "\"value", 1, instructionLength + 5, 2, 2);
-        });
-
         it("missing name", function() {
             let diagnostics = validateDockerfile("FROM node\n" + instruction + " x=y =z a=b");
             assert.equal(diagnostics.length, 1);