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

Redirect bugfix #747

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Redirect bugfix #747

wants to merge 1 commit into from

Conversation

polymorphm
Copy link
Contributor

tiny but important fix:

diff --git a/bottle.py b/bottle.py
index 535ddd3..359ceaf 100644
--- a/bottle.py
+++ b/bottle.py
@@ -2418,7 +2418,7 @@ def redirect(url, code=None):
     res = response.copy(cls=HTTPResponse)
     res.status = code
     res.body = ""
-    res.set_header('Location', urljoin(request.url, url))
+    res.set_header('Location', url)
     raise res

@avelino
Copy link
Member

avelino commented Apr 2, 2015

Ideally meet the two use cases! Needs test writes test to pass by the two conditions.

@polymorphm
Copy link
Contributor Author

I do not think that tests may be writed.

because problem displayed -- when we using complex Front-End (example as Mozilla Firefox.. but NOT curl) or complex system of back-end wrappers.

see next :)

easy example

see HTTP header Alt-Svc: ...

if back-end will use this HTTP header:

bottle.response.set_header('Alt-Svc', 'h2=":443"')

that Mozilla Firefox (v37++) will use 443-port (and https:// protocol) , instead of 80-port (and instead of http:// protocol)

BUT on front-end side (in User Interface of Mozilla Firefox, AND for Javascript-Files, and for CSS-files, and for GUI URL Field) -- we will see http:// protocol (not https:// protocol).

this easy example shows us -- that back-end and front-end -- may has different information about URL.

BUT when back-end will do redirect('other_page.html') -- it must NOT to force using backend information.

@defnull
Copy link
Member

defnull commented Apr 2, 2015

Uhm... http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30

"The Location response-header field is used to redirect the recipient to a location other than the Request-URI for completion of the request or identification of a new resource. For 201 (Created) responses, the Location is that of the new resource which was created by the request. For 3xx responses, the location SHOULD indicate the server's preferred URI for automatic redirection to the resource. The field value consists of a single absolute URI."

@polymorphm
Copy link
Contributor Author

@defnull , OK ! if relative-URL-path (in redirect) is conflicted with RFC.. I think will be fairly (rightly) to reject this pull-request #747 (and same issue:#748).

I sure that in real life (in modern time) -- relative-URL may be used (and it used! yes) in http-redirects. because relative-URL in http-redirects purposely not prone to problems .

but conflicting with RFC -- it strong reason too.

I will respect any your decision about accept-or-reject relative http-redirects in Bottle!

@polymorphm
Copy link
Contributor Author

and what about adding new keyword-argument in redirect(...) ?

example:

def redirect(url, code=None, use_relative=None):
    if use_relative is None:
        use_relative = False

    # ...
    # ...
    #...

IMHO, that's will be great! :)

@polymorphm
Copy link
Contributor Author

@defnull , very very sorry for my many comments :( .. but yet comment.

http://tools.ietf.org/html/rfc7231#section-7.1.2

*** begin of quote ***

The "Location" header field is used in some responses to refer to a
specific resource in relation to the response. The type of
relationship is defined by the combination of request method and
status code semantics.

 Location = URI-reference

The field value consists of a single URI-reference. When it has the
form of a relative reference
([RFC3986], Section 4.2), the final
value is computed by resolving it against the effective request URI
([RFC3986], Section 5).

*** end of quote ***

@defnull
Copy link
Member

defnull commented Apr 2, 2015

Oh, okay. RFC7231 obsoletes RFC2616 and is more recent. In that case the change should really be discussed (perhaps in a new issue). I'll open one, just a second.

@ThinkChaos
Copy link

Please merge this, it's still causing pain 😢

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

Successfully merging this pull request may close these issues.

4 participants