From bbceb96b541556de49a1e554f63d2e4ee31f15c0 Mon Sep 17 00:00:00 2001 From: Remy Suen Date: Mon, 28 May 2018 21:59:40 -0400 Subject: [PATCH] Fix #40 Flag incorrectly quoted arguments Signed-off-by: Remy Suen --- CHANGELOG.md | 1 + src/dockerValidator.ts | 100 +++++++++++++++++++++-------------- test/dockerValidator.test.ts | 96 +++++++++++++++++++-------------- 3 files changed, 117 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8193d37..0282b1c 100644 --- 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 index 3938ebb..30b8e49 100644 --- 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 index 7966162..799927c 100644 --- 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);