Launchpad itself

redirects should canonicalise URLs

Bug #5461 reported by Dafydd Harries
4
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Medium
James Henstridge

Bug Description

class DistributionNavigation(...):
    redirection('+source', '..')
    ...

/distros/ubuntu/+source/ correctly redirects to /distros/ubuntu/
/distros/ubuntu/+source incorrectly redirects to /distros/

Steve suggested redirection('+source', './..') as a workaround, but it didn't work.

Steve Alexander (stevea)
Changed in launchpad:
assignee: nobody → stevea
status: New → Accepted
James Henstridge (jamesh)
Changed in launchpad:
assignee: stevea → jamesh
Revision history for this message
James Henstridge (jamesh) wrote :

It is now possible to calculate the redirect target in the navigation class with the following idiom:

  @redirection('+source', status=301)
  def redirect_source(self):
      return canonical_url(self.context)

(I included the status code here to indicate a permanent redirect)

Changed in launchpad:
status: Accepted → Fixed
Revision history for this message
Dafydd Harries (daf) wrote :

Great!

However, it strikes me that the 'status' keyword parameter is a little awkward, as it should always be either 301 or 302. Perhaps a 'permanent' parameter, defaulting to False, would make for a nicer API?

Revision history for this message
Steve Alexander (stevea) wrote :

I'd like to leave 'status' as it is. If you're explicitly using 'status' then you need to understand the implications of it. Also, this is in line with the Zope 3 API for redirection.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.