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

safeLoadAll does not respect JSON.parse behaviour #456

Closed
prydonius opened this issue Nov 8, 2018 · 2 comments
Closed

safeLoadAll does not respect JSON.parse behaviour #456

prydonius opened this issue Nov 8, 2018 · 2 comments

Comments

@prydonius
Copy link

safeLoadAll does not seem to respect the {json: true} option the same way safeLoad and loadAll do, not sure if this is the case for other options.

Example:

const yaml = require("js-yaml");

const yamlWithDupKey = `
test: true
test: false
`;

// Does not throw YAMLException duplicate key error
console.log(yaml.safeLoad(yamlWithDupKey, {json:true}));

// Does not throw YAMLException duplicate key error
console.log(yaml.loadAll(yamlWithDupKey, undefined, {json:true}));

// Unexpectedly throws YAMLException duplicate key error
console.log(yaml.safeLoadAll(yamlWithDupKey, undefined, {json:true}));

Output:

{ test: false }
[ { test: false } ]

/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:171
  throw generateError(state, message);
  ^
YAMLException: duplicated mapping key at line 3, column 1:
    test: false
    ^
    at generateError (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:165:10)
    at throwError (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:171:9)
    at storeMappingPair (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:308:7)
    at readBlockMapping (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:1071:9)
    at composeNode (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:1332:12)
    at readDocument (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:1492:3)
    at loadDocuments (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:1548:5)
    at loadAll (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:1556:19)
    at Object.safeLoadAll (/Users/adnan/Playground/test-yaml/node_modules/js-yaml/lib/js-yaml/loader.js:1585:12)
    at Object.<anonymous> (/Users/adnan/Playground/test-yaml/test.js:15:18)
@FauxFaux
Copy link

safeLoadAll ignores all options because the insane optional argument code is just plain wrong in at least two ways.

@puzrin
Copy link
Member

puzrin commented Dec 5, 2020

Close as self-fixed in 4.0 (see dev banch).

safe* mehods were dropped.

@puzrin puzrin closed this as completed Dec 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants