skeeto / pdjson

C JSON parser library that doesn't suck
The Unlicense
281 stars 36 forks source link

End-of-stream indication indistinguishable from error #22

Closed boris-kolpackov closed 4 years ago

boris-kolpackov commented 4 years ago

Currently, in the streaming mode the end-of-stream indicator is JSON_ERROR. For example:

$ ./stream <<<'1 2'
struct expect seq[] = {
    {JSON_NUMBER, "1"},
    {JSON_DONE},
    {JSON_NUMBER, "2"},
    {JSON_DONE},
    {JSON_ERROR},
};

Unfortunately, this is indistinguishable (or, at least, not easily distinguishable) from a real error. Compare:

$ ./stream <<<'1 2 }'
struct expect seq[] = {
    {JSON_NUMBER, "1"},
    {JSON_DONE},
    {JSON_NUMBER, "2"},
    {JSON_DONE},
    {JSON_ERROR},
};

It feels like a more natural indication would have been another JSON_DONE (in fact, since this is not documented in the README file, I initially assumed that's what happens). Are there any issues with this approach that I don't see? And if not, is there interest in making the change? If the answer to the second question is yes, I would be willing to work on a patch.

skeeto commented 4 years ago

You're completely right. In fact, even the tests themselves aren't properly distinguishing EOF from an actual error. I can abuse this in streaming tests expecting valid input by giving them trailing garbage, and they still pass. I'd be happy to have this fixed.

boris-kolpackov commented 4 years ago

Fixed by ea6c521d17b9bb46dd1a7c10fbd085d7aa32f195.