mafintosh / hyperdb

Distributed scalable database
MIT License
753 stars 75 forks source link

throws when trying to use `.authorized()` #98

Closed derhuerst closed 6 years ago

derhuerst commented 6 years ago

I've tried to reduce my code to this, hope it is understandable:

const hyperdb = require('hyperdb')
const findPeers = require('./lib/find-peers')

const onceAuthorized = (db, key, cb) => {
    const check = () => {
        db.authorized(key, (err, isAuthorized) => {
            if (err) cb(err)
            else if (isAuthorized === true) cb(null)
            else db.once('append', check) // todo: correct event?
        })
    }
    setImmediate(check)
}

const onReady = (db, name) => {
    findPeers(db.discoveryKey, (peer) => {
        const s = db.replicate({live: true})
        s.pipe(peer).pipe(s)
    })

    db.put('names/' + db.local.key.toString('hex'), name, (err) => {})

    const send = (content) => {
        let key = content.slice(12).trim()
        if (!validKey.test(key)) return null
        key = Buffer.from(key, 'hex')
        db.authorize(key, (err) => {})
    }

    // this crashes
    onceAuthorized(db, db.local.key, (err) => {})
}

const openChat = (dir, key, name) => {
    const db = hyperdb(dir, key, {
        valueEncoding: 'json'
    })

    db.ready((err) => {
        if (err) return null // todo
        onReady(db, name)
    })
}

On peer A, I've opened the hyperdb without a predefined key.

On peer B, I've opened the hyperdb with db.key from peer A as key. As I can tell from my logging, I have piped the replication streams together already (not sure why, the callback should have been called later).

The db.authorized(db.local.key, cb) call then throws with the following error:

/Users/j/web/hyper-chat-cli/node_modules/hyperdb/index.js:272
      var feedKey = self.feeds[head.clock[j]].key
                                              ^

TypeError: Cannot read property 'key' of undefined
    at /Users/j/web/hyper-chat-cli/node_modules/hyperdb/index.js:272:47
    at onhead (/Users/j/web/hyper-chat-cli/node_modules/hyperdb/index.js:309:10)
    at done (/Users/j/web/hyper-chat-cli/node_modules/hyperdb/index.js:800:5)
    at /Users/j/web/hyper-chat-cli/node_modules/hyperdb/index.js:467:5
    at /Users/j/web/hyper-chat-cli/node_modules/hypercore/index.js:134:15
    at apply (/Users/j/web/hyper-chat-cli/node_modules/thunky/index.js:44:12)
    at process._tickCallback (internal/process/next_tick.js:114:19)

Using Node v9.10.1, hyperdb@3.0.0-1.

mafintosh commented 6 years ago

Could you try master real quick to see if the issue is there as well? Def looks like a bug tho

derhuerst commented 6 years ago

Yep, couldn't reproduce this on master. Seems fixed, thanks!

mafintosh commented 6 years ago

Great! I'll do a release today

jimpick commented 6 years ago

I'm seeing this one too, on master. Working on a test case...

jimpick commented 6 years ago

Here's a quick test case:

tape('unauthorized writer fails "authorized" api after some writes', function (t) {
  var a = create.one()
  a.ready(function () {
    a.put('foo', 'bar', function (err) {
      t.error(err)
      a.put('foo', 'bar2', function (err) {
        t.error(err)
        a.put('foo', 'bar3', function (err) {
          t.error(err)
          a.put('foo', 'bar4', function (err) {
            t.error(err)
            var b = create.one(a.key)
            b.ready(function () {
              replicate(a, b, function () {
                b.authorized(b.local.key, function (err, auth) {
                  t.error(err)
                  t.equals(auth, false)
                  t.end()
                })
              })
            })
          })
        })
      })
    })
  })
})
$ npx tape test/auth.js
TAP version 13
# unauthorized writer fails "authorized" api after some writes
ok 1 null
ok 2 null
ok 3 null
ok 4 null
/Users/jim/backblend-jpimac/clients/research-team-tool/hyperdb-tests/hyperdb/index.js:277
      var feedKey = self.feeds[head.clock[j]].key
                                              ^

TypeError: Cannot read property 'key' of undefined
    at /Users/jim/backblend-jpimac/clients/research-team-tool/hyperdb-tests/hyperdb/index.js:277:47
    at onhead (/Users/jim/backblend-jpimac/clients/research-team-tool/hyperdb-tests/hyperdb/index.js:314:10)
    at done (/Users/jim/backblend-jpimac/clients/research-team-tool/hyperdb-tests/hyperdb/index.js:819:5)
    at /Users/jim/backblend-jpimac/clients/research-team-tool/hyperdb-tests/hyperdb/index.js:478:5
    at /Users/jim/backblend-jpimac/clients/research-team-tool/hyperdb-tests/hyperdb/node_modules/hypercore/index.js:134:15
    at apply (/Users/jim/backblend-jpimac/clients/research-team-tool/hyperdb-tests/hyperdb/node_modules/thunky/index.js:44:12)
    at process._tickCallback (internal/process/next_tick.js:152:19)

I have to head out for a couple of hours. I'll work on a fix if it's needed.

jimpick commented 6 years ago

Is this the right fix? (edit: nope)

diff --git a/index.js b/index.js
index d46943e..93262de 100644
--- a/index.js
+++ b/index.js
@@ -274,7 +274,7 @@ HyperDB.prototype.authorized = function (key, cb) {
     }

     for (var j = 0; j < max; j++) {
-      var feedKey = self.feeds[head.clock[j]].key
+      var feedKey = self.feeds[j].key
       if (feedKey.equals(key)) {
         return cb(null, true)
       }
jimpick commented 6 years ago

Don't use the fix above ... it's wrong. I'm working on it.

mafintosh commented 6 years ago

Fixed in master, including @jimpick's test case. I'm doing a new release tonight. Wanna get a few more fixes in.

mafintosh commented 6 years ago

Now released