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

WIP: Remove the netifaces dependency #471

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

pimveldhuisen
Copy link
Member

Instead use the standard 'socket' library.

This is hard to test for all kinds of NAT and network setups, and it might result in non critical failures, so it might be hard to verify.

Another option is to only use this method as a fallback when netifaces fails.

@whirm
Copy link
Contributor

whirm commented Apr 26, 2016

This PR should go to next

@devos50
Copy link
Contributor

devos50 commented Mar 10, 2017

@pimveldhuisen what's the status of this?

@pimveldhuisen
Copy link
Member Author

Pff, this was a long time ago...
If I recall correctly, we decided to implement this as a fallback solution rather than a replacement of netifaces. That PR got merged.
I still think it would be nice to remove the dependency, but it would be hard to verify that it would not break anything on any machine / OS / NAT / Router combination.

@synctext
Copy link
Member

np. we can rely on our Beta testers....

@pimveldhuisen
Copy link
Member Author

I've just rebased the pull request on the new code. Before merging the code needs some polish, mainly by having retries and error handling, but I think we should have a discussion about whether this change is conceptually a good idea first.

This pull request removes the netifaces dependency, but it does not provides a direct replacement for netifaces functionality. Instead, it takes a different approach to the two tasks netifaces has:

  • The first task is to find the lan ip. Netifaces does this by quering the os for a list of interfaces, selecting the interface that is most likely to be used for internet traffic, and takes the reported lan value. The PR simply sets up a socket connection to an internet address, and checks which lan ip the os uses for this.

  • Another task is to determine whether an ip that makes an incoming connection is from our lan. Netifaces does this properly by checking it versus our lan ip and netmask. This PR bases its judgement on the idea that ip adresses are inherently lan or wan. If we are on the 192.168.0.x subnet, the address 192.168.1.12 might not be in our subnet, but it also should not be possible to receive connections from the address. We can thus just check ip addresses against the hardcoded lan ranges. This will lead to problems in lans that use wan ranges for the lan, but that would be a mess anyway.

Note that neither this solution nor the old one has support for ipv6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants