Merge lp:~james-w/summit/fix-overeager-escaping into lp:summit

Proposed by James Westby on 2011-09-21
Status: Merged
Approved by: Michael Hall on 2011-09-21
Approved revision: 187
Merged at revision: 190
Proposed branch: lp:~james-w/summit/fix-overeager-escaping
Merge into: lp:summit
Diff against target: 87 lines (+10/-16)
2 files modified
summit/schedule/render.py (+4/-9)
summit/schedule/tests.py (+6/-7)
To merge this branch: bzr merge lp:~james-w/summit/fix-overeager-escaping
Reviewer Review Type Date Requested Status
Michael Hall (community) 2011-09-21 Approve on 2011-09-21
Review via email: mp+76478@code.launchpad.net

Commit message

Don't escape URLs before putting them in the HTML.

Description of the change

Hi,

The moral of the story here is to not believe everything you read
on the internet.

Thanks,

James

To post a comment you must log in.
Michael Hall (mhall119) wrote :

Looks good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'summit/schedule/render.py'
2--- summit/schedule/render.py 2011-09-17 19:39:58 +0000
3+++ summit/schedule/render.py 2011-09-21 21:02:31 +0000
4@@ -38,11 +38,6 @@
5 return escape(escaped)
6
7
8-def urlsafe(value):
9- quoted = urllib.quote(value)
10- return escape_strings(quoted)
11-
12-
13 def get_style_def_for_meeting(meeting, pos=None):
14 if pos:
15 style = {
16@@ -762,16 +757,16 @@
17 if should_have_details_link:
18 html += (('<a href="%s"><img src="%simg/pad.png" ' +
19 'title="%s" alt="(%s)" height="16" width="16"/></a>\n')
20- % (urlsafe(meeting.meeting_page_url), settings.MEDIA_URL
21+ % (escape_strings(meeting.meeting_page_url), settings.MEDIA_URL
22 , "meeting details", "meeting details"))
23
24 if icon:
25 html += (('<a href="%s"><img src="%simg/blueprint-%s.png" ' +
26 'title="%s" alt="(%s)" height="14" width="14"/></a>\n')
27- % (urlsafe(spec), settings.MEDIA_URL,
28+ % (escape_strings(spec), settings.MEDIA_URL,
29 icon, label, label.lower()))
30 if url:
31- html += '<a href="%s">' % urlsafe(url)
32+ html += '<a href="%s">' % escape_strings(url)
33 html += escape_strings(meeting.title)
34 if url:
35 html += '</a>'
36@@ -803,7 +798,7 @@
37 html += ' <li class="missing">'
38 else:
39 html += ' <li>'
40- html += '<a href="http://launchpad.net/~%s" ' % urlsafe(attendee.user.username)
41+ html += '<a href="http://launchpad.net/~%s" ' % escape_strings(attendee.user.username)
42 html += 'title="%s">' % escape_strings(attendee.user.get_full_name())
43 html += '%s' % escape_strings(attendee.user.username)
44 html += '</a>'
45
46=== modified file 'summit/schedule/tests.py'
47--- summit/schedule/tests.py 2011-09-21 13:30:31 +0000
48+++ summit/schedule/tests.py 2011-09-21 21:02:31 +0000
49@@ -681,7 +681,7 @@
50
51 response = self.request_schedule()
52
53- self.assertContains(response, 'test%25meeting', 1)
54+ self.assertContains(response, 'test%meeting', 1)
55
56 def test_percent_in_meeting_title(self):
57 self.meeting1.title = 'test % meeting'
58@@ -706,7 +706,7 @@
59
60 response = self.request_schedule()
61
62- self.assertContains(response, 'spec%25url', 2)
63+ self.assertContains(response, 'spec%url', 2)
64
65 def test_percent_in_wiki_url(self):
66 self.meeting1.wiki_url = 'wiki%url'
67@@ -714,7 +714,7 @@
68
69 response = self.request_schedule()
70
71- self.assertContains(response, 'wiki%25url', 1)
72+ self.assertContains(response, 'wiki%url', 1)
73
74 def test_percent_in_room_title(self):
75 self.meeting1.type = 'talk'
76@@ -754,10 +754,9 @@
77
78 response = self.request_schedule()
79
80- # Once as the link to the LP page
81- self.assertContains(response, '~test%25user', 1)
82- # and once visible on the page
83- self.assertContains(response, '>test%user', 1)
84+ # Expect two instances, one in the link href, and one in the link
85+ # text
86+ self.assertContains(response, 'test%user', 2)
87 self.assertContains(response, 'Test % User', 1)
88
89

Subscribers

People subscribed via source and target branches