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

setting SERVER_NAME does not restrict routing for both subdomain_matching and host_matching #5634

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

davidism
Copy link
Member

I wrote a bunch in #5553 about investigating this. Essentially, we need to pass "" to disable subdomain matching, not None. The two tests that had to change were accidentally working because setting SERVER_NAME was bypassing subdomain_matching = False, and the tests should have been setting it to True since they were working with subdomains.

Now SERVER_NAME only affects routing if subdomain_matching=True, in which case it correctly limits routing to subdomains under that name.

fixes #5553

@davidism davidism added this to the 3.1.0 milestone Nov 11, 2024
@davidism davidism force-pushed the subdomain branch 2 times, most recently from b04dfc4 to cdc3756 Compare November 12, 2024 16:40
@davidism
Copy link
Member Author

While trying to write a test, I discovered that SERVER_NAME interfered with host_matching as well. If host_matching=True, server_name should not be passed to bind_to_environ, otherwise it's always used instead of the request's Host.

@davidism davidism marked this pull request as ready for review November 12, 2024 16:43
@davidism davidism changed the title fix subdomain_matching=False behavior setting SERVER_NAME does not restrict routing for both subdomain_matching and host_matching Nov 12, 2024
@davidism
Copy link
Member Author

I tested a corresponding change in Werkzeug to move subdomain_matching there, and this test still passes with no changes to Flask, and with changes to rely on the modified Werkzeug instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit SERVER_NAME's impact on routing and external URL generation
1 participant