Fix static route when using host_matching = True

[jab]

Change Flask.__init__ to accept two new keyword arguments, host_matching and static_host.

This enables host_matching to be set properly by the time the constructor adds
the static route, and enables the static route to be properly associated with
the required host.

Previously, you could only enable host_matching once your app was already
instantiated (e.g. app.url_map.host_matching = True), but at that point
the constructor would have already added the static route without host matching
and an associated host, leaving the static route in a broken state.

Fixes #1559.

[untitaker]

I think a better fix would be if we delay registration of the static route until the first request. This sort of thing may sound radical, but is already done for a lot of things that depend on configuration changes done past the instantiation of Flask().

[jab]

@untitaker Thanks for taking a look at this and sorry for the delay in responding! I've been swamped with urgent work, but I'd still love to contribute a fix for this issue, and make any required changes to get it landed, ideally before I forget what the problem is again and have to start over.

I think a better fix would be if we delay registration of the static route until the first request.

The user still needs a way to pass a host for the static route though, right? In that case, what is the advantage of delaying registration of the static route until the first request, when we can do it correctly at app instantiation time provided a static_host param?

Even still, I tried delaying registration of the static route until the first request anyway just to see if it would work, but it didn't. Ultimately there is a chicken-and-egg problem. By the time we get the first request and go to add the static route, if the first request was for the static endpoint, we will already get an incorrect 404 for it before we've had a chance to add the static route.

I worked up a new PR for this in #2211 which applies cleanly against latest master. In that version, rather than add the two new constructor parameters host_matching and static_host, where host_matching should be True if static_host is passed, I made it so that there's only the single new static_host parameter, which if passed causes host_matching to be set to True implicitly. Maybe that's preferable.

[untitaker]

Because your fix 1.) introduces another way to do the same thing 2.) will not work for existing code that modifies url_map in arbitrary ways.

I see how my suggestion falls apart, but I definitely think that we can initialize the static URL path before the request gets routed, so static requests should work for the first request as well.

I suppose static_host requires host_matching to be set? In that case we should raise an exception for the case static_host is not None and not host_matching. This is doable in both possible implementations.

[jab]

Thanks for the continued review, @untitaker. I addressed your comments in 7ea1ed6 as part of #2211. Specifically:

• Defer adding the static route until right before the first request.
• No longer set url_map.host_matching to True implicitly if static_host is
  provided. Instead raise an error at the time the static route is added if
  static_host is truthy and host_matching is falsy.

Please take a look.

I'll close this PR since #2211 is the one with the desired changes. Happy to squash and rebase that one as desired.

[jab]

Thanks for reopening, @untitaker. Just pushed 919e5dc. I think that should do the trick to get this all resolved. Please let me know if any further changes are needed.

[untitaker]

LGTM, please add changelog

[jab]

Done in 67d8d0f.

[jab]

This has been approved by @untitaker and @davidism (thanks for reviewing!). Can this be merged?

[untitaker]

There is a ResourceWarning when running the tests on Python 3 that needs to be fixed. I think you need to close your response explicitly to make this error go away.

[jab]

Oh, I thought that was due to an unrelated issue (#513). Added an explicit close() in 5511315 and now the tests are passing for me locally on Python 3, thanks @untitaker! Ready to merge now?

(Note: some targets are now failing on Travis (for REQUIREMENTS=devel and REQUIREMENTS=devel-simplejson), but that was the case for the most recent build of master too (revision ae1ac20), so that seems unrelated to this PR and shouldn't block merging.)

[untitaker]

I now kicked over the broken builds. We can't test against those devel versions if they fail to install.

[untitaker]

Thanks!

[jab]

My pleasure, and thank you for reviewing and merging!

[bmjjr]

Thanks for this fix all. It quickly helped solve the issue of finding a way to easily allow a reverse proxy to multiple domains. But, breaks debug toolbar, related to flask-admin issue at flask-admin/issues/1395