Add JSON5 parser #381
Add JSON5 parser #381
Conversation
|
This PR is in draft: review and input is welcome. If we do decide to add a JSON5 parser to Dojo we'll have to figure out how we would do so. One simple possibility is to continue in the manner presented in this PR - copy the MIT-licensed source almost unmodified from the JSON5 project. |
|
Nicely done. Will it satisfy people worried about security or will they still be upset because dojo/parser has a call to |
|
This can be tested in a test app I created.
I haven't tested a build yet but the way this is implemented if |
|
Nice! |
|
I've added tests and the MIT license, and a README for process I'm using to import the JSON5 code into Dojo and propose be used for ongoing maintenance (feedback is welcome): |
dojo/parser relies on `eval` to parse data attributes. For data attributes that are JSON5 compliant a JSON5 parser can be used as a drop-in replacement which improves Dojo's ability to be used with a strict CSP policy that does not allow `unsafe-eval`
Previously the error was silently swallowed
Add ES5 String methods used by JSON5 parser Add JSON5 readme
string: add codePoint tests
|
I have reviewed use of I'm good with this PR being merged as-is. |
|
Sorry this has take a while, but my initial testing is very promising. I did have an issue building my own layers as json5\parse does have some trailing commas. Once I removed those, my app does seem to work. I will do some more testing, but just wanted to say testing has started... json5/parse.js | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/json5/parse.js b/json5/parse.js
index 738cb352..ac206138 100644
--- a/json5/parse.js
+++ b/json5/parse.js
@@ -540,14 +540,14 @@ define([
},
end: function () {
throw invalidChar(read());
- },
+ }
};
function newToken(type, value) {
return {
type: type,
value: value,
line: line,
- column: column,
+ column: column
};
}
function literal(s) {
@@ -713,7 +713,7 @@ define([
}
},
end: function () {
- },
+ }
};
function push() {
var value;
@@ -811,7 +811,7 @@ define([
'\v': '\\v',
'\0': '\\0',
'\u2028': '\\u2028',
- '\u2029': '\\u2029',
+ '\u2029': '\\u2029'
};
if (replacements[c]) {
return replacements[c]; |
|
@schallm thank you for the report! Trailing commas removed |
|
I download the pr change and do the test. parser.js:915 dojo/parser::parse() error Error: SyntaxError: JSON5: invalid character 'h' at 1:14 in data-dojo-props='container: this, iconClass: 'addIcon', label: 'Add Storage'' Could you help resolve this issue? Thanks. |
@changpeng789 This is an example of what @msssk pointed out in the PR description. Your |
dojo/parserrelies onevalto parse data attributes. For data attributes that are JSON5 compliant a JSON5 parser can be used as a drop-in replacement which improves Dojo's ability to be used with a strict CSP policy that does not allowunsafe-eval.This is a non-breaking change with opt-in security improvements. Any Dojo application that is currently deployed in a strict CSP environment with
has: { 'csp-restrictions': true }is restricted from usingdojo/parser. With this changedojo/parsercan be used and will parse all valid JSON5 data attributes.Invalid JSON5 (e.g. JS expressions) will result in an error in the console:
Dojo applications that are currently using JS expressions in data attributes can safely upgrade. As long as they don't specify
has: { 'csp-restrictions': true }theneval()will continue to be used and existing behavior and functionality will be preserved.There is a scenario where this could be a breaking change:
unsafe-evalhas: { 'csp-restrictions': true }In this case upgrading to a Dojo release incorporating this patch would result in errors when parsing invalid data attributes.
Closes #380