multiformats / js-multiformats

Multiformats interface (multihash, multicodec, multibase and CID)
Other
233 stars 54 forks source link

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

Open Gozala opened 4 years ago

Gozala commented 4 years ago

https://github.com/multiformats/js-multiformats/pull/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 commented 4 years ago

@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: