From 417acc3236bd9bb197aeadd74c902aec2edba7b4 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 b76818c6c256b043feccc0c2be3070020724bb58 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 ea07020d8ad3bd0afd85693e43a028668d7f1a28 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 084eb39fbd8f279c9b782baca4cd6aba1c917234 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 610b5a8225b35980cae04f4258e4e9abb179bba2 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 899ea3de44377731b72f811ef2f91edfe709bc44 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 557035c887ea38a557ff7c799f134be8d2a4ca8f 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 2abed15c99e303104b16b2db038c68be0318d989 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": "𐊧" + } } ] }