Flask: Prevent setting `SERVER_NAME` and `SESSION_COOKIE_DOMAIN` to an IP address

[untitaker]

See #1946 and #1906. I think we should prevent setting those things to IP addresses in the first place and only allow None and domain names != localhost.

[untitaker]

From #1955: We should also check that no port is contained.

[davidism]

Given that SERVER_NAME is used with url_for(..., _external=True) when not in a request context, preventing ports would break links. This comes up, for example, when using a Celery task to send an email containing a link. Maybe there's a more correct way to do external links outside request contexts, but if there is I couldn't find it at the time.

[untitaker]

Given that SERVER_NAME is used with url_for(..., _external=True) when not in a request context, preventing ports would break links.

It rather seems to me that using ports there only accidentally worked and was never expected by the API authors. SERVER_NAME is passed in Flask, yes. However, I don't think Werkzeug's Map.bind expects a port either, since 1.) the parameter is also called server_name 2.) server_name is then idna-encoded, which is nonsensical for host + port combinations and actually breaks for certain hostnames:

>>> 'foobar:443'.encode('idna')
'foobar:443'
>>> 'fööbar:443'.encode('idna')
b'xn--fbar:443-n4aa'

However, apparently app.run derives its default port from SERVER_NAME.

This is so broken...

[davidism]

So basically, if this were working correctly, there would be no way to generate a url for a non-standard port outside of a request context, and it already doesn't work in some cases.

This and the propagate exceptions issue are both cases of one option doing more than it should. We should probably do a more thorough review of what each option does and maybe add some more to disambiguate stuff before 1.0.

[untitaker]

Something like:

• Prevent ports in SERVER_NAME
• Add SERVER_PORT
• Change app.run behavior and url_for implementation accordingly.

[amneher]

would you guys mind if I had a go at this one? I'd like to help and also squash my first bug. Thanks!

[untitaker]

Ah, this isn't actually that beginner friendly since we still need to decide what to actually do.

[untitaker]

I do think that adding SERVER_PORT would be correct, but somebody might disagree!

[mferreira96]

I didn´t fully undestand this bug. Please correct me if I'm wrong. I think that one solution for this problem is check if the variables SERVER_NAME and SESSION_COOKIE_DOMAIN are an IP address and if so what should I do? Abort the program? Give a warning message? And do you want me to check if it is IPV4 or IPV6 or only IPV4?

[untitaker]

@mferreira96 IP addresses don't work when used in cookies, only domains do, this is why setting SESSION_COOKIE_DOMAIN to an IP is nonsensical. I can't remember why I wanted to prevent setting SERVER_NAME to an IP though (IPs should work for url_for).

Warning message sounds actually more reasonable. Both v4 and v6.

[mferreira96]

i am having some difficults in find where i should call the function that i build to give a warning when the person use an IP address in SESSION_COOKIE_DOMAIN, can you help me?

[HeyManLean]

I set the SESSION_COOKIR_DOMAIN as "mysite.com", but browser domain remain be ".mysite.com" and my subdomains can see my main site cookie.