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

Filter duplicate interfaces from incoming #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/inject.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ test('custom: keys injected', t => {
const keys = ssbKeys.generate()
const config = Config('testnet', { keys })

t.equal(keys.public, config.keys.public, 'keys exist')
t.equal(keys.public, config.keys.public, 'keys exist')
t.end()
})
76 changes: 46 additions & 30 deletions util/incoming-connections.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ module.exports = function (config) {
return defaultPort
}


//legacy configuration didn't have a scopes concept,
//so interpret that as every scope at once.
//I think there is probably a better way to do this,
//but am fairly sure this will probably work.
// legacy configuration didn't have a scopes concept,
// so interpret that as every scope at once.
// I think there is probably a better way to do this,
// but am fairly sure this will probably work.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style changes are best done in their own commit, it makes it significantly easier to review (I can browse the commits and focus my attention only on the non-style commits), and makes the git blame process easier too.

const allScopes = ['device', 'local', 'public']

// If `config.host` is defined then we don't need to enumerate interfaces.
Expand Down Expand Up @@ -68,35 +67,52 @@ module.exports = function (config) {
incoming = Object.keys(defaultPorts).map((service) => {
return {
service,
interfaces: Object.values(interfaces).reduce((acc, val) => {
// Future TODO: replace with shiny new `Array.prototype.flat()`.
return acc.concat(val)
}, []).filter(item => {
// We want to avoid scoped IPv6 addresses since they don't seem to
// play nicely with the Node.js networking stack. These addresses
// often start with `fe80` and throw EINVAL when we try to bind to
// them.
return item.scopeid == null || item.scopeid === 0
}).map(item => {
interfaces: Object.values(interfaces)
.reduce((acc, val) => {
// Future TODO: replace with shiny new `Array.prototype.flat()`.
return acc.concat(val)
}, [])
.filter(item => {
// We want to avoid scoped IPv6 addresses since they don't seem to
// play nicely with the Node.js networking stack. These addresses
// often start with `fe80` and throw EINVAL when we try to bind to
// them.
return item.scopeid == null || item.scopeid === 0
})
.reduce((acc, cur) => {
// It's possible to have two interfaces with the same IP address,
// but we don't want to try to listen on both of them. This only
// adds the interface to the list if it hasn't already been added.
const found = acc.find((item) =>
item.address === cur.address
)

if (found == null) {
acc.push(cur)
}

return acc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To simplify this code, I would probably refactor this arrow function to be a named function uniqueAddresses that's defined as a top-level function and then I'd use it as reduce(uniqueAddresses). I would do the same for the other parts of this chain of array methods.

}, [])
.map(item => {
// This bit is simple because the ssb-config options for `incoming`
// can either be hardcoded or directly inferred from `interfaces`.

//if an interface is internal, it can only be accessed from the device.
//if it's got a private ip address it can only be accessed from some network.
//otherwise, it's presumably a publically accessable address.
var scope = (
item.internal ? 'device'
: ip.isPrivate(item.address) ? 'local'
: 'public'
)
// if an interface is internal, it can only be accessed from the device.
// if it's got a private ip address it can only be accessed from some network.
// otherwise, it's presumably a publically accessable address.
var scope = (
item.internal ? 'device'
: ip.isPrivate(item.address) ? 'local'
: 'public'
)

return {
host: item.address,
port: getPort(service),
scope: [scope],
transform: 'shs'
}
})
return {
host: item.address,
port: getPort(service),
scope: [scope],
transform: 'shs'
}
})
}
}).reduce((result, obj) => {
// This `reduce()` step is necessary because we need to return an object
Expand Down