search

nodeca / js-yaml

JavaScript YAML parser and dumper. Very fast.
http://nodeca.github.io/js-yaml/
MIT License
6.28k stars 769 forks source link

Loading string with multiple line breaks results in unexpected null #681

Closed fischeversenker closed 2 years ago

fischeversenker commented 2 years ago

The load function returns undefined if you provide an empty string ('') or a string with a single line break ('\n'). For multiple line breaks ('\n\n') it unexpectedly returns null.

I provide tests for this in this commit in my fork.

I'm not entirely familiar with the YAML spec, so I'm not sure if this might even be intended behavior. Is it?

puzrin commented 2 years ago

AFAIK, it's intended, but i don't remember why. You can dig previous issues, this question has been discussed in the past several times.

fischeversenker commented 2 years ago

Thanks for the quick feedback, @puzrin.

I had a look at old issues, and it seems to me like it's not entirely clear how load should behave. On https://yaml-online-parser.appspot.com/?yaml=&type=json, it always returns null, regardless of how many empty lines there are. With this library, however, like described in this issue, we get undefined for one or two empty lines, but null with more than 2 empty lines: http://nodeca.github.io/js-yaml/#yaml=Cgo=, see also this comment.

Looking at the specification, I don't see anything about how to handle multiple consecutive empty lines.

As a user I would expect to get the same result from calling load with '', '\n', '\n\n', ... So the change proposed in https://github.com/nodeca/js-yaml/commit/98f3f8b898b48a2b5aad6bd4eb909c3d4de45513 seems very reasonable to me.

Related issues: #509, #567, #565