electron-userland / electron-json-storage

:package: Easily write and read user settings in Electron apps
1.44k stars 80 forks source link

URI encoded Keys are not decoded #75

Closed Yoann-Pearson closed 7 years ago

Yoann-Pearson commented 7 years ago

If I add a storage value with a key that contains URI encoded values (ex: "test:value"), if I do a 'storage.getAll', this value will not be extracted correctly.

Also, doing a storage.keys, the returned key will not be URI decoded, so I can not 'get' the stored data using that key without manually doing a decodeURIComponent on it.

jviotti commented 7 years ago

Can you provide me with a failing test case? I tried

it('should be able to store a valid JSON object in a file with a colon', function(done) {
  async.waterfall([
    function(callback) {
      storage.set('test:value', { foo: 'bar' }, callback);
    },
    function(callback) {
      storage.get('test:value', callback);
    }
  ], function(error, data) {
    m.chai.expect(error).to.not.exist;
    m.chai.expect(data).to.deep.equal({ foo: 'bar' });
    done();
  });
});

But that seems to work fine, at least on macOS.

jviotti commented 7 years ago

I'll push my changes just in case the CI servers reveal an issue in another platform.

Yoann-Pearson commented 7 years ago

Those 2 test cases fails on windows.

    describe('given stored keys with colon', function () {

      beforeEach(function (done) {
        async.parallel([
          _.partial(storage.set, 'foo', { name: 'foo' }),
          _.partial(storage.set, 'bar:colon', { name: 'bar' })
        ], done);
      });

      it('should return all stored keys', function (done) {
        storage.getAll(function (error, data) {
          m.chai.expect(error).to.not.exist;
          m.chai.expect(data).to.deep.equal({
            foo: { name: 'foo' },
            'bar:colon': { name: 'bar' }
          });
          done();
        });
      });

    });
it('should detect keys with colons', function(done) {
      async.waterfall([
        function(callback) {
          async.parallel([
            _.partial(storage.set, 'one', 'foo'),
            _.partial(storage.set, 'two', 'bar'),
            _.partial(storage.set, 'three:colon', 'baz')
          ], callback);
        },
        function(result, callback) {
          storage.keys(callback);
        }
      ], function(error, keys) {
        m.chai.expect(error).to.not.exist;
        m.chai.expect(keys).to.deep.equal([
          'one',
          'two',
          'three:colon'
        ]);
        done();
      });
    });
jviotti commented 7 years ago

Ah, I see what you mean now. See https://github.com/electron-userland/electron-json-storage/pull/78

jviotti commented 7 years ago

Check v3.0.7