Skip to content

Commit 4a11125

Browse files
deniszhdeniszh
authored
Merge pull request #2904 from graphite-project/copilot/fix-issue-2872
Fix test_standard_finder failure in Python 3.13
2 parents d6dfc59 + 99c1508 commit 4a11125

3 files changed

Lines changed: 69 additions & 21 deletions

File tree

‎webapp/graphite/url_shortener/views.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ def follow(request, link_id):
1414
"""Follow existing links"""
1515
key = base62.to_decimal(link_id)
1616
link = get_object_or_404(Link, pk=key)
17-
return HttpResponsePermanentRedirect(reverse('browser') + link.url)
17+
# Strip leading slashes from the stored URL to prevent open redirect via
18+
# protocol-relative URLs (e.g. //evil.com) being used as redirect targets.
19+
url = reverse('browser') + link.url.lstrip('/')
20+
return HttpResponsePermanentRedirect(url)
1821

1922

2023
def shorten(request, path):

‎webapp/tests/test_finders.py‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -148,83 +148,83 @@ def test_standard_finder(self,scandir_mock):
148148

149149
finder = get_finders('graphite.finders.standard.StandardFinder')[0]
150150

151-
scandir_mock.call_count = 0
151+
scandir_mock.reset_mock()
152152
nodes = finder.find_nodes(FindQuery('foo', None, None))
153153
self.assertEqual(len(list(nodes)), 2)
154154
self.assertEqual(scandir_mock.call_count, 0)
155155

156-
scandir_mock.call_count = 0
156+
scandir_mock.reset_mock()
157157
nodes = finder.find_nodes(FindQuery('foo.bar.baz', None, None))
158158
self.assertEqual(len(list(nodes)), 1)
159159
self.assertEqual(scandir_mock.call_count, 0)
160160

161161
# test for https://github.com/grafana/grafana/issues/5936
162-
scandir_mock.call_count = 0
162+
scandir_mock.reset_mock()
163163
nodes = finder.find_nodes(FindQuery('foo.{bar}.baz', None, None))
164164
self.assertEqual(len(list(nodes)), 1)
165165
self.assertEqual(scandir_mock.call_count, 0)
166166

167-
scandir_mock.call_count = 0
167+
scandir_mock.reset_mock()
168168
nodes = finder.find_nodes(FindQuery('foo.{bar,}.baz', None, None))
169169
self.assertEqual(len(list(nodes)), 1)
170170
self.assertEqual(scandir_mock.call_count, 0)
171171

172-
scandir_mock.call_count = 0
172+
scandir_mock.reset_mock()
173173
nodes = finder.find_nodes(FindQuery('*.ba?.{baz,foo}', None, None))
174174
self.assertEqual(len(list(nodes)), 2)
175175
self.assertEqual(scandir_mock.call_count, 4)
176176

177-
scandir_mock.call_count = 0
177+
scandir_mock.reset_mock()
178178
nodes = finder.find_nodes(FindQuery('{foo,bar}.{baz,bar}.{baz,foo}', None, None))
179179
self.assertEqual(len(list(nodes)), 2)
180180
self.assertEqual(scandir_mock.call_count, 0)
181181

182-
scandir_mock.call_count = 0
182+
scandir_mock.reset_mock()
183183
nodes = finder.find_nodes(FindQuery('{foo}.bar.*', None, None))
184184
self.assertEqual(len(list(nodes)), 1)
185185
self.assertEqual(scandir_mock.call_count, 1)
186186

187-
scandir_mock.call_count = 0
187+
scandir_mock.reset_mock()
188188
nodes = finder.find_nodes(FindQuery('foo.{ba{r,z},baz}.baz', None, None))
189189
self.assertEqual(len(list(nodes)), 1)
190190
self.assertEqual(scandir_mock.call_count, 0)
191191

192-
scandir_mock.call_count = 0
192+
scandir_mock.reset_mock()
193193
nodes = finder.find_nodes(FindQuery('{foo,garbage}.bar.baz', None, None))
194194
self.assertEqual(len(list(nodes)), 1)
195195
self.assertEqual(scandir_mock.call_count, 0)
196196

197-
scandir_mock.call_count = 0
197+
scandir_mock.reset_mock()
198198
nodes = finder.find_nodes(FindQuery('{fo{o}}.bar.baz', None, None))
199199
self.assertEqual(len(list(nodes)), 1)
200200
self.assertEqual(scandir_mock.call_count, 0)
201201

202-
scandir_mock.call_count = 0
202+
scandir_mock.reset_mock()
203203
nodes = finder.find_nodes(FindQuery('foo{}.bar.baz', None, None))
204204
self.assertEqual(len(list(nodes)), 1)
205205
self.assertEqual(scandir_mock.call_count, 0)
206206

207-
scandir_mock.call_count = 0
207+
scandir_mock.reset_mock()
208208
nodes = finder.find_nodes(FindQuery('{fo,ba}{o}.bar.baz', None, None))
209209
self.assertEqual(len(list(nodes)), 1)
210210
self.assertEqual(scandir_mock.call_count, 0)
211211

212-
scandir_mock.call_count = 0
212+
scandir_mock.reset_mock()
213213
nodes = finder.find_nodes(FindQuery('{fo,ba}{o,o}.bar.baz', None, None))
214214
self.assertEqual(len(list(nodes)), 1)
215215
self.assertEqual(scandir_mock.call_count, 0)
216216

217-
scandir_mock.call_count = 0
217+
scandir_mock.reset_mock()
218218
nodes = finder.find_nodes(FindQuery('{fo,ba}{o,z}.bar.baz', None, None))
219219
self.assertEqual(len(list(nodes)), 1)
220220
self.assertEqual(scandir_mock.call_count, 0)
221221

222-
scandir_mock.call_count = 0
222+
scandir_mock.reset_mock()
223223
nodes = finder.find_nodes(FindQuery('foo;bar=baz', None, None))
224224
self.assertEqual(len(list(nodes)), 1)
225225
self.assertEqual(scandir_mock.call_count, 0)
226226

227-
scandir_mock.call_count = 0
227+
scandir_mock.reset_mock()
228228
nodes = finder.find_nodes(FindQuery('foo;bar=baz2', None, None))
229229
self.assertEqual(len(list(nodes)), 1)
230230
self.assertEqual(scandir_mock.call_count, 0)
@@ -233,7 +233,7 @@ def test_standard_finder(self,scandir_mock):
233233
self.assertEqual(results, [])
234234

235235
finally:
236-
scandir_mock.call_count = 0
236+
scandir_mock.reset_mock()
237237
self.wipe_whisper()
238238

239239
@patch('graphite.finders.standard.scandir', wraps=scandir_mock)
@@ -244,18 +244,18 @@ def test_standard_finder_gzipped_whisper(self, scandir_mock):
244244
self.create_whisper(join('bar', 'baz', 'foo.wsp'))
245245
finder = get_finders('graphite.finders.standard.StandardFinder')[0]
246246

247-
scandir_mock.call_count = 0
247+
scandir_mock.reset_mock()
248248
nodes = finder.find_nodes(FindQuery('foo', None, None))
249249
self.assertEqual(len(list(nodes)), 2)
250250
self.assertEqual(scandir_mock.call_count, 0)
251251

252-
scandir_mock.call_count = 0
252+
scandir_mock.reset_mock()
253253
nodes = finder.find_nodes(FindQuery('foo{}.bar.baz', None, None))
254254
self.assertEqual(len(list(nodes)), 1)
255255
self.assertEqual(scandir_mock.call_count, 0)
256256

257257
finally:
258-
scandir_mock.call_count = 0
258+
scandir_mock.reset_mock()
259259
self.wipe_whisper()
260260

261261
def test_standard_finder_tagged_whisper_carbonlink(self):

‎webapp/tests/test_url_shortener.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
try:
2+
from django.urls import reverse
3+
except ImportError: # Django < 1.10
4+
from django.core.urlresolvers import reverse
5+
6+
from urllib.parse import urlparse
7+
8+
from .base import TestCase
9+
10+
11+
class UrlShortenerTest(TestCase):
12+
def test_shorten_and_follow(self):
13+
"""Test normal URL shortening and following."""
14+
shorten_url = reverse('shorten', kwargs={'path': 'render/'})
15+
response = self.client.get(shorten_url, {'target': 'test'})
16+
self.assertEqual(response.status_code, 200)
17+
short_path = response.content.decode('utf-8')
18+
19+
follow_response = self.client.get(short_path)
20+
self.assertEqual(follow_response.status_code, 301)
21+
redirect_url = follow_response['Location']
22+
# Should redirect to an internal path, not a protocol-relative or absolute URL
23+
self.assertFalse(redirect_url.startswith('//'))
24+
parsed = urlparse(redirect_url)
25+
self.assertFalse(bool(parsed.netloc),
26+
'Redirect to external domain detected: %s' % redirect_url)
27+
28+
def test_follow_open_redirect_prevention(self):
29+
"""Test that protocol-relative URLs stored in links cannot cause open redirects."""
30+
# Simulate shortening a crafted path that starts with //
31+
shorten_url = reverse('shorten', kwargs={'path': '//evil.com'})
32+
response = self.client.get(shorten_url)
33+
self.assertEqual(response.status_code, 200)
34+
short_path = response.content.decode('utf-8')
35+
36+
follow_response = self.client.get(short_path)
37+
self.assertEqual(follow_response.status_code, 301)
38+
redirect_url = follow_response['Location']
39+
# The redirect must not be a protocol-relative URL pointing to an external domain
40+
self.assertFalse(redirect_url.startswith('//'),
41+
'Open redirect detected: %s' % redirect_url)
42+
# Should not redirect to an external host
43+
parsed = urlparse(redirect_url)
44+
self.assertFalse(bool(parsed.netloc),
45+
'Open redirect to external domain detected: %s' % redirect_url)

0 commit comments

Comments
 (0)