SWProxy
SWProxy copied to clipboard
kakaroto
Allow option to set the listening host
- Use the -o or --host option to set the host to listen on, in cases where the correct IP cannot be determined.
On my machine I have a lot of different ethernet addresses so being able to override it is helpful. I was getting exceptions thrown preventing it form running in some cases.
Thanks for the pull. I would rather do a :
if options.host:
my_ip = options.host
else:
my_ip = get_external_ip
As for the GUI, we could just make the ip field user-editable.
That works, too. In the GUI, it still uses get_external_ip but it causes a thrown exception if it can't find the IP so that GUI can't even start.
Ah ok, I hadn't understood that. In that case, it needs to be fixed but not by forcing a -o option. Can you show me the exception you get and the result of the various stages in get_external_ip so we can track down why it can't find your IP ? I thought your issue was that you had multiple interfaces, each with an external IP (multi-homed system with multiple NICs) and you needed to select which one was on the same router as your phone.
There's actually two issues I'm facing:
- On one machine, the wrong IP is getting selected (it's picking up a virtual NIC that is used by vmware for bridged networks)
- On another machine, I get an exception:
Traceback (most recent call last):
File "SWProxy.py", line 162, in <module>
win = gui.MainWindow(get_external_ip(), options.port)
File "SWProxy.py", line 85, in get_external_ip
ips = [l for l in ([ip for ip in socket.gethostbyname_ex(socket.gethostname())[2] if not ip.startswith("127.")][:1], sockets) if l]
socket.gaierror: [Errno 8] nodename nor servname provided, or not known
Any way this sort of feature can get pushed upstream? I'll update my PR with the code. Not having to maintain a local branch would be nice.
the issues you were facing should be corrected by now. The force option should be useful for some scenarios and its simple enough to maintain/support +1 for merging this (.but your branch have some conflicts ).
Yeah my PR was from a previous version. I pulled the latest code and I still have an issue.
I'll fix it up.
Many people are reporting that sw picks the wrong IP address. I guess this feature can help them
Yeah, but also, WHY is it picking the wrong IP.. it didn't do that before.. I think the change to auto-detect ip in a "reliable" way somehow screwed things up. Being able to manually set it will be great for some people, but most non-tech people will be lost and wouldn't know what to put in there.
during the 0.98 release, some people reported that problem. I opened an issue #48 and checked what have changed since 0.97 and found this change ffa2ff11c760784624d36cd97b1073804e8440cd which seems to be an evolution from the previous code to fix something. What was fixed ?
Instead of reversing the change I decided to move forward and exclude all invalid ip and order the list putting common local ips on top. Lots of people reported that the change fixed their problems then I closed the issue.
Maybe the old code also had some problems (I don't know why it was changed) and we didn't notice that earlier because we had few mac users before.
@kakaroto @lstern checkout the branch/PR I pushed, it enables a --ipdetect2 option, can you guys test? It requires netifaces plugin, but should be cross platform.
Since it seems like a better defined solution (if it works) perhaps we can chuck out all the current IP method detection in favor of using this? let me know what you think
Oh yeah, I remember what the issue is... The original code I had was doing something really simple : connect to 8.8.8.8 using any interface (let the kernel decide how to connect to that IP), then it asks the socket what is the local ip that it connected from, thus giving us the main, primary interface that is internet-enabled and is able to connect to external servers... I had found that code snippet from stackoverflow but couldn't remember the link to it.. then when aztheros wrote the code cleaning patch, he put the link to the stackoverflow article (I think a different one, but using a similar line) and added something else to it which makes it look up the local hostname first. It's based on this https://stackoverflow.com/questions/166506/finding-local-ip-addresses-using-pythons-stdlib/1267524#1267524 and @azrethos replaced my original one liner by what appears to be an update to it by its original author of that one liner :
- my_ip = [[(s.connect(('8.8.8.8', 80)), s.getsockname()[0], s.close()) for s in [socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]][0][1]][0]
+def get_external_ip():
+ # ref: http://stackoverflow.com/a/1267524
+ try:
+ sockets = [[(s.connect(('8.8.8.8', 80)), s.getsockname()[0], s.close()) for s in [socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]][0][1]]
+ ips = [l for l in ([ip for ip in socket.gethostbyname_ex(socket.gethostname())[2] if not ip.startswith("127.")][:1], sockets) if l]
+ return ips[0][0]
+ except KeyError:
+ # fallback on error
+ return socket.gethostbyname(socket.gethostname())
If I run the first line (without selecting the internal value from it), I get this :
> [(s.connect(("8.8.8.8", 53)), s.getsockname()[0], s.close()) for s in [socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]]
[(None, '192.168.2.44', None)]
If I run that second metod, I get this :
> socket.gethostbyname_ex(socket.gethostname())
('localhost.localdomain', ['localhost', 'kakaroto'], ['127.0.0.1'])
And it looks like that gethostname() just gives whatever hostname is set in /etc/hosts (on linux/mac) so the final code basically merges those two lists, then skips the 127.* ip addresses, and in my case, we end up with just the 192.168.x address. In some other people's case, you may end up with whatever other local hostname gives you.. people with some kind of VPN app get something else, etc.. and in general, it all becomes a mess... Even your alternative method of using the netifaces might be more elaborate, but it will just give you a bunch of interfaces, and you still have to take the first one you find pretty much, which is wrong, because you don't know if that first interface is a valid one, a virtual interface, a virtual machine interface, a VPN interface, etc... and you can try to add as many exceptions as you like to avoid 127.x or whatever other IP addresses are considered 'invalid', but the fact remains that it's not a good solution. I think the best solution is still the very first one, the original code that simply used whatever interface is able to connect to 8.8.8.8. Unless someone is multi-homed and has more than one network interface with its own valid routing to the internet (extremely rare), then the first method will work perfectly. So I vote for just reverting to that original one liner and call it a day.
There's no guarantee that old code will select the correct interface. Not sure if code was broke to someone but made a pull request with a hybrid from old and new version but instead of merging the socket.gethostbyname_ex list, it just order and filter the existing interface list.
While writing this code, I found 1 error on current one, code was filtering 192.168.0.0/12 (I did this mistake while trying to fix the code)
Well, filtering 192.168.0.0/12 might be why people were having issues :p Either way, yes, there is actually a guarantee that the old code will select the correct interface, because we create an outgoing socket that connects to 8.8.8.8 (google's DNS server FYI) so whatever IP it gives us, it's on the interface which has external connection routing. So basically, any of the virtual machines, or loopback interfaces would not get used because they wouldn't be able to connect to 8.8.8.8. I saw your pull request #69 but the filtering you do would be useless (and doing it this way is exactly to avoid having to start filtering a hardcoded list of ip addresses) since those ips would not result in a successful outgoing connection.
hey guys; this issue is directly related to simply adding an option to set --host 10.0.2.4 or whatever; I think we should move IP detection alg talk into a relevant issue/PR. Can we just get custom host setter code merged in? seems like a small change, I can submit it in a fresh pull request if you'd like (or attach it to my ip-detect-2 branch)
The code was broken before the 192.168.0.0/12 filter was implemented it didn't break sw proxy for me (my local ip begins with 192.168) and somehow fixed sw proxy for some people(those who motivated me to implement the "fix") but might have broken SW Proxy for some others :p
I agree, i would still like the ability to set a host manually along with the detection fix. I have certain situations where two NICs may be able to access 8.8.8.8 for DNS routing but one NIC doesn't actually have full internet access.
The if host / else detect would be great.
for the record, I do run a "more than one network interface with its own valid routing to the internet" @kakaroto
Oh yes, of course having the ability to set it manually is wanted, that's why this issue isn't closed, but those who understand what an IP address is and know how to set it are a minority of the users. We need to fix the detection for everyone else. And for those who are multi-homed, like @azrethos, those can set the host manually and that's it.
I committed this to the swproxy plugins repo - should be applicable
https://github.com/Nightreaver/SWProxy-plugins/commit/b29ac7283587fad06b5e5dfe05657ba6bbe33334 https://github.com/Nightreaver/SWProxy-plugins/commit/49ef772c74e43f3f13b3602855f4c109b615a3ab
I also think "host" is misleading, as the proxy will always run locally but on different interfaces
@Nightreaver those look good, if you send a pull request with those commits, I can get it merged. thanks