From 01b2d50b65e16dd439177c8ba3599b8cc0102a15 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Fri, 3 Apr 2020 22:19:49 +0300 Subject: [PATCH] Breaking: fix iterating buffer keys that contain bytes 196-255 (#88) --- README.md | 2 +- leveldown.js | 21 +++++++-- test/index.js | 116 +++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 109 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index bb7ef58..c6bd458 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ The `prefix` must be a string. If omitted, the effective prefix is two separator The optional `options` parameter has the following `subleveldown` specific properties: -- `separator` _(string, default: `'!'`)_ Character for separating sublevel prefixes from user keys and each other. Should be outside the character (or byte) range of user keys. +- `separator` _(string, default: `'!'`)_ Character for separating sublevel prefixes from user keys and each other. Must sort before characters used in prefixes. An error will be thrown if that's not the case. - `open` _(function)_ Optional open hook called when the underlying `levelup` instance has been opened. The hook receives a callback which must be called to finish opening. Any other `options` are passed along to the underlying [`levelup`][levelup] and [`encoding-down`][encoding-down] constructors. See their documentation for further details. diff --git a/leveldown.js b/leveldown.js index 0e10283..5a26e9f 100644 --- a/leveldown.js +++ b/leveldown.js @@ -7,7 +7,6 @@ var matchdown = require('./matchdown') var rangeOptions = 'start end gt gte lt lte'.split(' ') var defaultClear = abstract.AbstractLevelDOWN.prototype._clear var hasOwnProperty = Object.prototype.hasOwnProperty -var END = Buffer.from([0xff]) function concat (prefix, key, force) { if (typeof key === 'string' && (force || key.length)) return prefix + key @@ -55,6 +54,15 @@ function SubDown (db, prefix, opts) { if (prefix[0] === separator) prefix = prefix.slice(1) if (prefix[prefix.length - 1] === separator) prefix = prefix.slice(0, -1) + var code = separator.charCodeAt(0) + 1 + var ceiling = String.fromCharCode(code) + + Buffer.from(prefix).forEach(function (byte) { + if (byte <= code) { + throw new RangeError('Prefix must sort after ' + code) + } + }) + this.db = db this.leveldown = null this.ownPrefix = separator + prefix + separator @@ -68,8 +76,11 @@ function SubDown (db, prefix, opts) { return concat(self.prefix, x || '', true) }, lt: function (x) { - if (Buffer.isBuffer(x) && !x.length) x = END - return concat(self.prefix, x || '\xff') + if (!x || isEmptyBuffer(x)) { + return self.prefix.slice(0, -1) + ceiling + } else { + return concat(self.prefix, x) + } } } @@ -154,6 +165,10 @@ function isRangeOption (k) { return rangeOptions.indexOf(k) !== -1 } +function isEmptyBuffer (key) { + return Buffer.isBuffer(key) && key.length === 0 +} + // TODO (refactor): use addRestOptions instead function extend (xopts, opts) { xopts.keys = opts.keys diff --git a/test/index.js b/test/index.js index f5ce0d3..a0b6309 100644 --- a/test/index.js +++ b/test/index.js @@ -13,35 +13,39 @@ var abstract = require('abstract-leveldown') var inherits = require('util').inherits // Test abstract-leveldown compliance -suite({ - test: test, - factory: function () { - return subdown(levelup(memdown()), 'test') - }, - - // Unsupported features - seek: false, - createIfMissing: false, - errorIfExists: false, - - // Opt-in to new clear() tests - clear: true +function runSuite (factory) { + suite({ + test: test, + factory: factory, + + // Unsupported features + seek: false, + createIfMissing: false, + errorIfExists: false, + + // Opt-in to new clear() tests + clear: true + }) +} + +// Test basic prefix +runSuite(function factory () { + return subdown(levelup(memdown()), 'test') +}) + +// Test empty prefix +runSuite(function factory () { + return subdown(levelup(memdown()), '') +}) + +// Test custom separator +runSuite(function factory () { + return subdown(levelup(memdown()), 'test', { separator: '%' }) }) // Test without a user-provided levelup layer -suite({ - test: test, - factory: function () { - return subdown(memdown(), 'test') - }, - - // Unsupported features - seek: false, - createIfMissing: false, - errorIfExists: false, - - // Opt-in to new clear() tests - clear: true +runSuite(function factory () { + return subdown(memdown(), 'test') }) // Additional tests for this implementation @@ -403,6 +407,66 @@ test('SubDb main function', function (t) { } }) +// https://github.com/Level/subleveldown/issues/87 +test('can store any key', function (t) { + t.test('iterating buffer keys with bytes above 196', function (t) { + t.plan(3) + + var db = levelup(memdown()) + var sub = subdb(db, 'test', { keyEncoding: 'binary' }) + + sub.once('open', function () { + const batch = sub.batch() + + for (let i = 0; i < 256; i++) { + batch.put(Buffer.from([i]), 'test') + } + + batch.write(function (err) { + t.ifError(err, 'no write error') + + concat(sub.iterator(), function (err, entries) { + t.ifError(err, 'no concat error') + t.is(entries.length, 256, 'sub yields all entries') + }) + }) + }) + }) + + t.test('range logic', function (t) { + const db = levelup(memdown()) + const a = subdb(db, 'a', { separator: '#' }) + const aA = subdb(a, 'a', { separator: '#' }) + const b = subdb(db, 'b', { separator: '#' }) + const next = after(3, verify) + + a.once('open', next) + aA.once('open', next) + b.once('open', next) + + function wrapper (sub) { + return reachdown(sub, 'subleveldown')._wrap + } + + function verify () { + const ranges = [ + wrapper(a).gt(), + wrapper(aA).gt(), + wrapper(aA).lt(), + wrapper(a).lt(), + wrapper(b).gt(), + wrapper(b).lt() + ] + + t.same(ranges, ['#a#', '#a##a#', '#a##a$', '#a$', '#b#', '#b$']) + t.same(ranges.slice().sort(), ranges) + t.end() + } + }) + + t.end() +}) + // Test that we peel off the levelup, deferred-leveldown and encoding-down // layers from db, but stop at any other intermediate layer like encrypt-down, // cachedown, etc.