KotlinCrypto / secure-random

A Kotlin Multiplatform library for obtaining cryptographically secure random data from system sources
Apache License 2.0
26 stars 3 forks source link

Compiled JS failing when ran with Bunjs #8

Closed ronhombre closed 8 months ago

ronhombre commented 9 months ago

image

Reproduction Steps:

  1. Run gradle browserWebpack
  2. Copy files in build/compileSync/js/main/productionExecutable/kotlin into a NodeJS project.
  3. Use the Kotlin/JS compiled files that use secure-random.
  4. Run Bunjs (Fails here).

Note: Additionally, using the compiled Webpack also causes the same bug when running on NodeJS.

Work around: Delete the first if-statement and leave whatever was in the else statement.

05nelsonm commented 9 months ago

Wondering if this is a KotlinJS bug or not as a ByteArray should register as a UInt8Array

ronhombre commented 8 months ago

image

It still breaks when run with Bunjs.

Removing the code below fixes it.

if (_get_isNode__xss3j(Companion_getInstance())) {
  _get_crypto__fwud1y(Companion_getInstance()).randomFillSync(array);
} else {

}

Leaving just the code below allows it to run.

var offset = 0;
while (offset < bytes.length) {
  var len = bytes.length > 65536 ? 65536 : bytes.length;
  _get_crypto__fwud1y(Companion_getInstance()).getRandomValues(array.subarray(offset, offset + len | 0));
  offset = offset + len | 0;
}
05nelsonm commented 8 months ago

What version of Node.js is being utilized by bunjs

randomFillSync should accept a TypedArray, not just a Uint8Array.

05nelsonm commented 8 months ago

14 is up with a published SNAPSHOT. Could you test it out to verify the fix?

  1. Add the following to your root project's repositories block
    allprojects {
        repositories {
            // ...
            maven("https://s01.oss.sonatype.org/content/repositories/snapshots/")
        }
    }
  2. Use version org.kotlincrypto:secure-random:0.3.1-SNAPSHOT
ronhombre commented 8 months ago

@05nelsonm Confirmed. Thanks for the fix.

image

05nelsonm commented 8 months ago

@05nelsonm Confirmed. Thanks for the fix.

image

Great, will merge the fix and get a release out in a moment