-
Notifications
You must be signed in to change notification settings - Fork 69
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
Broken on node v5.6.0 #26
Comments
I think the best fix is to "recognize" if the passed in instance is a 'bl' instance, and append all the single buffers. This would be "introspecting" the passed instance, but I think that is better than allocating a big buffer for the sole purpose of appending it to another list. The original code also might create issues when appending to the nested bl, so we should probably fix it here. |
mcollina
added a commit
that referenced
this issue
Feb 10, 2016
This provides support for node v5.6.0. Fixes #26.
mcollina
added a commit
that referenced
this issue
Feb 11, 2016
This provides support for node v5.6.0. Fixes #26.
rvagg
added a commit
that referenced
this issue
Feb 11, 2016
rvagg
added a commit
that referenced
this issue
Feb 11, 2016
This was referenced Feb 11, 2016
rvagg
added a commit
that referenced
this issue
Feb 11, 2016
rvagg
added a commit
that referenced
this issue
Feb 11, 2016
rvagg
added a commit
that referenced
this issue
Feb 11, 2016
rvagg
added a commit
that referenced
this issue
Feb 12, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
bl tests are failing on node v5.6.0 because of nodejs/node#4951. What is failing is appending multiple buffer list within themselves: https://github.com/rvagg/bl/blob/master/test/test.js#L79-L94.
I am not sure we were relying on some broken functionality of core (not checking if arguments were buffers). In fact, the only thing of
Buffer
is relying on is acopy
method.Not sure what is the best fix for this, either on node or here.
Also reported here: moscajs/mosca#412.
cc @rvagg
The text was updated successfully, but these errors were encountered: