Fix regression that broke IP binding to 0.0.0.0

[mattwlowe]

Commit https://github.com/junalmeida/Sick-Beard/commit/371d37928469e3615a12c84ce0258106bdf55a17 changed the default webhost to 127.0.0.1 for better Windows compatibility. The result of that commit is below: https://github.com/junalmeida/Sick-Beard/blob/5d2b0ef3a9efb809e77831052abb168d06be7ab4/SickBeard.py#L355-L361

The above logic prevents setting the webhost to 0.0.0.0 when running in IPv4 mode. It also gives an inconsistent default webhost depending on whether IPv6 is enabled or not. This commit fixes the regression, and also makes the behavior consistent between IPv4 and IPv6.

[VeNoMouS]

@mattwlowe just fyi, pretty sure from memory if you try bind 0.0.0.0 under wintendo on ipv4 it craps the bed, and that's why i had put that if in there... bare in mind these PR's were pending for about a year, i also added flags for start up as well, if you wanted to bind to a specific ip you can use -a flag, you can also tell it what ip to bind in the config menu, but over all i have no issue with your PR... I assume you have tested on linux and wintendo to make sure about the binding issues with 0.0.0.0?

I mean your if statement only takes into account if ipv6 is enabled and changes the bind to :: which is basicly 0.0.0.0 but at the same time isnt... what if ipv6 is disabled, problem then comes ipv4 trying to bind 0.0.0.0 if someone specifies it.

[mattwlowe]

@VeNoMouS My commit doesn't really change the current behavior, the default is still set to 127.0.0.1. This commit only addresses the edge case where the webhost is specifically set to 0.0.0.0 in the config but is overridden by the default 127.0.0.1 instead. This effectively breaks IPv4 systems that are accessed remotely, since the binding is forced to be the loopback adapter, even if 0.0.0.0 is specifically provided as the webhost in the config.

I have tested on Linux, but not Windows. Since this commit only changes this edge case behavior, which is obviously a bug anyway, I'm not sure that would be necessary.

Regarding binding 0.0.0.0 on Windows, that should work too. That functionality is pretty much a requirement to even boot Windows these days. I would suspect something else is afoot.

[VeNoMouS]

@mattwlowe , well except ..

    if sickbeard.WEB_HOST:
        webhost = sickbeard.WEB_HOST
    else:
        webhost = '127.0.0.1'

If some one puts 0.0.0.0 and cherrypy still has an issue binding to 0.0.0.0 under windows.. then your going to cause an error ..

[VeNoMouS]

Ah it would seem cherrypy might have finally fixed the issue... they use to have this function in their code

def client_host(server_host):
--
  | """Return the host on which a client can connect to the given listener."""
  | if server_host == '0.0.0.0':
  | # 0.0.0.0 is INADDR_ANY, which should answer on localhost.
  | return '127.0.0.1'
  | if server_host in ('::', '::0', '::0.0.0.0'):
  | # :: is IN6ADDR_ANY, which should answer on localhost.
  | # ::0 and ::0.0.0.0 are non-canonical but common ways to write IN6ADDR_ANY.
  | return '::1'
  | return server_host

but seem to have been removed now... it was under cherrypy/process/servers.py

[VeNoMouS]

Ah, the cherrypy thats currently in SB is really really really old... I will have to upgrade it

[mattwlowe]

@VeNoMouS I would suspect the problem is unrelated to cherrypy. Binding to 0.0.0.0 is extremely common. If that didn't work on Windows there would tons of projects with issues.

The function you posted is only for determining a valid address to connect to the socket over, it is unrelated to the actual step where the socket is bound to an address.

I think this commit is still the best solution. If people want to bind 0.0.0.0 (meaning any IP address on the system) then they should be able to do that. As it stands right now, in IPv4 mode that's impossible. Any installation running in a VM or on a remote server is likely to be broken by https://github.com/junalmeida/Sick-Beard/commit/371d37928469e3615a12c84ce0258106bdf55a17 . That has to be a much larger issue, right?

Allowing users to bind to 0.0.0.0 is the proper way to do this. We shouldn't workaround bugs elsewhere in the code by eliminating this core functionality of IP sockets, instead we should just fix the actual bug.

[VeNoMouS]

its a documented ticket with cherrypy :p about cant bind with 0.0.0.0 :P

As it stands right now, in IPv4 mode that's impossible. Any installation running in a VM or on a remote server is likely to be broken by 371d379 . That has to be a much larger issue, right?

I think you mean any system running ipv6 trying to bind to all interfaces...

also as i said, in the i have tested this in the past and it broke.,.. run the tests and see if it still is broken on windows.

[mattwlowe]

Can you link? I can't find it

[mattwlowe]

I think you mean any system running ipv6 trying to bind to all interfaces...

No, any system running IPv4 trying to bind all interfaces...

Evaluate the current code if sickbeard.WEB_HOST is '0.0.0.0' and sickbeard.WEB_IPV6 is False.

if sickbeard.WEB_HOST and sickbeard.WEB_HOST != '0.0.0.0':
     webhost = sickbeard.WEB_HOST
else:
     if sickbeard.WEB_IPV6:
         webhost = '::'
     else:
         webhost = '127.0.0.1'

also as i said, in the i have tested this in the past and it broke.,.. run the tests and see if it still is broken on windows.

Fair enough, but clearly we should be fixing it for both platforms not just one.

[VeNoMouS]

@mattwlowe after looking at this , appears you can bind on wintendo fine now..

c:\temp\Sick-Beard>python
Python 2.7.13 (v2.7.13:a06454b1afa1, Dec 17 2016, 20:53:40) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from lib import cherrypy
>>> cherrypy.server.socket_port=7777
>>> cherrypy.server.socket_host='0.0.0.0'
>>> cherrypy.server.start()
[09/Apr/2018:11:28:28] ENGINE Serving on http://0.0.0.0:7777
[09/Apr/2018:11:28:28] ENGINE Serving on http://0.0.0.0:7777
>>>

[junalmeida]

@mattwlowe If you still want this PR to be merged, please recreate a pr to the new branches