Test illegal and stringified key types (#139)

* catch IndexedDB key and value errors on get, put, del and batch

* test illegal and stringified key types

* move isDataCloneError into structured-clone-test

Author: vweevers

index.js

@@ -8,7 +8,6 @@ var AbstractLevelDOWN = require('abstract-leveldown').AbstractLevelDOWN
 var inherits = require('inherits')
 var Iterator = require('./iterator')
 var mixedToBuffer = require('./util/mixed-to-buffer')
-var isDataCloneError = require('./util/is-data-clone-error')
 var setImmediate = require('./util/immediate')
 var support = require('./util/support')
 
@@ -71,7 +70,17 @@ Level.prototype.await = function (request, callback) {
 }
 
 Level.prototype._get = function (key, options, callback) {
-  this.await(this.store('readonly').get(key), function (err, value) {
+  var store = this.store('readonly')
+
+  try {
+    var req = store.get(key)
+  } catch (err) {
+    return setImmediate(function () {
+      callback(err)
+    })
+  }
+
+  this.await(req, function (err, value) {
     if (err) return callback(err)
 
     if (value === undefined) {
@@ -88,21 +97,27 @@ Level.prototype._get = function (key, options, callback) {
 }
 
 Level.prototype._del = function (key, options, callback) {
-  this.await(this.store('readwrite').delete(key), callback)
+  var store = this.store('readwrite')
+
+  try {
+    var req = store.delete(key)
+  } catch (err) {
+    return setImmediate(function () {
+      callback(err)
+    })
+  }
+
+  this.await(req, callback)
 }
 
 Level.prototype._put = function (key, value, options, callback) {
   var store = this.store('readwrite')
 
   try {
-    // Will throw a DataCloneError if the environment
-    // does not support serializing the key or value.
+    // Will throw a DataError or DataCloneError if the environment
+    // does not support serializing the key or value respectively.
     var req = store.put(value, key)
   } catch (err) {
-    if (!isDataCloneError(err)) {
-      throw err
-    }
-
     return setImmediate(function () {
       callback(err)
     })
@@ -148,9 +163,10 @@ Level.prototype._batch = function (operations, options, callback) {
   var store = this.store('readwrite')
   var transaction = store.transaction
   var index = 0
+  var error
 
   transaction.onabort = function () {
-    callback(transaction.error || new Error('aborted by user'))
+    callback(error || transaction.error || new Error('aborted by user'))
   }
 
   transaction.oncomplete = function () {
@@ -161,7 +177,14 @@ Level.prototype._batch = function (operations, options, callback) {
   function loop () {
     var op = operations[index++]
     var key = op.key
-    var req = op.type === 'del' ? store.delete(key) : store.put(op.value, key)
+
+    try {
+      var req = op.type === 'del' ? store.delete(key) : store.put(op.value, key)
+    } catch (err) {
+      error = err
+      transaction.abort()
+      return
+    }
 
     if (index < operations.length) {
       req.onsuccess = loop

test/index.js

@@ -27,3 +27,5 @@ require('abstract-leveldown/abstract/iterator-range-test').all(leveljs, test, te
 require('./custom-test')(leveljs, test, testCommon)
 require('./structured-clone-test')(leveljs, test, testCommon)
 require('./key-type-test')(leveljs, test, testCommon)
+require('./key-type-illegal-test')(leveljs, test, testCommon)
+require('./key-type-stringified-test')(leveljs, test, testCommon)

test/key-type-illegal-test.js

@@ -0,0 +1,86 @@
+/* global indexedDB */
+
+'use strict'
+
+var support = require('../util/support')
+
+// Key types not supported by IndexedDB Second Edition.
+var illegalTypes = [
+  // Allow failure because IE11 treats this as a valid key.
+  { name: 'NaN Date', allowFailure: true, key: new Date(''), error: 'DataError' },
+  { name: 'Error', key: new Error(), error: 'DataError' },
+  { name: 'Function', key: function () {}, error: 'DataError' },
+  { name: 'DOMNode', key: global.document, error: 'DataError' },
+  { name: 'Boolean(true)', key: new Boolean(true), error: 'DataError' }, // eslint-disable-line
+  { name: 'Boolean(false)', key: new Boolean(false), error: 'DataError' }, // eslint-disable-line
+]
+
+// These are only tested if the environment supports array keys.
+// Cyclical arrays are not tested because our #_serializeKey goes into a loop.
+var illegalArrays = [
+  // This type gets rejected by abstract-leveldown. As for why the error says
+  // "empty String" rather than "empty Array", see Level/abstract-leveldown#230.
+  { name: 'empty Array', key: [], message: 'key cannot be an empty String' },
+
+  // These contain a valid element to ensure we don't hit an empty key assertion.
+  { name: 'Array w/ null', key: ['a', null], error: 'DataError' },
+  { name: 'Array w/ undefined', key: ['a', undefined], error: 'DataError' },
+
+  { name: 'sparse Array', key: new Array(10), error: 'DataError' }
+]
+
+module.exports = function (leveljs, test, testCommon) {
+  test('setUp', testCommon.setUp)
+
+  if (support.test(['1'])(indexedDB)) {
+    illegalTypes = illegalTypes.concat(illegalArrays)
+  }
+
+  illegalTypes.forEach(function (item) {
+    var skip = item.allowFailure ? 'pass' : 'fail'
+    var db
+
+    test('open', function (t) {
+      db = leveljs(testCommon.location())
+      db.open(t.end.bind(t))
+    })
+
+    test('put() illegal key type: ' + item.name, function (t) {
+      db.put(item.key, 'value', verify.bind(null, t))
+    })
+
+    test('del() illegal key type: ' + item.name, function (t) {
+      db.del(item.key, verify.bind(null, t))
+    })
+
+    test('get() illegal key type: ' + item.name, function (t) {
+      db.get(item.key, function (err) {
+        verify(t, /NotFound/.test(err) ? null : err)
+      })
+    })
+
+    test('batch() put illegal key type: ' + item.name, function (t) {
+      db.batch([{ type: 'put', key: item.key, value: 'value' }], verify.bind(null, t))
+    })
+
+    test('batch() del illegal key type: ' + item.name, function (t) {
+      db.batch([{ type: 'del', key: item.key }], verify.bind(null, t))
+    })
+
+    test('close', function (t) { db.close(t.end.bind(t)) })
+
+    function verify (t, err) {
+      if (!err) {
+        t[skip]('type is treated as valid in this environment')
+        return t.end()
+      }
+
+      if ('error' in item) t.is(err.name, item.error, 'is ' + item.error)
+      if ('message' in item) t.is(err.message, item.message, item.message)
+
+      t.end()
+    }
+  })
+
+  test('tearDown', testCommon.tearDown)
+}

test/key-type-stringified-test.js

@@ -0,0 +1,38 @@
+'use strict'
+
+// Key types that are not supported by IndexedDB Second Edition, but get
+// stringified for abstract-leveldown compatibility.
+var stringifiedTypes = [
+  { name: 'true', input: true, output: 'true' },
+  { name: 'false', input: false, output: 'false' },
+  { name: 'NaN', input: NaN, output: 'NaN' }
+]
+
+module.exports = function (leveljs, test, testCommon) {
+  test('setUp', testCommon.setUp)
+
+  stringifiedTypes.forEach(function (item) {
+    test('stringified key type: ' + item.name, function (t) {
+      var db = leveljs(testCommon.location())
+
+      db.open(function (err) {
+        t.ifError(err, 'no open error')
+
+        db.put(item.input, 'value', function (err) {
+          t.ifError(err, 'no put error')
+
+          var it = db.iterator({ keyAsBuffer: false, valueAsBuffer: false })
+
+          testCommon.collectEntries(it, function (err, entries) {
+            t.ifError(err, 'no iterator error')
+            t.same(entries, [{ key: item.output, value: 'value' }])
+
+            db.close(t.end.bind(t))
+          })
+        })
+      })
+    })
+  })
+
+  test('tearDown', testCommon.tearDown)
+}

test/key-type-test.js

@@ -5,39 +5,26 @@
 var ta = require('./util/create-typed-array')
 var support = require('../util/support')
 
+// All key types supported by IndexedDB Second Edition.
 var types = [
-  { type: 'number', value: -20 },
-  { type: '+Infinity', value: Infinity },
-  { type: '-Infinity', value: -Infinity },
-  { type: 'string', value: 'test' },
-  { type: 'Date', ctor: true, value: new Date() },
-  { type: 'Array', ctor: true, allowFailure: true, value: [0, '1'] },
-  { type: 'ArrayBuffer', ctor: true, allowFailure: true, value: ta(Buffer).buffer },
-  { type: 'Int8Array', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Uint8Array', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Uint8ClampedArray', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Int16Array', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Uint16Array', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Int32Array', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Uint32Array', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Float32Array', ctor: true, allowFailure: true, createValue: ta, view: true },
-  { type: 'Float64Array', ctor: true, allowFailure: true, createValue: ta, view: true }
+  { type: 'number', key: -20 },
+  { type: '+Infinity', key: Infinity },
+  { type: '-Infinity', key: -Infinity },
+  { type: 'string', key: 'test' },
+  { type: 'Date', ctor: true, key: new Date() },
+  { type: 'Array', ctor: true, allowFailure: true, key: [0, '1'] },
+  { type: 'ArrayBuffer', ctor: true, allowFailure: true, key: ta(Buffer).buffer },
+  { type: 'Int8Array', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Uint8Array', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Uint8ClampedArray', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Int16Array', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Uint16Array', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Int32Array', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Uint32Array', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Float32Array', ctor: true, allowFailure: true, createKey: ta, view: true },
+  { type: 'Float64Array', ctor: true, allowFailure: true, createKey: ta, view: true }
 ]
 
-// TODO: test types that are not supported by IndexedDB Second Edition
-// - Date NaN (should be rejected by IndexedDB)
-// - empty array (should be rejected by abstract-leveldown)
-// - array containing null (should be rejected by IndexedDB)
-// - cyclical array (not sure)
-// - Array(10) (not sure)
-// var illegalTypes = []
-
-// TODO: test types that are not supported by IndexedDB Second Edition, but get
-// stringified for abstract-leveldown compatibility.
-// - NaN
-// - boolean
-// var stringifiedTypes = []
-
 module.exports = function (leveljs, test, testCommon) {
   var db
 
@@ -53,16 +40,16 @@ module.exports = function (leveljs, test, testCommon) {
     test('key type: ' + testName, function (t) {
       var Constructor = item.ctor ? global[item.type] : null
       var skip = item.allowFailure ? 'pass' : 'fail'
-      var input = item.value
+      var input = item.key
 
       if (item.ctor && !Constructor) {
         t[skip]('constructor is undefined in this environment')
         return t.end()
       }
 
-      if (item.createValue) {
+      if (item.createKey) {
         try {
-          input = item.createValue(Constructor)
+          input = item.createKey(Constructor)
         } catch (err) {
           t[skip]('constructor is not spec-compliant in this environment')
           return t.end()

test/structured-clone-test.js

@@ -1,8 +1,11 @@
 'use strict'
 
-var isDataCloneError = require('../util/is-data-clone-error')
 var ta = require('./util/create-typed-array')
 
+function isDataCloneError (err) {
+  return err.name === 'DataCloneError' || err.code === 25
+}
+
 // level-js supports all types of the structured clone algorithm
 // except for null and undefined (unless nested in another type).
 var types = [