toBase64 results differ from js-base64

[pke]

I have run the same byte arrays through js-base64 and cpp64 with those results:

 LOG  [82, 235, 19, 104, 164, 2, 214, 83, 201, 109, 220, 113, 198, 127, 246, 41, 43, 88, 224, 70, 83, 8, 70, 37, 84, 13, 192, 247, 128, 225, 4, 94]
 LOG  cpp64: UusTaKQC1lPJbdxxxn/2KStY4EZTCEYlVA3A94DhBF4=
 LOG  jsb64: UusTaKQC1lPJbdxxxn/2KStY4EZTCEYlVA3A94DhBF4=
 LOG  [11, 80, 135, 209, 171, 28, 64, 255, 39, 210, 24, 201, 226, 41, 153, 223, 120, 249, 81, 218, 135, 69, 207, 111]
 LOG  cpp64: C1CH0ascQP8n0hjJ4imZ33j5UdqHRc9v
 LOG  jsb64: C1CH0ascQP8n0hjJ4imZ33j5UdqHRc9v
 LOG  [184, 179, 212, 210, 19, 142, 29, 203, 166, 118, 216, 245, 113, 162, 222, 115]
 LOG  cpp64: AAAAAAAAAAAAAAAAAAAAALiz1NITjh3LpnbY9XGi3nM=
 LOG  jsb64: uLPU0hOOHcumdtj1caLecw==

The A padding on the last tuple is wrong..

Any idea what could be wrong?

[craftzdog]

Android? iOS? Reproduction code?

[pke]

That would be Android.
Not sure how quickly I can extract the code, but its basically just the ByteArray above the cpp64 line that gets encoded.

import { fromByteArray } from "react-native-quick-base64"
import { fromUint8Array } from "js-base64"

function transferEncode(value: Uint8Array) {
  console.log(value)
  console.log("cpp64:", fromByteArray(value))
  console.log("jsb64:", fromUint8Array(value))
}
transferEncode(Uint8Array.from([184, 179, 212, 210, 19, 142, 29, 203, 166, 118, 216, 245, 113, 162, 222, 115]))

[pke]

Ok, so I have checked with the original library that is used here and wrote a gtest for this particular array:

TEST(Base64Encode, encode_special_byte_array) {
    const unsigned char input[] = { 184, 179, 212, 210, 19, 142, 29, 203, 166, 118, 216, 245, 113, 162, 222, 115 };
    const std::string output = base64_encode(input, sizeof(input));
    ASSERT_STREQ(output.c_str(), "uLPU0hOOHcumdtj1caLecw==");
}

It runs fine.
So the problem must be in the Android/Bridge code.

So I investigated the code and tried to reproduce what the code does in my gtest:

This line will crash the test. One can't create a string from the given byte array:

std::string strBase64 = base64_encode(str);

Therefore the Android bridge code must always use the function with the signature:
base64_encode(unsigned char*, size_t) and not trying to convert a byte array (which could contain 0 bytes) into a std::string.

[pke]

Hmm... after reinstalling this RN plugin and rebuilding the app, the base64cpp output now matches the one from base64. So, I don't know what was going on :(
Closing this issue for now, hope it does not pop up again and was just a weird configuration issue.

[craftzdog]

Thanks. Yeah, that's strange.
If you found a bug in the cpp implementation, you can report it to https://github.com/ReneNyffenegger/cpp-base64.
The JSI bridge also could cause the issue. Let me know if you found a root culprit.

[pke]

shoot. I just had this popping up again!

LOG  [156, 184, 240, 9, 139, 217, 71, 112, 169, 167, 208, 240, 115, 170, 126, 64, 229, 92, 159, 151]
LOG  cpp64: AAAAAAAAAAAAAAAAAAAAAJy48AmL2UdwqafQ8HOqfkDlXJ+X
LOG  jsb64: nLjwCYvZR3Cpp9Dwc6p+QOVcn5c=

When I convert the cpp64 result back to hex
00,00,00,00,00,00,00,00,00,00,00,00,00,00,00,00,9c,b8,f0,09,8b,d9,47,70,a9,a7,d0,f0,73,aa,7e,40,e5,5c,9f,97

it has exactly 16 0-bytes in front. The Input byte array is 20 bytes long and follows those 16 "empty" bytes.

So its probably related to the bridge then. How could we track this down? Race condition?
Could it be that the input array buffer is in a state, where its internal cursor is on the wrong position?
Could the way how the string value is handed over (via pointer reference) be a problem in the std::string copy algo?

[pke]

OK, the error is probably in JSI. It does not properly take into account ArrayBuffer with byteOffset greater than 0. The tweetnacl package nacl.box returns an ArrayBuffer with a size of 20 bytes but with byteOffset if 16. That's 16 bytes of 0. Why the box function allocates the array in that way is beyond me.

However, the JSI function to get the underlaying data does not take the byteOffset into account. I have filed a bug over there.

A temporary fix for now is to wrap the ArrayBuffer into a new ArrayBuffer. The new buffer will alway have byteOffset of 0.
fromByteArray(new Uint8Array(source))

EDIT

Hmmm, it seems the error might still be within this bundle. I checked again how the buffer is transferred to the c++ side. And if I change

export function fromByteArray(uint8: Uint8Array): string {
-  return base64FromArrayBuffer(uint8.buffer);
+  return base64FromArrayBuffer(uint8.buffer.slice(uint8.byteOffset));
}

While this fixes it, I still think the error is in the jni side when handling the BufferView and ignoring its byteOffset.
Do you see any way to fix this without resorting to .slice?

[pke]

Ok, I have created a fix in the cpp wrapper code.
It's working, it does seem to be slower than the slice method. And for my short strings, even js-base64 seems to be fast enough ;)

 LOG  cpp64   : tt4oOmGUfAlbuczTPq4vZJUBPptMx3qWVFmaagDndCY= 0.19234305620193481
 LOG  cpp64fix: tt4oOmGUfAlbuczTPq4vZJUBPptMx3qWVFmaagDndCY= 0.09135401248931885
 LOG  jsb64   : tt4oOmGUfAlbuczTPq4vZJUBPptMx3qWVFmaagDndCY= 0.11026102304458618

I'll add you performance test screen to my app and run the benchmark there.

A PR will follow.

[craftzdog]

Wow, your fix improved the performance? Cool.
The overhead of the bridge on Android looks slower than on iOS.

[craftzdog]

Ah, so how does it affect on iOS?

[pke]

haven't tested on iOS yet. But I made some observations.

1. the js-base64 library is fast, faster than the one used in your benchmarks it seems.
2. The bridge overhead is huge. See my observations over here

Learning question:
I have changed the signature of the valueToString to take a reference to a string as last parameter instead of a pointer. Why did you choose a pointer there? Is it faster in terms of std::string construction/copy or was this just an old "C" habit ;)?

[craftzdog]

Yeah, it's interesting that js-base64 is so fast.

I borrowed valueToString from another JSI repository. So, that's not my decision.
Copying an instance is usually slower than passing a pointer. It'd be fine with passing a reference instead.

[craftzdog]

perhaps it's something to do with this report?

I've managed to fix it: efdb589