Automatic redirect with "/" breaks routes with different methods #1783

JelteF · 2016-04-16T11:59:16Z

I have found an issue that is caused by the automatic redirect of routes to the version with a "/" at the end. See the code below for a minimal example:

from flask import Flask
app = Flask(__name__)


@app.route("/a/", methods=["GET"])
def hello():
    return "Hello GET!\n"


@app.route("/a", methods=["POST"])
def hello_post():
    return "Hello POST!\n"

if __name__ == "__main__":
    app.run()

The POST route is unreachable as the "/" redirect is done without looking at the method. See the following curl commands (the -L flag is to follow redirects):

$ curl localhost:5000/a/
Hello GET!

$ curl localhost:5000/a
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>Redirecting...</title>
<h1>Redirecting...</h1>
<p>You should be redirected automatically to target URL: <a href="http://localhost:5000/a/">http://localhost:5000/a/</a>.  If not click the link.

$ curl localhost:5000/a -L
Hello GET!

$ curl localhost:5000/a -X POST
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>Redirecting...</title>
<h1>Redirecting...</h1>
<p>You should be redirected automatically to target URL: <a href="http://localhost:5000/a/">http://localhost:5000/a/</a>.  If not click the link.

$ curl localhost:5000/a/ -X POST
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>405 Method Not Allowed</title>
<h1>Method Not Allowed</h1>
<p>The method is not allowed for the requested URL.</p>

$ curl localhost:5000/a -X POST -L
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>405 Method Not Allowed</title>
<h1>Method Not Allowed</h1>
<p>The method is not allowed for the requested URL.</p>

Edit: My python version is Python3.4 and Werkzeug is 0.11.8 but it was also possible with the git version of Werkzeug.

The text was updated successfully, but these errors were encountered:

JelteF · 2016-04-16T12:38:26Z

Switching the order of the two routes fixes the issue btw:

from flask import Flask
app = Flask(__name__)


@app.route("/a", methods=["POST"])
def hello_post():
    return "Hello POST!\n"

@app.route("/a/", methods=["GET"])
def hello():
    return "Hello GET!\n"

if __name__ == "__main__":
    app.run()

That way POST does get matched:

$ curl localhost:5000/a -L
Hello GET!
$ curl localhost:5000/a -X POST -L
Hello POST!

ThiefMaster · 2016-04-16T12:40:17Z

Mixing /a/ and /a seems like a really bad idea to me. Try app.route(.., strict_slashes=False) to disable the automated redirect from /a to /a/ if you really need it though. You can also do this globally using app.url_map.strict_slashes = False

If that solves it for you I'd say it's not a bug.

JelteF · 2016-04-16T12:50:10Z

I agree that it is not very nice. But I found this bug during a refactor that had the side of importing the routes in a different order and suddenly different parts of my application were broken because there were some GET routes that redirected my PUT routes. It seems bad that routes are redirected when the resulting route will result in a 405 error, especially when the first route was in the routing table for that method.

Disabling strict slashes for the GET route in the example works strict_slashes, but this is not really what I want since like strict slashes and have them enabled for a reason.

N0odlez · 2016-04-16T17:22:15Z

Hello @JelteF
Really, your method of doing this is bad practice. I'll show you a method that will solve this issue for you.

from flask import Flask, request

app = Flask(__name__)


@app.route("/hello", methods=["GET", "POST"])
def hello():
    if request.method == "POST":
        return "Hello POST!"
    return "Hello GET!"


if __name__ == '__main__':
    app.run()

I always put both get and post in to one method. It's best practice and it keeps your code really clean and easy to read.

JelteF · 2016-04-16T17:46:31Z

@AzureusNation When using REST like applications I think it is a very weird practice to do it like this. Since the GET, PUT and POST methods probably do entirely different things, but putting them in the same function. It is also shown in examples that you can simple have functions with the same route but with different methods:
http://flask.pocoo.org/snippets/1/

I've also used this method for more than two years now and have never had any problems with it except the one I'm showing here.

ThiefMaster · 2016-04-16T17:48:56Z

You could use class-based views or an extension like flask-restful

N0odlez · 2016-04-16T17:54:10Z

@JelteF Here's an idea so not all the code is in the same function. You could have your functions in a separate py file and call the functions like this.

@app.route("/hello", methods=["GET", "POST"])
def hello():
    if request.method == "POST":
        return doPost()
    return doGet()

But personally I don't see the need of having 3 functions when you can compress it down in to one.

ThiefMaster · 2016-04-16T17:56:25Z

Pretty ugly IMO (not to mention that camelCase doesn't belong into python code).

jeffwidman · 2016-04-17T20:26:17Z

It doesn't look like there's a bug here since the behavior is configurable. Having the strict slashes redirect default to on seems fine to me--I prefer that we have a redirect one way or the other as I think it leads to cleaner URLs by default. If a user for some reason wants to mix the route styles, it's not hard to google how to turn off that redirect.

Arguably, the redirect could kick in after the routes table is parsed, in which case the new POST would work, but if you'd previously had a route that was working due to a redirect, adding this new route would (somewhat unexpectedly) break the original route. For now, I'm closing this, if there's significant interest in changing the evaluation order so the redirect happens after the routes table, then we can re-open and re-evaluate.

JelteF · 2016-04-17T20:51:50Z

The redirect itself is not the problem, since that is exactly what I want. The problem is that it redirects even though the method is invalid after the redirect, especially because the exact route is valid for that method. If a GET route would be defined with and without a slash I would argue that this behaviour is not a bug. But in this case a redirect is done to a route that will definitely return an error in favour of not redirecting and having a route that matches directly.

untitaker · 2016-04-17T21:12:26Z

FTR changing the evaluation order to anything "complexity-based" seems like a no-go to me since the resulting behavior would be harder to reason about. You're going to end up with edgecases like this one either way: I think the current way has fewer ones. At least you can just change the order of registration to work around it, in systems where the routes are resorted you can't.

jeffwidman · 2016-04-18T20:40:52Z

Having thought about this more, I agree the behavior is definitely surprising and does feel like a bug. If a route doesn't define a HTTP method, then another route that handles that method shouldn't be overridden, regardless of auto-redirects.

I'm not sure how to best solve it. Probably the redirect should only kick in for the defined HTTP methods on a route. That's the behavior I would expect.

untitaker · 2016-04-18T20:44:23Z

Sorry, yes, that is a bug. I didn't even sense this from the original post, probably was half-sleeping.

While I think this can maybe be solved in a backwards-compatible manner, I don't think it qualifies for a bugfix release, as it might break applications.

JelteF · 2016-04-19T18:16:13Z

Glad to hear you guys agree. After looking into to Flask internals a bit I think this is actually a Werkzeug bug instead of a Flask bug.

JelteF · 2016-04-19T18:51:12Z

I've just submitted a very simple pull request in the werkzeug repo to fix this bug. pallets/werkzeug#907

jeffwidman · 2016-04-19T19:23:49Z

Closing, this is now tracked over in pallets/werkzeug#907

Thanks again @JelteF for working on this.

untitaker · 2016-04-19T19:29:33Z

no please keep this issue open, PRs are different from issues.

jeffwidman · 2016-04-19T20:34:57Z

No problem, but I thought the switch to fixing this in Werkzeug means it's not an issue with Flask but with upstream? Do we keep Flask issues open if the underlying problem is in a dependancy?

untitaker · 2016-04-19T20:39:30Z

It would be better refiled against Werkzeug but more importantly, some issue should stay open.

mitsuhiko · 2016-05-22T10:20:31Z

I would close this and file it again against Werkzeug. This actually is also related to this issue: pallets/werkzeug#185

(As the flipping of the order of the decorator should not have made a difference)

jeffwidman closed this as completed Apr 17, 2016

jeffwidman reopened this Apr 18, 2016

JelteF mentioned this issue Apr 19, 2016

Don't redirect to slash route when method does not match pallets/werkzeug#907

Merged

jeffwidman closed this as completed Apr 19, 2016

untitaker reopened this Apr 19, 2016

untitaker assigned mitsuhiko Apr 19, 2016

mitsuhiko closed this as completed May 22, 2016

jeffwidman mentioned this issue Jun 3, 2016

OPTIONS requests sometimes match the wrong url_rule if there are multiple route handlers for different methods #1604

Closed

Parakleta mentioned this issue Feb 17, 2017

With strict_slashes disabled, leaf paths mask branch paths. pallets/werkzeug#1074

Closed

d-Rickyy-b mentioned this issue Jan 6, 2019

Mixed content violation with certain WEBROOT_REDIRECT setting Mailu/Mailu#801

Closed

github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic redirect with "/" breaks routes with different methods #1783

Automatic redirect with "/" breaks routes with different methods #1783

JelteF commented Apr 16, 2016 •

edited

Loading

JelteF commented Apr 16, 2016

ThiefMaster commented Apr 16, 2016 •

edited

Loading

JelteF commented Apr 16, 2016

N0odlez commented Apr 16, 2016 •

edited

Loading

JelteF commented Apr 16, 2016

ThiefMaster commented Apr 16, 2016

N0odlez commented Apr 16, 2016 •

edited

Loading

ThiefMaster commented Apr 16, 2016

jeffwidman commented Apr 17, 2016 •

edited

Loading

JelteF commented Apr 17, 2016

untitaker commented Apr 17, 2016 •

edited

Loading

jeffwidman commented Apr 18, 2016

untitaker commented Apr 18, 2016 •

edited

Loading

JelteF commented Apr 19, 2016

JelteF commented Apr 19, 2016

jeffwidman commented Apr 19, 2016

untitaker commented Apr 19, 2016

jeffwidman commented Apr 19, 2016

untitaker commented Apr 19, 2016

mitsuhiko commented May 22, 2016

Automatic redirect with "/" breaks routes with different methods #1783

Automatic redirect with "/" breaks routes with different methods #1783

Comments

JelteF commented Apr 16, 2016 • edited Loading

JelteF commented Apr 16, 2016

ThiefMaster commented Apr 16, 2016 • edited Loading

JelteF commented Apr 16, 2016

N0odlez commented Apr 16, 2016 • edited Loading

JelteF commented Apr 16, 2016

ThiefMaster commented Apr 16, 2016

N0odlez commented Apr 16, 2016 • edited Loading

ThiefMaster commented Apr 16, 2016

jeffwidman commented Apr 17, 2016 • edited Loading

JelteF commented Apr 17, 2016

untitaker commented Apr 17, 2016 • edited Loading

jeffwidman commented Apr 18, 2016

untitaker commented Apr 18, 2016 • edited Loading

JelteF commented Apr 19, 2016

JelteF commented Apr 19, 2016

jeffwidman commented Apr 19, 2016

untitaker commented Apr 19, 2016

jeffwidman commented Apr 19, 2016

untitaker commented Apr 19, 2016

mitsuhiko commented May 22, 2016

JelteF commented Apr 16, 2016 •

edited

Loading

ThiefMaster commented Apr 16, 2016 •

edited

Loading

N0odlez commented Apr 16, 2016 •

edited

Loading

N0odlez commented Apr 16, 2016 •

edited

Loading

jeffwidman commented Apr 17, 2016 •

edited

Loading

untitaker commented Apr 17, 2016 •

edited

Loading

untitaker commented Apr 18, 2016 •

edited

Loading