Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Possible bug with binary data attributes #348

Closed
sinistersig opened this issue Feb 11, 2016 · 3 comments
Closed

Possible bug with binary data attributes #348

sinistersig opened this issue Feb 11, 2016 · 3 comments
Labels

Comments

@sinistersig
Copy link
Contributor

I think this may be a bug in the library.

https://github.com/mcavage/node-ldapjs/blob/540a3a1f5c2f23ade95142fa2f43fb87db585b48/lib/change.js#L87

_attr.addValue(val[k].toString());
converts buffers to strings which causes an issue when it is added as an attribute

https://github.com/mcavage/node-ldapjs/blob/540a3a1f5c2f23ade95142fa2f43fb87db585b48/lib/attribute.js#L71

if (Buffer.isBuffer(val)) will always be false

I added a check to ensure that it doesn't call .toString() if the value is a buffer

var currVal = val[k];
var attrVal = Buffer.isBuffer(currVal) ? currVal : currVal.toString();
_attr.addValue(attrVal);

This question, although doesn't use ldapjs, has more details on the encoding issue:
http://stackoverflow.com/questions/19569986/node-js-uploading-output-of-imagemagick-to-aws-s3

See #346 as a possible solution.

@gmcdowell
Copy link

Any updates here? I see that CI has failed on the PR.

@jsumners
Copy link
Member

#383 is also a potential resolution.

@jsumners
Copy link
Member

👋

On February 22, 2023, we released version 3 of this library. As a result, we are closing this issue/pull request.

Please see issue #839 for more information, including how to proceed if you feel this closure is in error.

@ldapjs ldapjs locked as resolved and limited conversation to collaborators Feb 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants