Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(yieldable-parser): parseAsync sometimes blocks for a *long* time #36 #37

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
"index.js",
"yieldable-stringify.js",
"yieldable-parser.js",
"test/"
"test/",
"utils/"
],
"devDependencies": {
"eslint": "^6.6.0",
Expand Down
1 change: 1 addition & 0 deletions test/big-json-with-string.txt

Large diffs are not rendered by default.

54 changes: 54 additions & 0 deletions test/test-index-of-generator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
const tap = require('tap');
const { indexOfGenerator } = require('../utils/index-of-generator');

function promisifyGenerator(generator){
return (...args) => {
const gen = generator(...args);

function handleNext(resolvedValue){
if(resolvedValue.done){
return Promise.resolve(resolvedValue.value);
}
return Promise.resolve(gen.next(resolvedValue.value)).then(handleNext);
}


return Promise.resolve(gen.next())
.then((resolvedValue) => handleNext(resolvedValue), (error) => Promise.reject(gen.throw(error)));
}
}

const asyncIndexOf = promisifyGenerator(indexOfGenerator);

const stringAsArray = ['s', 'o', 'm', 'e'];
const text = stringAsArray.join('');

tap.test('should return -1 if not found due to initialIndex', async () => {
const index = await asyncIndexOf(text, 's', 1);
const syncIndexOf = text.indexOf('s', 1);
tap.equal(index, syncIndexOf);
})

tap.test('should return proper index', async () => {
const index = await asyncIndexOf(text, 'e');
const syncIndexOf = text.indexOf('e');
tap.equal(index, syncIndexOf);
})

tap.test('should return proper test with initialIndex', async () => {
const index = await asyncIndexOf(text, 'e', 2);
const syncIndexOf = text.indexOf('e', 2);
tap.equal(index, syncIndexOf);
})

tap.test('should return correct indexOf if initialIndex is same', async () => {
const index = await asyncIndexOf(text, 'e', 3);
const syncIndexOf = text.indexOf('e', 3);
tap.equal(index, syncIndexOf);
})

tap.test('should return -1 if not found', async () => {
const index = await asyncIndexOf(text, 'x');
const syncIndexOf = text.indexOf('x');
tap.equal(index, syncIndexOf);
})
15 changes: 15 additions & 0 deletions test/test-parse-big-escaped-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

const yj = require('../index');
const tap = require('tap');
const { readFileSync } = require('fs');
const str = readFileSync('./test/big-json-with-string.txt');
const nodeJSNativeParse = JSON.parse(str);

// Make sure the API works well without the optional parameters.
yj.parseAsync(str, (err, obj) => {
if (!err)
tap.equal(obj.text, nodeJSNativeParse.text, 'Failed to parse big string message - omitting logs', { diagnostic: false });
else
tap.fail(err);
});
26 changes: 26 additions & 0 deletions utils/index-of-generator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//This number may require proper tuning - selecting 10K since any string bigger than that may benefit from specific algorithms.
//Like if more than 50k characters, use BHM Algorithm
//We are considering a big string to be ~10k characters
const MAX_STRING_SIZE = 10000;

module.exports.indexOfGenerator = function* indexOfGenerator(stringInput, searchInput, initialIndex = 0) {
if (!stringInput || !stringInput.length) {
return -1;
}
stringInput = stringInput.slice(initialIndex);
let innerAt = 0;
let index = -1;
while (true) {
const at = Math.max(innerAt - 1, 0);
yield chunk = stringInput.slice(at, innerAt + MAX_STRING_SIZE);
if (!chunk.length) {
return index;
}
const indexOf = chunk.indexOf(searchInput);
if (indexOf !== -1) {
yield index = indexOf + at + initialIndex;
return index;
}
innerAt += chunk.length + 1;
}
}
11 changes: 6 additions & 5 deletions yieldable-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* Multiple authors (IBM Corp.) - initial implementation and documentation
***************************************************************************/
'use strict';
const { indexOfGenerator } = require('./utils/index-of-generator');

/**
* This method parses a JSON text to produce an object or array.
Expand Down Expand Up @@ -64,14 +65,14 @@ let parseWrapper = (text, reviver, intensity, cb) => {
};

// Process strings specially.
let normalizeUnicodedString = () => {
let normalizeUnicodedString = function*() {
let inQuotes = ' ';
let tempIndex = at;
let index = 0;
let slash = 0;
let c = '"';
while (c) {
index = parseStr.indexOf('"', tempIndex + 1);
index = yield * indexOfGenerator(parseStr, '"', tempIndex + 1);
tempIndex = index;
ch = parseStr.charAt(tempIndex - 1);
while (ch === '\\') {
Expand All @@ -88,7 +89,7 @@ let parseWrapper = (text, reviver, intensity, cb) => {
}

// When parsing string values, look for " and \ characters.
index = inQuotes.indexOf('\\');
index = yield * indexOfGenerator(inQuotes, '\\');
while (index >= 0) {
let escapee = {
'"': '"',
Expand Down Expand Up @@ -124,7 +125,7 @@ let parseWrapper = (text, reviver, intensity, cb) => {
at = index + 1;
} else
break;
index = inQuotes.indexOf('\\', at);
index = yield * indexOfGenerator(inQuotes, '\\', at);
}
at = 0;
return inQuotes;
Expand Down Expand Up @@ -225,7 +226,7 @@ let parseWrapper = (text, reviver, intensity, cb) => {
return inQuotes;
} else {
seek();
return normalizeUnicodedString();
return yield * normalizeUnicodedString();
}
case '0':
case '1':
Expand Down