Skip to content

Remove cid.buffer in favor of cid.bytes.buffer to avoid issues with node libraries #30

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

Open
Gozala opened this issue Aug 7, 2020 · 1 comment

Comments

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2020

#29 repurposed former buffer: Buffer property to buffer: ArrayBuffer property that as @mikeal points out can lead to issues in node land that would happily accept either and mutate it.

This means that it could lead to issues when cid.byteOffset > 0 || cid.byteLength !== cid.buffer.byteLength. To avoid these problems we can rename .buffer to .arrayBuffer instead.

@Gozala Gozala changed the title Rename .buffer to .arrayBuffer to avoid issues with node libraries Remove cid.buffer in favor of cid.bytes.buffer to avoid issues with node libraries Aug 7, 2020
@Gozala
Copy link
Contributor Author

Gozala commented Aug 7, 2020

@mikeal I recall your primary argument was that lot of node utils would take either and would write into ArrayBuffer just as if it was Buffer. However now that I think about it, writing into a cid.buffer would create whole class of other issues, so I'm no longer sue this is important.

I think we have also considered alternative of keeping buffer: ArrayBuffer and throwing if cid represents a view into slice of the ArrayBuffer. This got me thinking about following:

  • We could emit warning and return a slice of this.arrayBuffer.slice(this.byteOffset, this.byteLength) in such case.
    • That would alleviate issues with node libraries.
    • It is also worth pointing out that CID will only be view into slice of ArrayBuffer when constructor is used instead of factory.

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

No branches or pull requests

1 participant