oplik0 / base91-deno

more space efficient encoding than base64
MIT License
2 stars 0 forks source link

Specific input data can trigger encode to return undefined. #1

Closed pyrote closed 2 years ago

pyrote commented 2 years ago

If you try to encode certain Uint8Array's, encode will return undefined because the queue variable goes negative.

let array = new Uint8Array([114,14,42,44,172,152,61,121,54,115,14,238,1,93,77,172]);
let encoded: string  = encode(array);
console.log('Encoded '+encoded);

Will encode to 6o#DGft<cVD.N)9AaSundefinedundefined

Or with a hex string,

let hex_str = "\x8a\x04\x5a\x2a\x25\x5c\x23\x7d\x4a\x33\x64\x00\x65\x1a\x08";
let hex_array = new TextEncoder().encode(hex_str);
let encoded: string = encode(hex_array);
console.log('Encoded '+encoded);

Will encode to Yelt,de)Uz)OYvMA0kundefinedundefined

The fix is to change (numbits >= 13) to (numbits > 13) then queue will not go negative and return undefined values.

diff --git a/base91.ts b/base91.ts
index e94e6fd..2fc94e3 100644
--- a/base91.ts
+++ b/base91.ts
@@ -21,7 +21,7 @@ export function encode(uint8: Uint8Array): string {
   for (let i = 0, len = uint8.length; i < len; i++) {
     queue |= uint8[i] << numbits;
     numbits += 8;
-    if (numbits >= 13) {
+    if (numbits > 13) {
       value = queue & 8191;
       if (value > 88) {
         queue >>= 13;
oplik0 commented 2 years ago

Thank you, seems that even the less usual strings from the utf-8 sampler didn't contain anything that triggered this bug. I'll publish 1.1 with the fix. Also added one of your examples to the tests, though it's not a kind of library I'll probably be updating frequently enough for this to reoccur somehow :)

EDIT: v1.1 is published. Also, please ignore CI failing for 79fe64914ba27586f3cae6c027425472c869d802 - I forgot it checked for style and didn't run deno fmt before commiting. Fixed it before release :)

pyrote commented 2 years ago

Glad to help. It was a total edge case that showed up maybe 50/10000 runs when encoding SHA sums.