Level / memdown

In-memory abstract-leveldown store for Node.js and browsers.
MIT License
287 stars 37 forks source link

Pass ltgt as comparator to rbtree, to make it buffer-aware #41

Closed vweevers closed 8 years ago

vweevers commented 8 years ago

Otherwise, buffer keys might be considered equal (because JS calls buffer.toString() when comparing), and one key will overwrite the other.

nolanlawson commented 8 years ago

This is a cool PR and taught me something about how Buffer.toString() works. Thanks!

nolanlawson commented 8 years ago

For those interested, here's some more explanation:

var one = new Buffer('80', 'hex' )
var two = new Buffer('c0', 'hex' )
one.toString('binary') === two.toString('binary') // false
one.toString('hex')    === two.toString('hex')    // false
one.toString()         === two.toString()         // true

The reason the last one is true is that if you don't pass an encoding to toString(), it uses 'utf-8' as the default. When "encoded" as UTF-8, both of those buffers have code points that are too high (0x80 or 128 and 0xc0 or 192, repectively, which are out of the ASCII range of 0-127) so you get "�", aka the replacement character at char code 65533, aka the thing it shows when it's not able to encode. And "�" == "�", hence the bug.

vweevers commented 8 years ago

Thanks for the quick merge and educational effort :+1:

nolanlawson commented 8 years ago

Published in 1.1.1, thanks!