Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Range tests on zero-length buffer+string are incompatible with bytewise, charwise & IDB #318

Closed
vweevers opened this issue Dec 25, 2018 · 2 comments
Labels
bug Something isn't working

Comments

@vweevers
Copy link
Member

vweevers commented Dec 25, 2018

They assume that a zero-length string or zero-length Buffer means "not defined". IMO that should be left up to an implementation.

if (testCommon.bufferKeys) {
rangeTest('test iterator with gte as empty buffer', {
gte: Buffer.alloc(0)
}, data)
rangeTest('test iterator with start as empty buffer - legacy', {
start: Buffer.alloc(0)
}, data)
rangeTest('test iterator with lte as empty buffer', {
lte: Buffer.alloc(0)
}, data)
rangeTest('test iterator with end as empty buffer - legacy', {
end: Buffer.alloc(0)
}, data)
}
rangeTest('test iterator with gte as empty string', {
gte: ''
}, data)
rangeTest('test iterator with start as empty string - legacy', {
start: ''
}, data)
rangeTest('test iterator with lte as empty string', {
lte: ''
}, data)
rangeTest('test iterator with end as empty string - legacy', {
end: ''
}, data)

In bytewise, charwise and IndexedDB, an empty string is a significant type. In all three, the sort order of types is:

  • null (not supported by IDB)
  • false (not supported by IDB)
  • true (not supported by IDB)
  • number (numeric)
  • date (numeric, epoch offset)
  • binary (bitwise) (not supported by charwise and IDB First Edition)
  • string (lexicographic)
  • array (componentwise)
  • undefined (not supported by IDB)

For example, charwise encodes an empty string as 'J'.

I suggest:

@vweevers vweevers added the bug Something isn't working label Dec 25, 2018
@vweevers
Copy link
Member Author

Note that level-js is currently passing the test suite of abstract-leveldown@5 because abstract-leveldown used to filter out range options that were a zero-length string or buffer.

@vweevers
Copy link
Member Author

vweevers commented Dec 25, 2018

Correction: unlike bytewise, IDB sorts binary after string. For reference, here's a quick test of IDB type order:

Click to expand
function cmp (a, b) {
  // # https://www.w3.org/TR/IndexedDB-2/#key-construct
  // "Let ta be the type of a, let tb be the type of b."
  var ta = type(a)
  var tb = type(b)

  // "If ta is array and tb is binary, string, date or number, return 1."
  if (ta === 'array' && (tb === 'binary' || tb === 'string' || tb === 'date' || tb === 'number')) return 1

  // "If tb is array and ta is binary, string, date or number, return -1."
  if (tb === 'array' && (ta === 'binary' || ta === 'string' || ta === 'date' || ta === 'number')) return -1

  // "If ta is binary and tb is string, date or number, return 1."
  if (ta === 'binary' && (tb === 'string' || tb === 'date' || tb === 'number')) return 1

  // "If tb is binary and ta is string, date or number, return -1."
  if (tb === 'binary' && (ta === 'string' || ta === 'date' || ta === 'number')) return -1

  // "If ta is string and tb is date or number, return 1."
  if (ta === 'string' && (tb === 'date' || tb === 'number')) return 1

  // "If tb is string and ta is date or number, return -1."
  if (tb === 'string' && (ta === 'date' || ta === 'number')) return -1

  // "If ta is date and tb is number, return 1."
  if (ta === 'date' && (tb === 'number')) return 1

  // "If tb is date and ta is number, return -1."
  if (tb === 'date' && (ta === 'number')) return -1

  throw new Error('value comparison is not implemented')
}

function type (value) {
  const t = typeof value

  if (t === 'string' || t === 'number') return t
  if (Buffer.isBuffer(value)) return 'binary' // stored same as arraybuffer
  if (Array.isArray(value)) return 'array'
  if (Object.prototype.toString.call(value) === '[object Date]') return 'date'

  throw new Error('unsupported type')
}

// [ 3, <date>, '', <Buffer>, [] ]
console.log([
  new Date(),
  '',
  3,
  [],
  Buffer.alloc(0)
].sort(cmp))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant