Skip to content

error parsing named null#155

Merged
dervus merged 1 commit intonodeca:masterfrom
paolochiodi:master
Dec 18, 2014
Merged

error parsing named null#155
dervus merged 1 commit intonodeca:masterfrom
paolochiodi:master

Conversation

@paolochiodi
Copy link
Copy Markdown
Contributor

@paolochiodi paolochiodi commented Dec 18, 2014

I have this sample:

require('js-yaml').load('---\ntest: !!null \ntest2: some text'); // notice the space between null and \n

Throws TypeError: Cannot read property 'length' of null (attached full stack)

While this

require('js-yaml').load('---\ntest: !!null null\ntest2: some text');

and this

require('js-yaml').load('---\n- ntest: !!null \n- test2: some text');

work properly.

I've seen that adding

  if (data === null) {
    return true;
  }

here https://github.com/nodeca/js-yaml/blob/master/lib/js-yaml/type/null.js#L6 solves the problem, but I'm unsure this is the correct way to tackle the problem.

If you could point me to the right direct I'd be happy to provide a pull request.

Attached: Full stackstrace

TypeError: Cannot read property 'length' of null
    at Type.resolveYamlNull [as resolve] (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/type/null.js:6:17)
    at composeNode (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/loader.js:1377:17)
    at readBlockMapping (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/loader.js:1037:11)
    at composeNode (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/loader.js:1317:12)
    at readDocument (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/loader.js:1478:3)
    at loadDocuments (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/loader.js:1530:5)
    at load (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/loader.js:1547:19)
    at Object.safeLoad (/Users/paolochiodi/test/node_modules/js-yaml/lib/js-yaml/loader.js:1565:10)
    at repl:1:4
    at REPLServer.self.eval (repl.js:110:21)

@dervus dervus added the bug label Dec 18, 2014
@dervus dervus self-assigned this Dec 18, 2014
@dervus
Copy link
Copy Markdown
Collaborator

dervus commented Dec 18, 2014

Your solution is correct, actually. If you'll make a pull request with an appropriate unit test, it would be very welcome.

@paolochiodi
Copy link
Copy Markdown
Contributor Author

paolochiodi commented Dec 18, 2014

Where do you want me to put the unit test? Under test/issues?

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Dec 18, 2014

@paolochiodi yes, everything detected as "bug" in tracker, goes to test/issue for historic reasons.

dervus added a commit that referenced this pull request Dec 18, 2014
@dervus
Merge pull request #155 from paolochiodi/master
25f5c87 
error parsing named null
@dervus dervus merged commit 25f5c87 into nodeca:master Dec 18, 2014
@dervus
Copy link
Copy Markdown
Collaborator

dervus commented Dec 18, 2014

Thanks! GitHub doesn't send notifications on new commits by some reason, so I've not noticed that in time.

@paolochiodi
Copy link
Copy Markdown
Contributor Author

paolochiodi commented Dec 19, 2014

Thank you @dervus
Do you have an estimated date for a new release including this fix?

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Dec 19, 2014

Released. Thanks for report & PR!

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Dec 27, 2014

The same bug also exists at least in int, bool & binary :) . See pending PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@dervus dervus

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@paolochiodi @dervus @puzrin