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

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

Open
mikelehen opened this issue Feb 3, 2019 · 4 comments
Open

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

mikelehen opened this issue Feb 3, 2019 · 4 comments
Labels

Comments

@mikelehen
Copy link

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
Copy link
Collaborator

brettz9 commented Feb 3, 2019

Which version of indexeddbshim are you using?

@mikelehen
Copy link
Author

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

@brettz9 brettz9 added the Bug label Feb 4, 2019
@brettz9
Copy link
Collaborator

brettz9 commented Feb 4, 2019

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
Copy link
Author

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants