Skip to content

Commit

Permalink
Fix #191 Single quote LABEL values should be literal
Browse files Browse the repository at this point in the history
If an environment variable such as a $var or ${var} is found in a
LABEL's value that is contained in single quotes, they should be
ignored. This is because the content in a single quoted string should
be read literally and not go through any processing for variable
expansions and such.

Signed-off-by: Remy Suen <[email protected]>
  • Loading branch information
rcjsuen committed Nov 14, 2017
1 parent 3b201c7 commit 4358777
Show file tree
Hide file tree
Showing 6 changed files with 275 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
### Fixed
- prevent completion items from being displayed in comments ([#190](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/190))
- expand environment variables when validating an EXPOSE ([#192](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/192))
- ignore variables that are in a LABEL's single quoted value string ([#191](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/191))

## [0.0.10] - 2017-10-23
### Added
Expand Down
24 changes: 24 additions & 0 deletions src/parser/instructions/label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,39 @@
* ------------------------------------------------------------------------------------------ */
import { TextDocument, Range } from 'vscode-languageserver';
import { Dockerfile } from '../dockerfile';
import { Variable } from '../variable';
import { Property } from '../property';
import { PropertyInstruction } from './propertyInstruction';
import { Util } from '../../docker';

export class Label extends PropertyInstruction {

constructor(document: TextDocument, range: Range, dockerfile: Dockerfile, escapeChar: string, instruction: string, instructionRange: Range) {
super(document, range, dockerfile, escapeChar, instruction, instructionRange);
}

public getVariables(): Variable[] {
const variables = super.getVariables();
const properties = this.getProperties();
// iterate over all of this LABEL's properties
for (const property of properties) {
const value = property.getRawValue();
// check if the value is contained in single quotes,
// single quotes would indicate a literal value
if (value.length >= 2 && value.charAt(0) === '\'' && value.charAt(value.length - 1) === '\'') {
const range = property.getValueRange();
for (let i = 0; i < variables.length; i++) {
// if a variable is in a single quote, remove it from the list
if (Util.isInsideRange(variables[i].getRange().start, range)) {
variables.splice(i, 1);
i--;
}
}
}
}
return variables;
}

public getProperties(): Property[] {
return super.getProperties();
}
Expand Down
48 changes: 48 additions & 0 deletions test/dockerDefinition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,18 @@ describe("Dockerfile Document Definition tests", function() {
location = computeDefinition(document, Position.create(2, 5));
assertLocation(location, document.uri, 0, 4, 0, 7);
});

it("label value with single quotes", function() {
let document = createDocument(instruction + " var\nLABEL label='${var}'");
let location = computeDefinition(document, Position.create(1, 17));
assert.equal(location, null);
});

it("label value with double quotes", function() {
let document = createDocument(instruction + " var\nLABEL label=\"${var}\"");
let location = computeDefinition(document, Position.create(1, 17));
assertLocation(location, document.uri, 0, 4, 0, 7);
});
});

describe("$var", function() {
Expand Down Expand Up @@ -305,6 +317,18 @@ describe("Dockerfile Document Definition tests", function() {
location = computeDefinition(document, Position.create(2, 5));
assertLocation(location, document.uri, 0, 4, 0, 7);
});

it("label value with single quotes", function() {
let document = createDocument(instruction + " var\nLABEL label='$var'");
let location = computeDefinition(document, Position.create(1, 15));
assert.equal(location, null);
});

it("label value with double quotes", function() {
let document = createDocument(instruction + " var\nLABEL label=\"$var\"");
let location = computeDefinition(document, Position.create(1, 15));
assertLocation(location, document.uri, 0, 4, 0, 7);
});
});
});

Expand Down Expand Up @@ -486,6 +510,18 @@ describe("Dockerfile Document Definition tests", function() {
location = computeDefinition(document, Position.create(7, 5));
assertLocation(location, document.uri, 5, 4, 5, 7);
});

it("label value with single quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label='$var'");
let location = computeDefinition(document, Position.create(2, 15));
assert.equal(location, null);
});

it("label value with double quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label=\"$var\"");
let location = computeDefinition(document, Position.create(2, 15));
assertLocation(location, document.uri, 1, 4, 1, 7);
});
});

describe("$var", function() {
Expand Down Expand Up @@ -719,6 +755,18 @@ describe("Dockerfile Document Definition tests", function() {
location = computeDefinition(document, Position.create(7, 5));
assertLocation(location, document.uri, 5, 4, 5, 7);
});

it("label value with single quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label='${var}'");
let location = computeDefinition(document, Position.create(2, 17));
assert.equal(location, null);
});

it("label value with double quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var\nLABEL label=\"${var}\"");
let location = computeDefinition(document, Position.create(2, 17));
assertLocation(location, document.uri, 1, 4, 1, 7);
});
});
});
});
Expand Down
88 changes: 88 additions & 0 deletions test/dockerHighlight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,50 @@ describe("Dockerfile Document Highlight tests", function() {
ranges = computeHighlightRanges(document, 3, 12);
assertHighlightRanges(ranges, expected);
});

it("$var in LABEL value with single quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='$var'");
let ranges = computeHighlightRanges(document, 0, 5);
assertHighlightRanges(ranges, [ declaration ]);

ranges = computeHighlightRanges(document, 1, 15);
assert.equal(ranges.length, 0);
});

it("$var in LABEL value with double quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
let labelValue = DocumentHighlight.create(Range.create(Position.create(1, 14), Position.create(1, 17)), DocumentHighlightKind.Read);
let expected = [ declaration, labelValue ];
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"$var\"");
let ranges = computeHighlightRanges(document, 0, 5);
assertHighlightRanges(ranges, expected);

ranges = computeHighlightRanges(document, 1, 15);
assertHighlightRanges(ranges, expected);
});

it("${var} in LABEL value with single quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='${var}'");
let ranges = computeHighlightRanges(document, 0, 5);
assertHighlightRanges(ranges, [ declaration ]);

ranges = computeHighlightRanges(document, 1, 17);
assert.equal(ranges.length, 0);
});

it("${var} in LABEL value with double quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(0, 4), Position.create(0, 7)), DocumentHighlightKind.Write);
let labelValue = DocumentHighlight.create(Range.create(Position.create(1, 15), Position.create(1, 18)), DocumentHighlightKind.Read);
let expected = [ declaration, labelValue ];
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"${var}\"");
let ranges = computeHighlightRanges(document, 0, 5);
assertHighlightRanges(ranges, expected);

ranges = computeHighlightRanges(document, 1, 17);
assertHighlightRanges(ranges, expected);
});
});

describe("build stage", function() {
Expand Down Expand Up @@ -722,6 +766,50 @@ describe("Dockerfile Document Highlight tests", function() {
ranges = computeHighlightRanges(document, 9, 11);
assertHighlightRanges(ranges, expectedB);
});

it("$var in LABEL value with single quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='$var'");
let ranges = computeHighlightRanges(document, 1, 5);
assertHighlightRanges(ranges, [ declaration ]);

ranges = computeHighlightRanges(document, 2, 15);
assert.equal(ranges.length, 0);
});

it("$var in LABEL value with double quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
let labelValue = DocumentHighlight.create(Range.create(Position.create(2, 14), Position.create(2, 17)), DocumentHighlightKind.Read);
let expected = [ declaration, labelValue ];
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"$var\"");
let ranges = computeHighlightRanges(document, 1, 5);
assertHighlightRanges(ranges, expected);

ranges = computeHighlightRanges(document, 2, 15);
assertHighlightRanges(ranges, expected);
});

it("${var} in LABEL value with single quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='${var}'");
let ranges = computeHighlightRanges(document, 1, 5);
assertHighlightRanges(ranges, [ declaration ]);

ranges = computeHighlightRanges(document, 2, 17);
assert.equal(ranges.length, 0);
});

it("${var} in LABEL value with double quotes", function() {
let declaration = DocumentHighlight.create(Range.create(Position.create(1, 4), Position.create(1, 7)), DocumentHighlightKind.Write);
let labelValue = DocumentHighlight.create(Range.create(Position.create(2, 15), Position.create(2, 18)), DocumentHighlightKind.Read);
let expected = [ declaration, labelValue ];
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"${var}\"");
let ranges = computeHighlightRanges(document, 1, 5);
assertHighlightRanges(ranges, expected);

ranges = computeHighlightRanges(document, 2, 17);
assertHighlightRanges(ranges, expected);
});
});
});
}
Expand Down
22 changes: 22 additions & 0 deletions test/dockerHover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,28 @@ describe("Dockerfile hover", function() {
assert.equal(onHover(document, 3, 9), null);
assert.equal(onHover(document, 4, 11), null);
});

it("$var in LABEL value with single quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='$var'");
assert.equal(onHover(document, 1, 15), null);
});

it("$var in LABEL value with double quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"$var\"");
let hover = onHover(document, 1, 15);
assert.equal(hover.contents, "value");
});

it("${var} in LABEL value with single quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='${var}'");
assert.equal(onHover(document, 1, 17), null);
});

it("${var} in LABEL value with double quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"${var}\"");
let hover = onHover(document, 1, 17);
assert.equal(hover.contents, "value");
});
});
}

Expand Down
92 changes: 92 additions & 0 deletions test/dockerRename.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,52 @@ describe("Dockerfile Document Rename tests", function() {
assertEdit(edits[4], "renamed", 4, 11, 4, 14);
assertEdit(edits[5], "renamed", 5, 11, 5, 14);
});

it("$var in LABEL value with single quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='$var'");
let edits = rename(document, 0, 5, "renamed");
assert.equal(edits.length, 1);
assertEdit(edits[0], "renamed", 0, 4, 0, 7);

edits = rename(document, 1, 15, "renamed");
assert.equal(edits.length, 0);
});

it("$var in LABEL value with double quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"$var\"");
let edits = rename(document, 0, 5, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 0, 4, 0, 7);
assertEdit(edits[1], "renamed", 1, 14, 1, 17);

edits = rename(document, 1, 15, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 0, 4, 0, 7);
assertEdit(edits[1], "renamed", 1, 14, 1, 17);
});

it("${var} in LABEL value with single quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label='${var}'");
let edits = rename(document, 0, 5, "renamed");
assert.equal(edits.length, 1);
assertEdit(edits[0], "renamed", 0, 4, 0, 7);

edits = rename(document, 1, 17, "renamed");
assert.equal(edits.length, 0);
});

it("${var} in LABEL value with double quotes", function() {
let document = createDocument(instruction + " var" + delimiter + "value\nLABEL label=\"${var}\"");
let edits = rename(document, 0, 5, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 0, 4, 0, 7);
assertEdit(edits[1], "renamed", 1, 15, 1, 18);

edits = rename(document, 1, 17, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 0, 4, 0, 7);
assertEdit(edits[1], "renamed", 1, 15, 1, 18);
});
});

describe("build stage", function() {
Expand Down Expand Up @@ -423,6 +469,52 @@ describe("Dockerfile Document Rename tests", function() {
assertEdits(rename(document, 12, 12, "renamed"), expectedEdits2);
assertEdits(rename(document, 13, 13, "renamed"), expectedEdits2);
});

it("$var in LABEL value with single quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='$var'");
let edits = rename(document, 1, 5, "renamed");
assert.equal(edits.length, 1);
assertEdit(edits[0], "renamed", 1, 4, 1, 7);

edits = rename(document, 2, 15, "renamed");
assert.equal(edits.length, 0);
});

it("$var in LABEL value with double quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"$var\"");
let edits = rename(document, 1, 5, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 1, 4, 1, 7);
assertEdit(edits[1], "renamed", 2, 14, 2, 17);

edits = rename(document, 2, 15, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 1, 4, 1, 7);
assertEdit(edits[1], "renamed", 2, 14, 2, 17);
});

it("${var} in LABEL value with single quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label='${var}'");
let edits = rename(document, 1, 5, "renamed");
assert.equal(edits.length, 1);
assertEdit(edits[0], "renamed", 1, 4, 1, 7);

edits = rename(document, 2, 17, "renamed");
assert.equal(edits.length, 0);
});

it("${var} in LABEL value with double quotes", function() {
let document = createDocument("FROM alpine\n" + instruction + " var" + delimiter + "value\nLABEL label=\"${var}\"");
let edits = rename(document, 1, 5, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 1, 4, 1, 7);
assertEdit(edits[1], "renamed", 2, 15, 2, 18);

edits = rename(document, 2, 17, "renamed");
assert.equal(edits.length, 2);
assertEdit(edits[0], "renamed", 1, 4, 1, 7);
assertEdit(edits[1], "renamed", 2, 15, 2, 18);
});
});
});
}
Expand Down

0 comments on commit 4358777

Please sign in to comment.