search

dignifiedquire / pull-length-prefixed

Streaming length prefixed buffers with pull-streams
MIT License
8 stars 18 forks source link

feat: add maxLength to allow controlling max buffer length during decode #9

Closed dryajov closed 7 years ago

dryajov commented 7 years ago

@dignifiedquire adding this since we need to control the max msg size in circuit.

FYI, some tests are failing, but they were failing before my changes, I haven't yet taken a look at it, will do soon, but you might know whats going on.

dryajov commented 7 years ago

One more thing, I have tentatively set maxLength to 1024 as a default, but we might want to increase that to something larger if its too low of a number.

dignifiedquire commented 7 years ago

@dryajov I've updated master to have passing tests again. Please rebase onto it so I can merge

dryajov commented 7 years ago

@dignifiedquire thanks, I'll update the length!

dryajov commented 7 years ago

@dignifiedquire think this is ready now?

dryajov commented 7 years ago

@dignifiedquire any reason to not just do require('safe-buffer') and let it polyfill automatically here - https://github.com/dignifiedquire/pull-length-prefixed/blob/master/src/encode.js#L3?

dryajov commented 7 years ago

tests are failing on node 4 even tho I'm pulling safe-buffer, looking into it.

dryajov commented 7 years ago

Seems like in node 4, if the fill value is something other than a string and no encoding is explicitly specified, then it will not encode it with anything and simply fill in the raw bytes - https://github.com/nodejs/node/blob/v4.x/lib/buffer.js#L109, which is different from how later versions behave, where buffer is also defaulted to 'utf8'.

dryajov commented 7 years ago

@dignifiedquire can we get this published to npm please 😄? This is needed for https://github.com/libp2p/js-libp2p-circuit/pull/9

dignifiedquire commented 7 years ago

sorry for the delay, published a new version @dryajov