Code

Opened 3 years ago

Closed 6 weeks ago

#16727 closed Bug (fixed)

contrib.contenttypes.views.shortcut error

Reported by: anonymous Owned by: tcsorrel
Component: contrib.contenttypes Version: master
Severity: Normal Keywords:
Cc: thomas@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There is a problem during absurl processing:

absurl = obj.get_absolute_url()
if absurl.startswith('http://') or absurl.startswith('https://'):
    return http.HttpResponseRedirect(absurl)
else:
    ...

So, if get_absolute_url returns 'sub.example.com/entry.html' shortcut view will return something like 'http://example.com//sub.example.com/entry.html'

This problem was founded with help of django_hosts application. Author of this app decided to return urls without 'http:' or 'https:' prefix. Here is the explanation:

The double slash at the beginning of the href is an easy way to not have to worry about which scheme (http or https) is used. Your browser will automatically choose the currently used scheme. If you're on https://mysite.com/ a link with an href of mysite.com/about/ would actually point to https://mysite.com/about/. For more information see the The protocol-relative URL article by Paul Irish or the appropriate section in RFC 3986.

I think the better way is urlparse lib using to decide on the url kind.

Attachments (0)

Change History (6)

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

By grepping the entire codebase for startswith\(['"]https?://, I have found this pattern in the following files:

  • django.contrib.contenttypes.views (line 25) => it's the bug reported here,
  • django.contrib.syndication.views (line 11) => it's a similar bug,
  • django.utils.html (line 132) => it's debatable, we'd probably better not touch this.

comment:2 Changed 3 years ago by aaugustin

#16753 was opened for the bug in syndication. This one is just about contenttypes.

comment:3 Changed 2 months ago by tyrion

  • Has patch set
  • Needs tests set
  • Version changed from 1.3 to master

comment:4 Changed 7 weeks ago by tcsorrel

  • Cc thomas@… added
  • Owner changed from nobody to tcsorrel
  • Status changed from new to assigned

comment:5 Changed 6 weeks ago by tcsorrel

  • Needs tests unset

Please find a fix with the corresponding tests included in this pull request :
https://github.com/django/django/pull/2388
I preferred not to use regular expression thinking that 3 "startswith" tests might be faster.

comment:6 Changed 6 weeks ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 53c576452e2e8feef857cd67a5c17f9159d95eb6:

Fixed #16727 -- Added protocol-relative URL support to contenttypes.views.shortcut.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.