Opened 5 years ago

Closed 3 years 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.

Change History (6)

comment:1 Changed 5 years ago by Aymeric Augustin

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

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 5 years ago by Aymeric Augustin

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

comment:3 Changed 3 years ago by Germano Gabbianelli

Has patch: set
Needs tests: set
Version: 1.3master

comment:4 Changed 3 years ago by tcsorrel

Cc: thomas@… added
Owner: changed from nobody to tcsorrel
Status: newassigned

comment:5 Changed 3 years 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 3 years ago by Baptiste Mispelon <bmispelon@…>

Resolution: fixed
Status: assignedclosed

In 53c576452e2e8feef857cd67a5c17f9159d95eb6:

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

Note: See TracTickets for help on using tickets.
Back to Top