From 02afdb03fa950ac6aad663a3837179476b3e6d4d Mon Sep 17 00:00:00 2001 From: asweingarten Date: Thu, 11 Jan 2018 17:35:44 -0800 Subject: [PATCH 1/8] [1083] Svelte should throw a compile time error when illegal characters are used in computed names Approach: For each property name, construct a string that defines a function and see if parsing that string with Acorn throws an exception. If it does, assemble an informative error message that states which property is invalid, the first invalid character, and the location of that character within the name. Changes to codebase: - Added new validator test "properties-computed-must-be-valid-function-names" - Added new check into src/validate/js/propValidators/computed.ts, "checkForValidIdentifiers" - this check was added to src/validate/js/utils/checkForValidIdentifiers.ts like the other checks in "computed.ts" --- src/validate/js/propValidators/computed.ts | 2 ++ .../js/utils/checkForValidIdentifiers.ts | 21 +++++++++++++++++++ .../errors.json | 8 +++++++ .../input.html | 17 +++++++++++++++ 4 files changed, 48 insertions(+) create mode 100644 src/validate/js/utils/checkForValidIdentifiers.ts create mode 100644 test/validator/samples/properties-computed-must-be-valid-function-names/errors.json create mode 100644 test/validator/samples/properties-computed-must-be-valid-function-names/input.html diff --git a/src/validate/js/propValidators/computed.ts b/src/validate/js/propValidators/computed.ts index 644043ddb1..1bf3144c31 100644 --- a/src/validate/js/propValidators/computed.ts +++ b/src/validate/js/propValidators/computed.ts @@ -1,5 +1,6 @@ import checkForDupes from '../utils/checkForDupes'; import checkForComputedKeys from '../utils/checkForComputedKeys'; +import checkForValidIdentifiers from '../utils/checkForValidIdentifiers'; import { Validator } from '../../'; import { Node } from '../../../interfaces'; import walkThroughTopFunctionScope from '../../../utils/walkThroughTopFunctionScope'; @@ -20,6 +21,7 @@ export default function computed(validator: Validator, prop: Node) { checkForDupes(validator, prop.value.properties); checkForComputedKeys(validator, prop.value.properties); + checkForValidIdentifiers(validator, prop.value.properties); prop.value.properties.forEach((computation: Node) => { if (!isFunctionExpression.has(computation.value.type)) { diff --git a/src/validate/js/utils/checkForValidIdentifiers.ts b/src/validate/js/utils/checkForValidIdentifiers.ts new file mode 100644 index 0000000000..2e5bdd08d6 --- /dev/null +++ b/src/validate/js/utils/checkForValidIdentifiers.ts @@ -0,0 +1,21 @@ +import { Validator } from '../../'; +import { Node } from '../../../interfaces'; +import getName from '../../../utils/getName'; +import { parse } from 'acorn'; + +export default function checkForValidIdentifiers( + validator: Validator, + properties: Node[] +) { + properties.forEach(prop => { + const name = getName(prop.key); + const functionDefinitionString = `function ${name}() {}`; + try { + parse(functionDefinitionString); + } catch(exception) { + const invalidCharacter = functionDefinitionString[exception.pos] + validator.error(`Computed property name "${name}" is invalid. Character '${invalidCharacter}' at position ${exception.pos} is illegal in function identifiers`, prop.start); + } + + }); +} diff --git a/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json b/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json new file mode 100644 index 0000000000..c5ad38b51d --- /dev/null +++ b/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json @@ -0,0 +1,8 @@ +[{ + "message": "Computed property name \"hours-hyphen\" is invalid. Character '-' at position 14 is illegal in function identifiers", + "loc": { + "line": 14, + "column": 6 + }, + "pos": 172 +}] diff --git a/test/validator/samples/properties-computed-must-be-valid-function-names/input.html b/test/validator/samples/properties-computed-must-be-valid-function-names/input.html new file mode 100644 index 0000000000..74ddb45b96 --- /dev/null +++ b/test/validator/samples/properties-computed-must-be-valid-function-names/input.html @@ -0,0 +1,17 @@ +

+ The hour is + {{hours}} +

+ + From cfdc8902ab2f2f850832045a7266a48fdcbe3885 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 13 Jan 2018 20:05:27 -0500 Subject: [PATCH 2/8] run prettier (spaces -> tabs) --- src/validate/js/propValidators/computed.ts | 2 +- .../js/utils/checkForValidIdentifiers.ts | 22 ++++++++++------- .../input.html | 24 +++++++++---------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/validate/js/propValidators/computed.ts b/src/validate/js/propValidators/computed.ts index 1bf3144c31..9ee4ed5ebe 100644 --- a/src/validate/js/propValidators/computed.ts +++ b/src/validate/js/propValidators/computed.ts @@ -21,7 +21,7 @@ export default function computed(validator: Validator, prop: Node) { checkForDupes(validator, prop.value.properties); checkForComputedKeys(validator, prop.value.properties); - checkForValidIdentifiers(validator, prop.value.properties); + checkForValidIdentifiers(validator, prop.value.properties); prop.value.properties.forEach((computation: Node) => { if (!isFunctionExpression.has(computation.value.type)) { diff --git a/src/validate/js/utils/checkForValidIdentifiers.ts b/src/validate/js/utils/checkForValidIdentifiers.ts index 2e5bdd08d6..3b9968f902 100644 --- a/src/validate/js/utils/checkForValidIdentifiers.ts +++ b/src/validate/js/utils/checkForValidIdentifiers.ts @@ -8,14 +8,18 @@ export default function checkForValidIdentifiers( properties: Node[] ) { properties.forEach(prop => { - const name = getName(prop.key); - const functionDefinitionString = `function ${name}() {}`; - try { - parse(functionDefinitionString); - } catch(exception) { - const invalidCharacter = functionDefinitionString[exception.pos] - validator.error(`Computed property name "${name}" is invalid. Character '${invalidCharacter}' at position ${exception.pos} is illegal in function identifiers`, prop.start); - } - + const name = getName(prop.key); + const functionDefinitionString = `function ${name}() {}`; + try { + parse(functionDefinitionString); + } catch (exception) { + const invalidCharacter = functionDefinitionString[exception.pos]; + validator.error( + `Computed property name "${name}" is invalid. Character '${ + invalidCharacter + }' at position ${exception.pos} is illegal in function identifiers`, + prop.start + ); + } }); } diff --git a/test/validator/samples/properties-computed-must-be-valid-function-names/input.html b/test/validator/samples/properties-computed-must-be-valid-function-names/input.html index 74ddb45b96..92da829133 100644 --- a/test/validator/samples/properties-computed-must-be-valid-function-names/input.html +++ b/test/validator/samples/properties-computed-must-be-valid-function-names/input.html @@ -1,17 +1,17 @@

- The hour is - {{hours}} + The hour is + {{hours}}

From 146327e87f89c49788644f4380c16c68ad4a745c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 13 Jan 2018 20:06:16 -0500 Subject: [PATCH 3/8] fix expected error position, tweak expected message to include suggested alternative --- .../errors.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json b/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json index c5ad38b51d..2b6a36ecd1 100644 --- a/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json +++ b/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json @@ -1,8 +1,8 @@ [{ - "message": "Computed property name \"hours-hyphen\" is invalid. Character '-' at position 14 is illegal in function identifiers", + "message": "Computed property name 'hours-hyphen' is invalid — must be a valid identifier such as hours_hyphen", "loc": { "line": 14, - "column": 6 + "column": 3 }, - "pos": 172 + "pos": 150 }] From b19303679d803ea4166182ba1dbfa68365c5c6e1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 13 Jan 2018 22:44:45 -0500 Subject: [PATCH 4/8] simplify test slightly, add test for reserved words --- .../errors.json | 9 +++++++++ .../input.html | 12 ++++++++++++ .../errors.json | 6 +++--- .../input.html | 9 ++------- 4 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 test/validator/samples/properties-computed-cannot-be-reserved/errors.json create mode 100644 test/validator/samples/properties-computed-cannot-be-reserved/input.html diff --git a/test/validator/samples/properties-computed-cannot-be-reserved/errors.json b/test/validator/samples/properties-computed-cannot-be-reserved/errors.json new file mode 100644 index 0000000000..39c4039743 --- /dev/null +++ b/test/validator/samples/properties-computed-cannot-be-reserved/errors.json @@ -0,0 +1,9 @@ +[{ + "message": + "Computed property name 'new' is invalid — cannot be a JavaScript reserved word", + "loc": { + "line": 9, + "column": 3 + }, + "pos": 87 +}] diff --git a/test/validator/samples/properties-computed-cannot-be-reserved/input.html b/test/validator/samples/properties-computed-cannot-be-reserved/input.html new file mode 100644 index 0000000000..06bd2bf694 --- /dev/null +++ b/test/validator/samples/properties-computed-cannot-be-reserved/input.html @@ -0,0 +1,12 @@ + diff --git a/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json b/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json index 2b6a36ecd1..ebaac56a8f 100644 --- a/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json +++ b/test/validator/samples/properties-computed-must-be-valid-function-names/errors.json @@ -1,8 +1,8 @@ [{ - "message": "Computed property name 'hours-hyphen' is invalid — must be a valid identifier such as hours_hyphen", + "message": "Computed property name 'with-hyphen' is invalid — must be a valid identifier such as with_hyphen", "loc": { - "line": 14, + "line": 9, "column": 3 }, - "pos": 150 + "pos": 87 }] diff --git a/test/validator/samples/properties-computed-must-be-valid-function-names/input.html b/test/validator/samples/properties-computed-must-be-valid-function-names/input.html index 92da829133..1c9173f523 100644 --- a/test/validator/samples/properties-computed-must-be-valid-function-names/input.html +++ b/test/validator/samples/properties-computed-must-be-valid-function-names/input.html @@ -1,17 +1,12 @@ -

- The hour is - {{hours}} -

- From 1833bc194f24b956a076ab37cd4d7d9388ec1e68 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 13 Jan 2018 22:45:15 -0500 Subject: [PATCH 5/8] use acorn.isIdentifierStart and isIdentifierChar to determine validity --- src/utils/isValidIdentifier.ts | 11 ++++++++ src/validate/js/propValidators/computed.ts | 22 ++++++++++++++-- .../js/utils/checkForValidIdentifiers.ts | 25 ------------------- 3 files changed, 31 insertions(+), 27 deletions(-) create mode 100644 src/utils/isValidIdentifier.ts delete mode 100644 src/validate/js/utils/checkForValidIdentifiers.ts diff --git a/src/utils/isValidIdentifier.ts b/src/utils/isValidIdentifier.ts new file mode 100644 index 0000000000..d129e1bd9b --- /dev/null +++ b/src/utils/isValidIdentifier.ts @@ -0,0 +1,11 @@ +import { isIdentifierStart, isIdentifierChar } from 'acorn'; + +export default function isValidIdentifier(str) { + if (!isIdentifierStart(str.charCodeAt(0), true)) return false; + + for (let i = 0; i < str.length; i += 1) { + if (!isIdentifierChar(str.charCodeAt(i), true)) return false; + } + + return true; +} \ No newline at end of file diff --git a/src/validate/js/propValidators/computed.ts b/src/validate/js/propValidators/computed.ts index 9ee4ed5ebe..94a7212881 100644 --- a/src/validate/js/propValidators/computed.ts +++ b/src/validate/js/propValidators/computed.ts @@ -1,6 +1,8 @@ import checkForDupes from '../utils/checkForDupes'; import checkForComputedKeys from '../utils/checkForComputedKeys'; -import checkForValidIdentifiers from '../utils/checkForValidIdentifiers'; +import getName from '../../../utils/getName'; +import isValidIdentifier from '../../../utils/isValidIdentifier'; +import reservedNames from '../../../utils/reservedNames'; import { Validator } from '../../'; import { Node } from '../../../interfaces'; import walkThroughTopFunctionScope from '../../../utils/walkThroughTopFunctionScope'; @@ -21,9 +23,25 @@ export default function computed(validator: Validator, prop: Node) { checkForDupes(validator, prop.value.properties); checkForComputedKeys(validator, prop.value.properties); - checkForValidIdentifiers(validator, prop.value.properties); prop.value.properties.forEach((computation: Node) => { + const name = getName(computation.key); + + if (!isValidIdentifier(name)) { + const suggestion = name.replace(/[^_$a-z0-9]/ig, '_').replace(/^\d/, '_$&'); + validator.error( + `Computed property name '${name}' is invalid — must be a valid identifier such as ${suggestion}`, + computation.start + ); + } + + if (reservedNames.has(name)) { + validator.error( + `Computed property name '${name}' is invalid — cannot be a JavaScript reserved word`, + computation.start + ); + } + if (!isFunctionExpression.has(computation.value.type)) { validator.error( `Computed properties can be function expressions or arrow function expressions`, diff --git a/src/validate/js/utils/checkForValidIdentifiers.ts b/src/validate/js/utils/checkForValidIdentifiers.ts deleted file mode 100644 index 3b9968f902..0000000000 --- a/src/validate/js/utils/checkForValidIdentifiers.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { Validator } from '../../'; -import { Node } from '../../../interfaces'; -import getName from '../../../utils/getName'; -import { parse } from 'acorn'; - -export default function checkForValidIdentifiers( - validator: Validator, - properties: Node[] -) { - properties.forEach(prop => { - const name = getName(prop.key); - const functionDefinitionString = `function ${name}() {}`; - try { - parse(functionDefinitionString); - } catch (exception) { - const invalidCharacter = functionDefinitionString[exception.pos]; - validator.error( - `Computed property name "${name}" is invalid. Character '${ - invalidCharacter - }' at position ${exception.pos} is illegal in function identifiers`, - prop.start - ); - } - }); -} From 66b64e254d077668c095cd98a47a7b4b929b23e2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Jan 2018 08:20:14 -0500 Subject: [PATCH 6/8] types --- src/utils/isValidIdentifier.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/isValidIdentifier.ts b/src/utils/isValidIdentifier.ts index d129e1bd9b..6c86967b0a 100644 --- a/src/utils/isValidIdentifier.ts +++ b/src/utils/isValidIdentifier.ts @@ -1,6 +1,6 @@ import { isIdentifierStart, isIdentifierChar } from 'acorn'; -export default function isValidIdentifier(str) { +export default function isValidIdentifier(str: string): boolean { if (!isIdentifierStart(str.charCodeAt(0), true)) return false; for (let i = 0; i < str.length; i += 1) { From a85b09ea97cf0b250a26bba2d6d1d9f5c654a704 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Jan 2018 09:25:50 -0500 Subject: [PATCH 7/8] handle wacky identifier names in templates --- src/parse/index.ts | 19 ++++++++- src/utils/fullCharCodeAt.ts | 10 +++++ src/utils/isValidIdentifier.ts | 10 +++-- .../samples/unusual-identifier/input.html | 3 ++ .../samples/unusual-identifier/output.json | 41 +++++++++++++++++++ 5 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 src/utils/fullCharCodeAt.ts create mode 100644 test/parser/samples/unusual-identifier/input.html create mode 100644 test/parser/samples/unusual-identifier/output.json diff --git a/src/parse/index.ts b/src/parse/index.ts index 6711e9929f..03c7cb95e5 100644 --- a/src/parse/index.ts +++ b/src/parse/index.ts @@ -1,9 +1,11 @@ +import { isIdentifierStart, isIdentifierChar } from 'acorn'; import { locate, Location } from 'locate-character'; import fragment from './state/fragment'; import { whitespace } from '../utils/patterns'; import { trimStart, trimEnd } from '../utils/trim'; import getCodeFrame from '../utils/getCodeFrame'; import reservedNames from '../utils/reservedNames'; +import fullCharCodeAt from '../utils/fullCharCodeAt'; import hash from './utils/hash'; import { Node, Parsed } from '../interfaces'; import CompileError from '../utils/CompileError'; @@ -147,7 +149,22 @@ export class Parser { readIdentifier() { const start = this.index; - const identifier = this.read(/[a-zA-Z_$][a-zA-Z0-9_$]*/); + + let i = this.index; + + const code = fullCharCodeAt(this.template, i); + if (!isIdentifierStart(code, true)) return null; + + i += code <= 0xffff ? 1 : 2; + + while (i < this.template.length) { + const code = fullCharCodeAt(this.template, i); + + if (!isIdentifierChar(code, true)) break; + i += code <= 0xffff ? 1 : 2; + } + + const identifier = this.template.slice(this.index, this.index = i); if (reservedNames.has(identifier)) { this.error(`'${identifier}' is a reserved word in JavaScript and cannot be used here`, start); diff --git a/src/utils/fullCharCodeAt.ts b/src/utils/fullCharCodeAt.ts new file mode 100644 index 0000000000..034da9258b --- /dev/null +++ b/src/utils/fullCharCodeAt.ts @@ -0,0 +1,10 @@ +// Adapted from https://github.com/acornjs/acorn/blob/6584815dca7440e00de841d1dad152302fdd7ca5/src/tokenize.js +// Reproduced under MIT License https://github.com/acornjs/acorn/blob/master/LICENSE + +export default function fullCharCodeAt(str: string, i: number): number { + let code = str.charCodeAt(i) + if (code <= 0xd7ff || code >= 0xe000) return code; + + let next = str.charCodeAt(i + 1); + return (code << 10) + next - 0x35fdc00; +} \ No newline at end of file diff --git a/src/utils/isValidIdentifier.ts b/src/utils/isValidIdentifier.ts index 6c86967b0a..20ceeb4bbf 100644 --- a/src/utils/isValidIdentifier.ts +++ b/src/utils/isValidIdentifier.ts @@ -1,10 +1,14 @@ import { isIdentifierStart, isIdentifierChar } from 'acorn'; +import fullCharCodeAt from './fullCharCodeAt'; export default function isValidIdentifier(str: string): boolean { - if (!isIdentifierStart(str.charCodeAt(0), true)) return false; + let i = 0; - for (let i = 0; i < str.length; i += 1) { - if (!isIdentifierChar(str.charCodeAt(i), true)) return false; + while (i < str.length) { + const code = fullCharCodeAt(str, i); + if (!(i === 0 ? isIdentifierStart : isIdentifierChar)(code, true)) return false; + + i += code <= 0xffff ? 1 : 2; } return true; diff --git a/test/parser/samples/unusual-identifier/input.html b/test/parser/samples/unusual-identifier/input.html new file mode 100644 index 0000000000..b7da53a800 --- /dev/null +++ b/test/parser/samples/unusual-identifier/input.html @@ -0,0 +1,3 @@ +{{#each things as 𐊧}} +

𐊧

+{{/each}} \ No newline at end of file diff --git a/test/parser/samples/unusual-identifier/output.json b/test/parser/samples/unusual-identifier/output.json new file mode 100644 index 0000000000..72f576c84d --- /dev/null +++ b/test/parser/samples/unusual-identifier/output.json @@ -0,0 +1,41 @@ +{ + "hash": 2991613308, + "html": { + "start": 0, + "end": 43, + "type": "Fragment", + "children": [ + { + "start": 0, + "end": 43, + "type": "EachBlock", + "expression": { + "type": "Identifier", + "start": 8, + "end": 14, + "name": "things" + }, + "children": [ + { + "start": 24, + "end": 33, + "type": "Element", + "name": "p", + "attributes": [], + "children": [ + { + "start": 27, + "end": 29, + "type": "Text", + "data": "𐊧" + } + ] + } + ], + "context": "𐊧" + } + ] + }, + "css": null, + "js": null +} \ No newline at end of file From 80c55b1e51a99dd0bb3242aca8cb2e919b58692b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Jan 2018 10:35:25 -0500 Subject: [PATCH 8/8] oops --- .../samples/unusual-identifier/input.html | 2 +- .../samples/unusual-identifier/output.json | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/test/parser/samples/unusual-identifier/input.html b/test/parser/samples/unusual-identifier/input.html index b7da53a800..71c3f4a12f 100644 --- a/test/parser/samples/unusual-identifier/input.html +++ b/test/parser/samples/unusual-identifier/input.html @@ -1,3 +1,3 @@ {{#each things as 𐊧}} -

𐊧

+

{{𐊧}}

{{/each}} \ No newline at end of file diff --git a/test/parser/samples/unusual-identifier/output.json b/test/parser/samples/unusual-identifier/output.json index 72f576c84d..6434526df5 100644 --- a/test/parser/samples/unusual-identifier/output.json +++ b/test/parser/samples/unusual-identifier/output.json @@ -1,13 +1,13 @@ { - "hash": 2991613308, + "hash": 795130236, "html": { "start": 0, - "end": 43, + "end": 47, "type": "Fragment", "children": [ { "start": 0, - "end": 43, + "end": 47, "type": "EachBlock", "expression": { "type": "Identifier", @@ -18,16 +18,21 @@ "children": [ { "start": 24, - "end": 33, + "end": 37, "type": "Element", "name": "p", "attributes": [], "children": [ { "start": 27, - "end": 29, - "type": "Text", - "data": "𐊧" + "end": 33, + "type": "MustacheTag", + "expression": { + "type": "Identifier", + "start": 29, + "end": 31, + "name": "𐊧" + } } ] }