Url conflict assert

[rawrgulmuffins]

This PR is an attempt to reconcile the fact that you can't have two routes with the same function name but you can create two routes with the same URL.

from flask import Flask
app = Flask("the_flask_module")

@app.route("/hello")
def hello_page():
    return "I'm a hello page"

@app.route("/different_route")
def hello_page():
    return "Different route"

will throw an AssertionError

but

@app.route("/hello")
def end_point1():
    return "I'm a hello page"

@app.route("/hello")
def end_point2():
    return "Different route"

wont. In fact the second route is unreachable.

This PR checks to see if three conditions are true before raising an assertion. The new rule has to have a subdomain, url_path, and one method that overlap with a current rule in the self.url_map object.

There's a couple of wonky pieces in this pull request that were required to get all of the unit tests to pass.

if prior_rule.rule == "/static/<path:filename>":
    continue

was required to get blueprints to not throw an assertion since each blueprint registers a "/static/<path:filename>" rule for each blueprint. I tried to find a way to not have to use a static string but alas I failed.

Secondly

 methods_minus_required = ((rule.methods & prior_rule.methods) 
     - required_methods)

is used to check for all explicitly registered methods for a route since there were a bunch of cases where the OPTIONS method was implicitly added to rules that shared a url_path.

---

This change is

[ThiefMaster]

I think all these commits could be easily squashed into a single one.

[rawrgulmuffins]

Done, everything was squashed down to that one commit.

[ThiefMaster]

I think you have some leftovers from failed merges with upstream in there. Check the diff and remove anything that's not actually related to your PR (and maybe try to switch to a <=51-char first line in the commit message when committing it again)

[rawrgulmuffins]

I rebased against master and re-added my changes. Looks like I missed a couple of wonky space changes, in app.py. I also changed to a hashing check rather than a iteration check.

[rawrgulmuffins]

The last commit was to add some in-line comments that got dropped when I re-added the changes.

[untitaker]

In principle I support a mechanism that detects and prevents shadowing of other URL rules, but this implementation isn't acceptable due to a few reasons: - The static path is hardcoded for some special casing - `prior_rules` will only be updated if the app registers its routes the "normal" way. It is possible for those two datastructures to get out of sync when the app dev modifies them directly, which is still practiced sometimes. - Unrelated code reformatting, not a minimal diff

…

On Mon, Apr 24, 2017 at 10:25:06AM -0700, David Lord wrote: @davidism requested your review on: pallets/flask#1568 Url conflict assert. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #1568 (comment)

[untitaker]

I will just close this since it's also unclear to me how a better implementation could look like.

[ThiefMaster]

Probably a "better implementation" should be in Werkzeug, not in Flask. i.e. fail when adding a conflicting rule to a url map