search

mafintosh / dns-packet

An abstract-encoding compliant module for encoding / decoding DNS packets
MIT License
207 stars 71 forks source link

Fix buffer management for stream encoding. #29

Closed pusateri closed 6 years ago

pusateri commented 6 years ago

Stream encode/decode worked fine for minimal responses but more complicated responses got corrupted because the buffer wasn't constructed correctly. This adds a test case to catch the problem and then fixes it. I also removed the streamDecode.bytes test because there is nothing to compare it to that makes sense.

pusateri commented 6 years ago

I think this failed on Node 4.0 because Buffer.from() was introduced in node 5.10.0.

pusateri commented 6 years ago

I don't want to copy the Buffer (which Buffer.from alleviates) so you can wait until 4-30-2018 when Node 4.0 is EOL if you want to merge this. Alternatively, we can drop Node 4.0 support early.

pusateri commented 6 years ago

Buffer.from() should be working fine in Node 4.0 with safe-buffer. According to this: https://github.com/feross/safe-buffer/pull/5 There must be another problem it failed in 4.0.

pusateri commented 6 years ago

This is failing in safe-buffer: Buffer.from(sbuf.buffer, sbuf.byteOffset + 2, sbuf.byteLength - 2) Line 830 Maybe I can rewrite it to make it work with safe-buffer.

silverwind commented 6 years ago

This is failing in safe-buffer: Buffer.from(sbuf.buffer, sbuf.byteOffset + 2, sbuf.byteLength - 2)

Interesting. May be worth to report at https://github.com/feross/safe-buffer