-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Reimplement a synchronous method - Fixes #13 #14
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,29 +8,41 @@ const defaults = { | |
v4: '127.0.0.1' | ||
}; | ||
|
||
function internalIp(family) { | ||
return defaultGateway[family]().then(result => { | ||
const interfaces = os.networkInterfaces(); | ||
const gatewayIp = ipaddr.parse(result.gateway); | ||
let ret; | ||
|
||
// Look for the matching interface in all local interfaces | ||
Object.keys(interfaces).some(name => { | ||
return interfaces[name].some(addr => { | ||
const prefix = ipaddr.parse(addr.netmask).prefixLengthFromSubnetMask(); | ||
const net = ipaddr.parseCIDR(`${addr.address}/${prefix}`); | ||
|
||
if (net[0].kind() === gatewayIp.kind() && gatewayIp.match(net)) { | ||
ret = net[0].toString(); | ||
} | ||
|
||
return Boolean(ret); | ||
}); | ||
function match(gateway, family) { | ||
const interfaces = os.networkInterfaces(); | ||
const gatewayIp = ipaddr.parse(gateway); | ||
let ret; | ||
|
||
// Look for the matching interface in all local interfaces | ||
Object.keys(interfaces).some(name => { | ||
return interfaces[name].some(addr => { | ||
const prefix = ipaddr.parse(addr.netmask).prefixLengthFromSubnetMask(); | ||
const net = ipaddr.parseCIDR(`${addr.address}/${prefix}`); | ||
|
||
if (net[0].kind() === gatewayIp.kind() && gatewayIp.match(net)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just noticed this: could you add a check for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a direct copy paste from the code as it presently is in master. mentioning only to confirm that you want to piggy back on this PR for the change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right, I can fix that later myself, no need to do it here. |
||
ret = net[0].toString(); | ||
} | ||
|
||
return Boolean(ret); | ||
}); | ||
}); | ||
|
||
return ret ? ret : defaults[family]; | ||
} | ||
|
||
return ret ? ret : defaults[family]; | ||
function promise(family) { | ||
return defaultGateway[family]().then(result => { | ||
return match(result.gateway, family); | ||
}).catch(() => defaults[family]); | ||
} | ||
|
||
module.exports.v6 = () => internalIp('v6'); | ||
module.exports.v4 = () => internalIp('v4'); | ||
function sync(family) { | ||
const result = defaultGateway[family].sync(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to wrap this line in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do |
||
return match(result.gateway, family); | ||
} | ||
|
||
module.exports.v6 = () => promise('v6'); | ||
module.exports.v4 = () => promise('v4'); | ||
|
||
module.exports.v6.sync = () => sync('v6'); | ||
module.exports.v4.sync = () => sync('v4'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
findIp
might be a better name.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from line 16
// Look for the matching interface in all local interfaces
- that's why I chose the name. doesn't much matter to me what you think it should be, happy to change it, but I thought it was appropriate given that commentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I still think
findIp
is more appropriate. The matching is only part of the process.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aye, will change