feat: custom length encoding and decoding #8
Conversation
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
This PR allows a custom length encoding/decoding function to be passed to `encode`/`decode`.
There's also a int32BE fixed length encoder/decoder. e.g.
```js
const lp = require('it-length-prefixed')
const { int32Encode, int32Decode } = require('it-length-prefixed')
await pipe(
[Buffer.from('hello world')],
lp.encode({ lengthEncoder: int32Encode }),
lp.decode({ lengthDecoder: int32Decode }),
async source => {
for await (const chunk of source) {
console.log(chunk.toString())
}
}
)
```
See updated README for more info.
BREAKING CHANGE: Additional validation now checks for messages with a length that is too long to prevent a possible DoS attack. The error code `ERR_MSG_TOO_LONG` has changed to `ERR_MSG_DATA_TOO_LONG` and the error code `ERR_MSG_LENGTH_TOO_LONG` has been added.
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
| if (dataLength > options.maxDataLength) { | ||
| throw Object.assign(new Error('message too long'), { code: 'ERR_MSG_TOO_LONG' }) | ||
| throw Object.assign(new Error('message data too long'), { code: 'ERR_MSG_DATA_TOO_LONG' }) |
There was a problem hiding this comment.
Please note this change! I noticed that while reading message length an attacker could send Buffers of 0x80 (128 or greater) and we're continue buffering it up indefinitely until we run out of memory. I believe this is possible in pull-length-prefixed also.
test/decode.spec.js
Outdated
| it('should not decode message length that is too long', async () => { | ||
| // A value < 0x80 signifies end of varint so pass buffers of >= 0x80 | ||
| // so that it will keep throwing a RangeError until we reach the max length | ||
| const lengths = times(randomInt(5, 10), () => Buffer.alloc(MAX_LENGTH_LENGTH / 4).fill(0x80)) |
There was a problem hiding this comment.
Is randomizing the times executed here needed? This should fail consistently on the 5th attempt correct?
There was a problem hiding this comment.
Should do yes, will switch to 5.
README.md
Outdated
| @@ -61,23 +61,31 @@ console.log(decoded) | |||
|
|
|||
| - `opts: Object`, optional | |||
| - `poolSize: 10 * 1024`: Buffer pool size to allocate up front | |||
| - `minPoolSize: 147`: The minimum size the pool can be before it is re-allocated. Note this is important | |||
There was a problem hiding this comment.
Note this is important
This ties directly to ones ability to read the length of the data correct? In decode.js the comment signifies this is equivalent to the number of bytes needed to read Varint.encode(Number.MAX_VALUE).length (147), Number.MAX_VALUE is a float though. Should it be Varint.encode(Number.MAX_SAFE_INTEGER).length (8)?
There was a problem hiding this comment.
When allocating a buffer the docs say "<integer>" so I assume then the biggest you can allocate would be Number.MAX_SAFE_INTEGER meaning yes, you're right this should be 8 bytes, not 147. Good catch.
This PR allows a custom length encoding/decoding function to be passed to
encode/decode.There's also a int32BE fixed length encoder/decoder. e.g.
See updated README for more info.
resolves #5
BREAKING CHANGE: Additional validation now checks for messages with a length that is too long to prevent a possible DoS attack. The error code
ERR_MSG_TOO_LONGhas changed toERR_MSG_DATA_TOO_LONGand the error codeERR_MSG_LENGTH_TOO_LONGhas been added.