dmonad / lib0

Monorepo of isomorphic utility functions
MIT License
345 stars 62 forks source link

Throw when reading past end of array #43

Closed broofa closed 1 year ago

broofa commented 2 years ago

Putting this up as an (untested) fix for #42. This isn't duck-typing but, instead, enforces the known array length. Has the additional benefit of catching other cases where the same infinite-loop problem may occur, such as this:

import { createDecoder, readVarUint } from 'lib0/decoding';
const decoder = createDecoder(new Uint8Array([200, 200, 200, 200]));
readVarUint(decoder);
NilSet commented 1 year ago

I came up with the same solution before I found this PR. I also wrote some tests which meet this repo's code coverage requirements, if you would like to use them.

/**
 * @param {t.TestCase} tc
 */
export const testInvalidVarIntEncoding = tc => {
  const encoded = new Uint8Array(1)
  encoded[0] = 255
  const decoder = decoding.createDecoder(encoded)
  let threw = false
  try {
    decoding.readVarInt(decoder)
  } catch (e) {
    if (e instanceof Error && e.message === 'Unexpected end of array') {
      threw = true
    }
  }
  t.assert(threw === true)
}

/**
 * @param {t.TestCase} tc
 */
export const testInvalidVarUintEncoding = tc => {
  const encoded = new Uint8Array(1)
  encoded[0] = 255
  const decoder = decoding.createDecoder(encoded)
  let threw = false
  try {
    decoding.readVarUint(decoder)
  } catch (e) {
    if (e instanceof Error && e.message === 'Unexpected end of array') {
      threw = true
    }
  }
  t.assert(threw === true)
}
broofa commented 1 year ago

@NilSet 'happy to merge a PR from you with those changes into this one, but @dmonad doesn't seem to feel this is important so it doesn't seem prudent to put time into this PR.

dmonad commented 1 year ago

Thank you @broofa for the PR. I'll merge it and get it into a release today. I think this issue is important. The comment you mentioned was about duck typing, which I don't want to support in this library.