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

upnp functionality is used despite being config'd as False #24

Closed
Mutnick opened this issue Aug 28, 2016 · 6 comments
Closed

upnp functionality is used despite being config'd as False #24

Mutnick opened this issue Aug 28, 2016 · 6 comments
Labels

Comments

@Mutnick
Copy link
Member

Mutnick commented Aug 28, 2016

In frame.py's OnFirstConnect function there are initializations and checks of upnp functionality even when upnp is configured as False (and such functionality is not desired or needed)

Please relocate

if self.np.config.sections["server"]["upnp"]:

to the very start of OnFirstConnect()

This is an issue as the reworked code broke the ability of the user to Torify Nicotine+.

@ghost
Copy link

ghost commented Aug 28, 2016

Yes I can move this line at the start of the function that's not a problem.
In this case we might have a look too at settingswindow.py at the end of the SetSettings function of the ServerFrame class.
What I'm trying to understand is what would break a Torify Nicotine+ ?
The line you're referring to 'upnp.IsPossible()' only checks if either the upnpc binary or python miniupnpc module is available and don't send any packet on the network as far as I know.

@ghost
Copy link

ghost commented Aug 28, 2016

I've relocate the line at the top of the function OnFirstConnect() in frame.py.

I've also found another line to relocate in the upnp.py init function:

# Local LAN IP
self.internalipaddress = gethostbyname(gethostname())

This line was doing a NS lookup and i've pushed it back down to the function actually doing the port mapping so it's not called at initialization/check times. See https://github.com/gfarmerfr/nicotine-plus/commit/710621bc69ffb9179123bd94d0f5675996d0b208 in https://github.com/gfarmerfr/nicotine-plus/tree/avoid-unwanted-traffic-upnp

@Mutnick
Copy link
Member Author

Mutnick commented Aug 28, 2016

The error occurs when resolving the host name.

upnp = UPnPPortMapping() leading to the gethostbyname(gethostname()) is where the error occurs.

On windows, gaierror: [Errno 11003] getaddrinfo failed
On a Debian based machine, gaierror: [Errno -2] Name or service not known

I'll look into this further but the point remains that if someone doesn't want the upnp functionality for whatever reason, there is no need to be calling it at all.

@ghost
Copy link

ghost commented Aug 28, 2016

Just posted a few seconds ago :)
Could you check my last post and tells me what do you think of the solution ?

@Mutnick
Copy link
Member Author

Mutnick commented Aug 28, 2016

Thank you, looks good.

@Mutnick Mutnick closed this as completed Aug 28, 2016
@ghost
Copy link

ghost commented Aug 28, 2016

Merged into master.

@ghost ghost added the bug label Aug 28, 2016
toofar pushed a commit to toofar/nicotine-plus that referenced this issue Apr 24, 2020
* Fix flake8 warnings

* Mark remaining flake8 warnings with noqa comment

* Update pipelines

Co-authored-by: droserasprout <droserasprout@tuta.io>
Co-authored-by: Lev Gorodetskiy <gorodetskiy@suicide.ventures>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

1 participant