holepunchto / bare-crypto

Cryptographic primitives for JavaScript
Apache License 2.0
6 stars 2 forks source link

Add randomFillSync #2

Closed yassernasc closed 3 months ago

yassernasc commented 3 months ago

It's hard to write unit tests on random values without mocking, so my approach was using big values and making it easy to change the sample size.

yassernasc commented 3 months ago

Sharp tips.

So, the key points of the incremented solution IMO are:

  1. I'm using bare-type to detect if the buf argument is an ArrayBuffer, TypedArray (Buffer), or DataView and reading those with js_get_arraybuffer_info, js_get_typedarray_info, or js_get_dataview_info, respectivally.

  2. OpenSSL's RAND_bytes function now writes directly to the passed buffer and the offset is applied using Pointer index: &buf[offset] (IDK if this technique is a safe approach).

yassernasc commented 3 months ago

yes indeed, sweet refactoring. 👍

yassernasc commented 3 months ago

Tricky changes, I tried not to proliferate many conditionals and keep the flow sane.

My solution was to refine and reshape the values from offset and size according to all contexts. For the "bytes per elements” problem I used multiplication on the value passed by argument only, e.g. offset *= buf.BYTES_PER_ELEMENT.

And to apply the window view I relied on byteOffset and byteLength as default values. A little adjustment needed was to combine the offset argument with the byteOffset from the view: offset += buf.byteOffset (the offset value is already potentially multiplied by buf.BYTES_PER_ELEMENT at this point).

In that way, we still pass all the buffer to the native code regardless of how small a window view could be the case, I was wondering about the subarray(start, end) resolving that and not passing any offset to the native code, just the needed buffer and the amount to bytes to be written, but ArrayBuffer and DataView classes doesn't implement the subarray method and IDK if adding a new Buffer/TypedArray layer would make the optimization be lost.

yassernasc commented 3 months ago

Checks were added, and I cleaned the code style a little bit.