Skip to content

Commit

Permalink
fix(Brain): avoid retaining dangerous robot/brain references
Browse files Browse the repository at this point in the history
  • Loading branch information
mistydemeo committed Mar 5, 2019
1 parent d2b57bd commit 947d988
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"devDependencies": {
"chai": "~2.1.0",
"coveralls": "^3.0.2",
"is-circular": "^1.0.2",
"mocha": "^5.2.0",
"mockery": "^1.4.0",
"nyc": "^13.0.0",
Expand Down
13 changes: 9 additions & 4 deletions src/brain.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ let reconstructUserIfNecessary = function (user, robot) {
// populating the new user with its values.
// Also add the `robot` field so it gets a reference.
user.robot = robot
let newUser = new User(id, user)
delete user.robot

return new User(id, user)
return newUser
} else {
return user
}
Expand All @@ -39,7 +41,9 @@ class Brain extends EventEmitter {
users: {},
_private: {}
}
this.robot = robot
this.getRobot = function () {
return robot
}

this.autoSave = true

Expand Down Expand Up @@ -146,7 +150,7 @@ class Brain extends EventEmitter {
if (data && data.users) {
for (let k in data.users) {
let user = this.data.users[k]
this.data.users[k] = reconstructUserIfNecessary(user, this.robot)
this.data.users[k] = reconstructUserIfNecessary(user, this.getRobot())
}
}

Expand All @@ -168,7 +172,7 @@ class Brain extends EventEmitter {
if (!options) {
options = {}
}
options.robot = this.robot
options.robot = this.getRobot()

if (!user) {
user = new User(id, options)
Expand All @@ -179,6 +183,7 @@ class Brain extends EventEmitter {
user = new User(id, options)
this.data.users[id] = user
}
delete options.robot

return user
}
Expand Down
5 changes: 5 additions & 0 deletions test/brain_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ chai.use(require('sinon-chai'))

const expect = chai.expect

const isCircular = require('is-circular')

// Hubot classes
const Brain = require('../src/brain')
const User = require('../src/user')
Expand Down Expand Up @@ -65,6 +67,7 @@ describe('Brain', function () {
expect(user.constructor.name).to.equal('User')
expect(user.id).to.equal('4')
expect(user.name).to.equal('new')
expect(isCircular(this.brain)).to.be.false
})
})

Expand Down Expand Up @@ -326,6 +329,8 @@ describe('Brain', function () {
for (let user of this.brain.usersForRawFuzzyName('Guy One')) {
expect(user.constructor.name).to.equal('User')
}

expect(isCircular(this.brain)).to.be.false
})
})
})

0 comments on commit 947d988

Please sign in to comment.