indexeddbshim / IndexedDBShim

A polyfill for IndexedDB using WebSql
Other
968 stars 191 forks source link

\0 interpreted wrongly in cursor range (nodejs). #334

Open mikelehen opened 5 years ago

mikelehen commented 5 years ago

I attempted to iterate all objects where the first element of the composite key is 'Johnson' by using the following index range (using 'Johnson\0' as the first entry that shouldn't match):

    const range = IDBKeyRange.bound(
      ['Johnson'],
      ['Johnson\0'],
      /*lowerOpen=*/ false, /*upperOpen=*/true
    );

This works as expected using native IndexedDb in a browser, but with indexeddbshim in node.js I get results like 'Johnson2 ...' that should be out of range. Based on the results I get, I think '\0' is being treated as '\...' or something (i.e. as a backslash instead of as a null character).

Repro code:


if (typeof process !== 'undefined') {
  const setGlobalVars = require('indexeddbshim');
  global.window = global;
  setGlobalVars(window, {checkOrigin: false, deleteDatabaseFiles: true});
}

const request = window.indexedDB.open("database", 1);
// Create schema
request.onupgradeneeded = event => {
    const db = event.target.result;

    const store = db.createObjectStore(
        "people",
        { keyPath: ["last", "first"] }
    );
};

request.onsuccess = event => {
  const db = request.result;
  const transaction = db.transaction(
      [ "people" ],
      "readwrite"
  );

  const mystore = transaction.objectStore("people");
  mystore.put({ last: 'Johnsom', first: 'NOPE' });
  mystore.put({ last: 'Johnson', first: 'Fred' });
  mystore.put({ last: 'Johnson', first: 'Sally' });
  mystore.put({ last: 'Johnson2', first: 'NOPE' });
  mystore.put({ last: 'Johnson\\', first: 'NOPE' });
  mystore.put({ last: 'Johnson]', first: 'NOPE' });

  transaction.oncomplete = () => {
    const transaction = db.transaction(
        [ "people" ],
        "readwrite"
    );
    const mystore = transaction.objectStore("people");

    // Now iterate people with lastname Johnson.
    const range = IDBKeyRange.bound(
      ['Johnson'],
      ['Johnson\0'],
      /*lowerOpen=*/ false, /*upperOpen=*/true
    );
    const request = mystore.openCursor(range, 'next');
    request.onsuccess = event => {
      const cursor = event.target.result;
      if (cursor) {
        console.log(cursor.value);
        cursor.continue();
      }
    };
  };
};

Result with native IndexedDb on Chrome, Firefox, and Safari:

{last: "Johnson", first: "Fred"}
{last: "Johnson", first: "Sally"}

Result with indexedDb in node:

{ last: 'Johnson', first: 'Fred' }
{ last: 'Johnson', first: 'Sally' }
{ last: 'Johnson2', first: 'NOPE' }
{ last: 'Johnson\\', first: 'NOPE' }

The last two entries shouldn't be there.

brettz9 commented 5 years ago

Which version of indexeddbshim are you using?

mikelehen commented 5 years ago

I reproduced it with the latest (4.1.0) using both node v8 and node v9.

brettz9 commented 5 years ago

Good find, thanks... Unfortunately, SQLite (at least the implementation we are using) does not work with NUL bytes (see https://github.com/mapbox/node-sqlite3/wiki/API#databaseexecsql-callback ). I wonder if https://github.com/JoshuaWise/better-sqlite3/ -- which I've been looking at using for the sake of synchronicity which can help us better mimic transaction timing -- might not have this limitation. (We still need some escaping for other purposes, however.)

I suppose our escaping of NUL bytes could instead could begin with \x01, but I guess the only way to be truly faithful here would be to add our own sorting separate from SQLite's. (I'm speaking from memory that this is the way it is done.)

PR's welcome, as this would not be at the top of my priority list (and I'm low on energy and time these days)...

mikelehen commented 5 years ago

@brettz9 Thanks for the response! I worked around the issue, so this isn't at the top of my priority list either. So definitely feel free to deprioritize (maybe revisit if/when you switch to better-sqlite3).