cherrypy icon indicating copy to clipboard operation
cherrypy copied to clipboard

Allow relative URLs in HTTPRedirect

Open ghost opened this issue 10 years ago • 7 comments

Originally reported by: devsnd (Bitbucket: devsnd, GitHub: devsnd)


CherryPy assumes that the path put into the HTTPRedirect should be returned to the browser as absolute URL. This was the correct behavior until RFC 7231, but now it states relative URLs should work as well.

This would make it easier to run CherryPy powered applications behind a proxy without using 'tools.proxy'.

See:

http://stackoverflow.com/a/25643550/1191373

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

BTW: There was a similar issue before, but at that time the RFC didn't state anything about relative URL.


  • Bitbucket: https://bitbucket.org/cherrypy/cherrypy/issue/1355

ghost avatar Jan 26 '15 21:01 ghost

@jaraco @webknjaz Is there any objection against implementing this? By implementing I mean changing the redirect code from:

        abs_urls = []
        for url in urls:
            url = tonative(url, encoding or self.encoding)

            # Note that urljoin will "do the right thing" whether url is:
            #  1. a complete URL with host (e.g. "http://www.example.com/test")
            #  2. a URL relative to root (e.g. "/dummy")
            #  3. a URL relative to the current path
            # Note that any query string in cherrypy.request is discarded.
            url = _urljoin(cherrypy.url(), url)
            abs_urls.append(url)
        self.urls = abs_urls

to

        self.urls = [tonative(url, encoding or self.encoding) for url in urls]

Safihre avatar Jan 05 '17 15:01 Safihre

There are almost certainly users relying on this existing behavior, so it probably demands a backward-incompatible release. @Safihre, I'd be happy to accept a PR with the above change if it also included:

  • [ ] A review of the docs to be sure that there aren't latent references in the docs contradicting this change.
  • [ ] A tool or compatibility hook or recipe that a CherryPy user could use to restore the old behavior.
  • [ ] A friendly message in the changelog describing the backward incompatibility, the implications, and what a site upgrading should do.

jaraco avatar Jan 05 '17 15:01 jaraco

I would add that such API could be also implemented as a separate exception, like raise HTTPURIRedirect('/uri/not/url/here'), which would be much more explicit, than hidden logic.

webknjaz avatar Jan 05 '17 21:01 webknjaz

@jaraco

  • The docs don't mention anything about HTTPRedirect existing besides a mention in the streaming-section, I think this should be added(?). What I get from stackoverflow questions there used to be docs about it, but not anymore.
  • Hook/recipe, would an extract argument (e.g. abs_path=False) suffice?

Safihre avatar Jan 06 '17 08:01 Safihre

Yeah, lots of docs got lost at some point after 3.2. Regarding the argument naming I'd use something which does correspond its purpose better. What is the absolute path? I'd say it's not URI, but URL. Also take into account that you can explicitly pass base= argument, so it would not be http://localhost:<port>

webknjaz avatar Jan 06 '17 09:01 webknjaz

So to be on the safe side, what are the use cases we should be aware of? Say a user says one of these things:

HTTPRedirect('/login/now')
HTTPRedirect('login/now')

Why would he expect different behavior than being redirect with the relative URL? Even behind a proxy or changed base?

Safihre avatar Jan 10 '17 11:01 Safihre

So how should I redirect a user? When doing raise cherrypy.HTTPRedirect("/") with cherrypy behind nginx, the user is redirected to HTTP on a different port, which is obviously unworkable.

The following solution works but is rather ugly:

def redir2URI(uri):
    cherrypy.response.headers['Location'] = uri
    cherrypy.response.status = '303 See Other'
    return '<a href="' + html.escape(uri) + '">Click here if your browser does not redirect you.</a>'

class yourproject(object):
    @cherrypy.expose
    def something(self):
        return redir2URI('/')

luc-x41 avatar Mar 06 '19 10:03 luc-x41