Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #316 (closed defect: fixed)

Opened 13 years ago

Last modified 11 years ago

[PATCH] Modify turbogears.url so that it can return an absolute path

Reported by: michele Owned by: Chris Arndt
Priority: low Milestone: 1.1b2
Component: TurboGears Version: 1.0.4.4
Severity: minor Keywords: needs tests, needs review
Cc:

Description

Link to the  discussion where Elvelind declared he had a possible patch. :P

Attachments

full_url_patch.diff Download (3.0 KB) - added by singletoned 12 years ago.
A patch to add full url capabilities into the url() function

Change History

comment:1 Changed 13 years ago by anonymous

  • Milestone set to 0.9

comment:2 Changed 13 years ago by kevin

  • Status changed from new to closed
  • Resolution set to wontfix

The problem with trying to build out a full URL (rather than just an absolute path on the same server) is that there could be problems if you're running behind a proxy (like Apache) and the server name shows up as localhost. There's really no need for a full URL to come from this function anyhow. An absolute URL or relative URL will get you where you need to be within your app.

comment:3 Changed 12 years ago by singletoned

  • Status changed from closed to reopened
  • Resolution wontfix deleted
  • Milestone changed from 0.9 to 1.0.2

Can this ticket be reopened? There are lots of valid reasons for wanting a full url, such as when sending out emails, creating RSS feeds, etc.

Changed 12 years ago by singletoned

A patch to add full url capabilities into the url() function

comment:4 Changed 12 years ago by singletoned

  • Summary changed from Modify turbogears.url so that it can return an absolute path to [patch] Modify turbogears.url so that it can return an absolute path

comment:5 Changed 12 years ago by alberto

Hmmm, patch breaks 6 tests. Can you please submit a modified version that works?

Thanks,

Alberto

comment:6 Changed 12 years ago by alberto

Where did I get that "6 tests" figure?... anyaway...

Ran 247 tests in 13.683s

FAILED (failures=1, errors=8)

comment:7 Changed 12 years ago by alberto

  • Summary changed from [patch] Modify turbogears.url so that it can return an absolute path to Modify turbogears.url so that it can return an absolute path

comment:8 Changed 12 years ago by alberto

  • Milestone changed from 1.0.2 to 1.0.3

comment:9 Changed 12 years ago by faide

  • Summary changed from Modify turbogears.url so that it can return an absolute path to [PATCH] Modify turbogears.url so that it can return an absolute path

comment:10 Changed 12 years ago by faide

  • Milestone changed from 1.0.3 to 1.1

comment:11 Changed 11 years ago by faide

  • Version set to 1.0.4.4

comment:12 Changed 11 years ago by Chris Arndt

Wow, this ticket is waaay old! It don't like the proposed patch because it changes the function signature by adding an extra keyword argument, which might (though unlikely) break existing code and also, it doesn't really work - explanation follows:

Unless you use HTTPS (more below), an absolute URL can easily be constructed with the following function:

from turbogears import config, url

def absolute_url(tgpath='/', params=None, **kw):
    """Returns absolute URL (including schema and host to this server)."""

    h = cherrypy.request.headers
    use xfh = config.get('base_url_filter.use_x_forwarded_host', False)
    base_url = 'http://%s' % h.get('X-Forwarded-Host', h['Host'])
    if config.get('base_url_filter.on', False) and not use_xfh:
        base_url = config.get('base_url_filter.base_url').rstrip('/')
    return 'http://%s%s' % (base_url, url(tgpath, params, **kw))

The problem with HTTPS is that there is no way to detect that the client is talking HTTPS when your app is running behind a reverse proxy (see also  this wiki document).

Two possible solutions:

  1. You can configure your web server to add a special header if the connection between the client and the webserver is over HTTPS and then use this information in absolute_url function:
def absolute_url(tgpath='/', params=None, **kw):
    """Returns absolute URL (including schema and host to this server)."""

    h = cherrypy.request.headers
    use xfh = config.get('base_url_filter.use_x_forwarded_host', False)
    if cherrypy.request.headers.get('X-Use-SSL'):
        scheme = 'https'
    else:
        scheme = 'http'
    base_url = '%s://%s' % (scheme, h.get('X-Forwarded-Host', h['Host']))
    if config.get('base_url_filter.on', False) and not use_xfh:
        base_url = config.get('base_url_filter.base_url').rstrip('/')
    return 'http://%s%s' % (base_url, url(tgpath, params, **kw))
  1. You can just set base_url_filter.on = True, base_url_filter.use_x_forwarded_host = False and base_url_filter.base_url to the base URL of your site including the HTTPS scheme, e.g. https://example.com.

To sum it up, there is no automatic way for TG to construct an absolute URL that works in all cases, without the help from either web server or application configuration. I suggest that we add the above function as a new feature to the 1.1 branch. Maybe it should then also determine the URL scheme from a new configuration setting, e.g. tg.url_scheme = 'https' or sth. similar.

comment:13 Changed 11 years ago by singletoned

  • Priority changed from normal to low
  • Status changed from reopened to closed
  • Resolution set to wontfix
  • Severity changed from normal to minor

To be honest, unless there's demand for this from other people, I'd just drop it. I stopped using TG quite a while ago, and therefore don't have any need for the feature anymore.

comment:14 Changed 11 years ago by faide

  • Status changed from closed to reopened
  • Resolution wontfix deleted

+1 for Chris solution. But this helper function is not a _MUST_ have and could be postponed a little bit more, or applied in 1.5...

comment:15 Changed 11 years ago by faide

Please if you don't maintain TG, don't close tickets when developers are working on them. It may become confusing to us :)

comment:16 Changed 11 years ago by faide

  • Milestone changed from 1.5 to 1.1

comment:17 Changed 11 years ago by Chris Arndt

  • Keywords needs tests, needs review added
  • Milestone changed from 1.1 to 1.1 beta 2

comment:18 Changed 11 years ago by Chris Arndt

  • Status changed from reopened to new
  • Owner changed from anonymous to Chris Arndt

comment:19 Changed 11 years ago by Chris Arndt

  • Status changed from new to assigned

comment:20 Changed 11 years ago by faide

  • Milestone changed from 1.1 beta 2 to 1.1

comment:21 Changed 11 years ago by faide

  • Milestone changed from 1.1 to 1.1 beta 2

comment:22 Changed 11 years ago by Chris Arndt

Added new absolute_url function to controllers module in TG 1.1 in r5690.

comment:23 Changed 11 years ago by Chris Arndt

  • Status changed from assigned to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.