better slice json, do not use javascript constants after it #599
Conversation
Codecov Report
@@ Coverage Diff @@
## master #599 +/- ##
===========================================
- Coverage 100.00% 99.48% -0.52%
===========================================
Files 7 7
Lines 558 580 +22
===========================================
+ Hits 558 577 +19
- Misses 0 3 +3
Continue to review full report at Codecov.
|
|
This pull request introduces 1 alert when merging edf71f2 into 9107e2d - view on LGTM.com new alerts:
|
test/util-test.js
Outdated
| describe('util.cutAfterJSON()', () => { | ||
| it('works with simple JSON', () => { | ||
| assert.equal(util.cutAfterJSON('{"a": 1, "b": 1}'), '{"a": 1, "b": 1}'); | ||
| }); | ||
| it('Cut extra characters after JSON end', () => { | ||
| assert.equal(util.cutAfterJSON('{"a": 1, "b": 1}abcd'), '{"a": 1, "b": 1}'); | ||
| }); | ||
| it('tolerant to string constants', () => { | ||
| assert.equal(util.cutAfterJSON('{"a": "}1", "b": 1}abcd'), '{"a": "}1", "b": 1}'); | ||
| }); | ||
| it('tolerant to string with escaped quoting', () => { | ||
| assert.equal(util.cutAfterJSON('{"a": "\\\"}1", "b": 1}abcd'), '{"a": "\\\"}1", "b": 1}'); | ||
| }); | ||
| it('works with nested', () => { | ||
| assert.equal(util.cutAfterJSON( | ||
| '{"a": "\\\"1", "b": 1, "c": {"test": 1}}abcd'), | ||
| '{"a": "\\\"1", "b": 1, "c": {"test": 1}}'); | ||
| }); | ||
| it('works with utf', () => { | ||
| assert.equal(util.cutAfterJSON( | ||
| '{"a": "\\\"фыва", "b": 1, "c": {"test": 1}}abcd'), | ||
| '{"a": "\\\"фыва", "b": 1, "c": {"test": 1}}'); | ||
| }); | ||
| it('works with \\\\ in string', () => { | ||
| assert.equal(util.cutAfterJSON( | ||
| '{"a": "\\\\фыва", "b": 1, "c": {"test": 1}}abcd'), | ||
| '{"a": "\\\\фыва", "b": 1, "c": {"test": 1}}'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
some newlines to match the rest would be appreciated
There was a problem hiding this comment.
You mean make arguments on new lines like last 3 tests done ?
There was a problem hiding this comment.
i know whe're not 100% consistent but sth like a \n before the describe and some more between those it
|
this is clever, I like 👍 @TimeForANinja thanks for taking a look |
|
I also think it would be better to make a function to extract jsons from js/html files and search ones which having expected content, it would fix many random errors in parts, where json extracted. |
|
This pull request introduces 1 alert when merging 239a371 into 9107e2d - view on LGTM.com new alerts:
|
use oldschool counter instead of iterator
| * @param {String} mixedJson | ||
| * @return {String} | ||
| */ | ||
| exports.cutAfterJSON = function(mixedJson) { |
There was a problem hiding this comment.
This should always return, but the return statement is conditioned?
There was a problem hiding this comment.
oh yeah, good catch. if the counter is over 0, then it's unclosed json, invalid, and should throw.
There was a problem hiding this comment.
Alright am I being an idiot or does this function not actually cut the JSON at all, but is currently just verifying its integrity.....lastIndexOf(';ytplayer.load') is dropped, so this function is handling the uncut JSON...
I personally think it should cut it as soon as it faces a semi-colon with isString at false
| return mixedJson.substr(0, i+1); | ||
| } | ||
| } | ||
| } |
|
|
||
| /** | ||
| * Match begin and end braces of input JSON, return only json | ||
| * it works with braced jsons only! |
There was a problem hiding this comment.
JSON is always braced.
There was a problem hiding this comment.
quoted string and unquoted number, and boolean are valid JSON
JSON.parse('1') // 1
This will fix problems when youtube code recompiled and after json in config something unexpected.
fixes #585