Skip to content

fix the case when toJSON() returns a Buffer#2

Closed
chatziko wants to merge 2 commits intosocketio:masterfrom
chatziko:master
Closed

fix the case when toJSON() returns a Buffer#2
chatziko wants to merge 2 commits intosocketio:masterfrom
chatziko:master

Conversation

@chatziko
Copy link

@chatziko chatziko commented Jan 8, 2016

has-binary assumes that toJSON() returns an object, but this is not necessary.
It incorrectly returns false for objects whose toJSON() returns a Buffer, eg:

{ a: 'a', toJSON: function() { return new Buffer('abc') } }

Returning Buffer from toJSON is useful for classes who want to serialize themselves in binary when sent over socket.io.

The fix is simple, I also added a test.

@darrachequesne
Copy link
Member

Merged as #6, thanks!

@chatziko
Copy link
Author

chatziko commented Mar 1, 2017

Thanks for the merge!

Maybe it's a detail, but if the aim of has-binary is to match JSON.stringify, then toJSON() should not be called again in the object returned by toJSON(). In other words:

> var o = { toJSON: function() { return this } }
> JSON.stringify(o)
'{}'
> hasBinary(o)
RangeError: Maximum call stack size exceeded

That's why my fix was a bit more complicated than a recursive call.

@darrachequesne
Copy link
Member

@chatziko #7 what do you think?

@chatziko
Copy link
Author

Looks good.

(looking forward to a release to stop maintaining my own fork :D)

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

Successfully merging this pull request may close these issues.

2 participants