Skip to content
This repository

Made static_view raise HTTP exceptions instead of return #593

Merged
merged 4 commits into from about 2 years ago

2 participants

Marin Rukavina Chris McDonough
Marin Rukavina
vipera commented

Referencing issue #586. Please note that HTTPMovedPermanently is likewise being raised, which may or may not be useful.

Chris McDonough
Owner
mcdonc commented

Thank you! Do you think I could tweak you into fixing the test failures that result from this change? (see HACKING.txt in the root to see how to run the tests).

Marin Rukavina
vipera commented

Thanks for the advice! I had to update the tests to check for raised exceptions, since they weren't doing that.

Chris McDonough mcdonc merged commit 3d65afe into from
Chris McDonough mcdonc closed this
Chris McDonough
Owner
mcdonc commented

Thanks! Do you think you might be able to issue another pull request that adds your name to CONTRIBUTORS.txt in the root?

Marin Rukavina
vipera commented

I've done that, thank you. Also thanks for the help with the tests, I didn't see that assertRaises was available so I used all that code. Well, now I know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 2 changed files with 112 additions and 41 deletions. Show diff stats Hide diff stats

  1. +6 6 pyramid/static.py
  2. +106 35 pyramid/tests/test_static.py
12 pyramid/static.py
@@ -101,17 +101,17 @@ def __call__(self, context, request):
101 101 path = _secure_path(path_tuple)
102 102
103 103 if path is None:
104   - return HTTPNotFound('Out of bounds: %s' % request.url)
  104 + raise HTTPNotFound('Out of bounds: %s' % request.url)
105 105
106 106 if self.package_name: # package resource
107 107
108 108 resource_path ='%s/%s' % (self.docroot.rstrip('/'), path)
109 109 if resource_isdir(self.package_name, resource_path):
110 110 if not request.path_url.endswith('/'):
111   - return self.add_slash_redirect(request)
  111 + self.add_slash_redirect(request)
112 112 resource_path = '%s/%s' % (resource_path.rstrip('/'),self.index)
113 113 if not resource_exists(self.package_name, resource_path):
114   - return HTTPNotFound(request.url)
  114 + raise HTTPNotFound(request.url)
115 115 filepath = resource_filename(self.package_name, resource_path)
116 116
117 117 else: # filesystem file
@@ -120,10 +120,10 @@ def __call__(self, context, request):
120 120 filepath = normcase(normpath(join(self.norm_docroot, path)))
121 121 if isdir(filepath):
122 122 if not request.path_url.endswith('/'):
123   - return self.add_slash_redirect(request)
  123 + self.add_slash_redirect(request)
124 124 filepath = join(filepath, self.index)
125 125 if not exists(filepath):
126   - return HTTPNotFound(request.url)
  126 + raise HTTPNotFound(request.url)
127 127
128 128 return FileResponse(filepath, request, self.cache_max_age)
129 129
@@ -132,7 +132,7 @@ def add_slash_redirect(self, request):
132 132 qs = request.query_string
133 133 if qs:
134 134 url = url + '?' + qs
135   - return HTTPMovedPermanently(url)
  135 + raise HTTPMovedPermanently(url)
136 136
137 137 _seps = set(['/', os.sep])
138 138 def _contains_slash(item):
141 pyramid/tests/test_static.py
@@ -38,11 +38,17 @@ def test_call_adds_slash_path_info_empty(self):
38 38 inst = self._makeOne('pyramid.tests:fixtures/static')
39 39 request = self._makeRequest({'PATH_INFO':''})
40 40 context = DummyContext()
41   - response = inst(context, request)
42   - response.prepare(request.environ)
43   - self.assertEqual(response.status, '301 Moved Permanently')
44   - self.assertTrue(b'http://example.com:6543/' in response.body)
45   -
  41 + from pyramid.httpexceptions import HTTPMovedPermanently
  42 + try:
  43 + response = inst(context, request)
  44 + except HTTPMovedPermanently as e:
  45 + self.assertEqual(e.code, 301)
  46 + self.assertTrue(b'http://example.com:6543/' in e.location)
  47 + else:
  48 + response.prepare(request.environ)
  49 + self.assertEqual(response.status, '301 Moved Permanently')
  50 + self.assertTrue(b'http://example.com:6543/' in response.body)
  51 +
46 52 def test_path_info_slash_means_index_html(self):
47 53 inst = self._makeOne('pyramid.tests:fixtures/static')
48 54 request = self._makeRequest()
@@ -70,16 +76,26 @@ def test_oob_dotdotslash(self):
70 76 inst = self._makeOne('pyramid.tests:fixtures/static')
71 77 request = self._makeRequest({'PATH_INFO':'/subdir/../../minimal.pt'})
72 78 context = DummyContext()
73   - response = inst(context, request)
74   - self.assertEqual(response.status, '404 Not Found')
  79 + from pyramid.httpexceptions import HTTPNotFound
  80 + try:
  81 + response = inst(context, request)
  82 + except HTTPNotFound as e:
  83 + self.assertEqual(e.code, 404)
  84 + else:
  85 + self.assertEqual(response.status, '404 Not Found')
75 86
76 87 def test_oob_dotdotslash_encoded(self):
77 88 inst = self._makeOne('pyramid.tests:fixtures/static')
78 89 request = self._makeRequest(
79 90 {'PATH_INFO':'/subdir/%2E%2E%2F%2E%2E/minimal.pt'})
80 91 context = DummyContext()
81   - response = inst(context, request)
82   - self.assertEqual(response.status, '404 Not Found')
  92 + from pyramid.httpexceptions import HTTPNotFound
  93 + try:
  94 + response = inst(context, request)
  95 + except HTTPNotFound as e:
  96 + self.assertEqual(e.code, 404)
  97 + else:
  98 + self.assertEqual(response.status, '404 Not Found')
83 99
84 100 def test_oob_os_sep(self):
85 101 import os
@@ -88,15 +104,25 @@ def test_oob_os_sep(self):
88 104 request = self._makeRequest({'PATH_INFO':'/subdir/%s%sminimal.pt' %
89 105 (dds, dds)})
90 106 context = DummyContext()
91   - response = inst(context, request)
92   - self.assertEqual(response.status, '404 Not Found')
  107 + from pyramid.httpexceptions import HTTPNotFound
  108 + try:
  109 + response = inst(context, request)
  110 + except HTTPNotFound as e:
  111 + self.assertEqual(e.code, 404)
  112 + else:
  113 + self.assertEqual(response.status, '404 Not Found')
93 114
94 115 def test_resource_doesnt_exist(self):
95 116 inst = self._makeOne('pyramid.tests:fixtures/static')
96 117 request = self._makeRequest({'PATH_INFO':'/notthere'})
97 118 context = DummyContext()
98   - response = inst(context, request)
99   - self.assertEqual(response.status, '404 Not Found')
  119 + from pyramid.httpexceptions import HTTPNotFound
  120 + try:
  121 + response = inst(context, request)
  122 + except HTTPNotFound as e:
  123 + self.assertEqual(e.code, 404)
  124 + else:
  125 + self.assertEqual(response.status, '404 Not Found')
100 126
101 127 def test_resource_isdir(self):
102 128 inst = self._makeOne('pyramid.tests:fixtures/static')
@@ -174,8 +200,13 @@ def test_not_found(self):
174 200 inst = self._makeOne('pyramid.tests:fixtures/static')
175 201 request = self._makeRequest({'PATH_INFO':'/notthere.html'})
176 202 context = DummyContext()
177   - response = inst(context, request)
178   - self.assertEqual(response.status, '404 Not Found')
  203 + from pyramid.httpexceptions import HTTPNotFound
  204 + try:
  205 + response = inst(context, request)
  206 + except HTTPNotFound as e:
  207 + self.assertEqual(e.code, 404)
  208 + else:
  209 + self.assertEqual(response.status, '404 Not Found')
179 210
180 211 def test_resource_with_content_encoding(self):
181 212 inst = self._makeOne('pyramid.tests:fixtures/static')
@@ -233,11 +264,17 @@ def test_call_adds_slash_path_info_empty(self):
233 264 request = self._makeRequest({'PATH_INFO':''})
234 265 request.subpath = ()
235 266 context = DummyContext()
236   - response = inst(context, request)
237   - response.prepare(request.environ)
238   - self.assertEqual(response.status, '301 Moved Permanently')
239   - self.assertTrue(b'http://example.com:6543/' in response.body)
240   -
  267 + from pyramid.httpexceptions import HTTPMovedPermanently
  268 + try:
  269 + response = inst(context, request)
  270 + except HTTPMovedPermanently as e:
  271 + self.assertEqual(e.code, 301)
  272 + self.assertTrue(b'http://example.com:6543/' in e.location)
  273 + else:
  274 + response.prepare(request.environ)
  275 + self.assertEqual(response.status, '301 Moved Permanently')
  276 + self.assertTrue(b'http://example.com:6543/' in response.body)
  277 +
241 278 def test_path_info_slash_means_index_html(self):
242 279 inst = self._makeOne('pyramid.tests:fixtures/static')
243 280 request = self._makeRequest()
@@ -251,33 +288,52 @@ def test_oob_singledot(self):
251 288 request = self._makeRequest()
252 289 request.subpath = ('.', 'index.html')
253 290 context = DummyContext()
254   - response = inst(context, request)
255   - self.assertEqual(response.status, '404 Not Found')
  291 + from pyramid.httpexceptions import HTTPNotFound
  292 + try:
  293 + response = inst(context, request)
  294 + except HTTPNotFound as e:
  295 + self.assertEqual(e.code, 404)
  296 + else:
  297 + self.assertEqual(response.status, '404 Not Found')
256 298
257 299 def test_oob_emptyelement(self):
258 300 inst = self._makeOne('pyramid.tests:fixtures/static')
259 301 request = self._makeRequest()
260 302 request.subpath = ('', 'index.html')
261 303 context = DummyContext()
262   - response = inst(context, request)
263   - self.assertEqual(response.status, '404 Not Found')
  304 + from pyramid.httpexceptions import HTTPNotFound
  305 + try:
  306 + response = inst(context, request)
  307 + except HTTPNotFound as e:
  308 + self.assertEqual(e.code, 404)
  309 + else:
  310 + self.assertEqual(response.status, '404 Not Found')
264 311
265 312 def test_oob_dotdotslash(self):
266 313 inst = self._makeOne('pyramid.tests:fixtures/static')
267 314 request = self._makeRequest()
268 315 request.subpath = ('subdir', '..', '..', 'minimal.pt')
269 316 context = DummyContext()
270   - response = inst(context, request)
271   - self.assertEqual(response.status, '404 Not Found')
  317 + from pyramid.httpexceptions import HTTPNotFound
  318 + try:
  319 + response = inst(context, request)
  320 + except HTTPNotFound as e:
  321 + self.assertEqual(e.code, 404)
  322 + else:
  323 + self.assertEqual(response.status, '404 Not Found')
272 324
273 325 def test_oob_dotdotslash_encoded(self):
274 326 inst = self._makeOne('pyramid.tests:fixtures/static')
275 327 request = self._makeRequest()
276 328 request.subpath = ('subdir', '%2E%2E', '%2E%2E', 'minimal.pt')
277 329 context = DummyContext()
278   - response = inst(context, request)
279   - self.assertEqual(response.status, '404 Not Found')
280   -
  330 + from pyramid.httpexceptions import HTTPNotFound
  331 + try:
  332 + response = inst(context, request)
  333 + except HTTPNotFound as e:
  334 + self.assertEqual(e.code, 404)
  335 + else:
  336 + self.assertEqual(response.status, '404 Not Found')
281 337 def test_oob_os_sep(self):
282 338 import os
283 339 inst = self._makeOne('pyramid.tests:fixtures/static')
@@ -285,16 +341,26 @@ def test_oob_os_sep(self):
285 341 request = self._makeRequest()
286 342 request.subpath = ('subdir', dds, dds, 'minimal.pt')
287 343 context = DummyContext()
288   - response = inst(context, request)
289   - self.assertEqual(response.status, '404 Not Found')
  344 + from pyramid.httpexceptions import HTTPNotFound
  345 + try:
  346 + response = inst(context, request)
  347 + except HTTPNotFound as e:
  348 + self.assertEqual(e.code, 404)
  349 + else:
  350 + self.assertEqual(response.status, '404 Not Found')
290 351
291 352 def test_resource_doesnt_exist(self):
292 353 inst = self._makeOne('pyramid.tests:fixtures/static')
293 354 request = self._makeRequest()
294 355 request.subpath = ('notthere,')
295 356 context = DummyContext()
296   - response = inst(context, request)
297   - self.assertEqual(response.status, '404 Not Found')
  357 + from pyramid.httpexceptions import HTTPNotFound
  358 + try:
  359 + response = inst(context, request)
  360 + except HTTPNotFound as e:
  361 + self.assertEqual(e.code, 404)
  362 + else:
  363 + self.assertEqual(response.status, '404 Not Found')
298 364
299 365 def test_resource_isdir(self):
300 366 inst = self._makeOne('pyramid.tests:fixtures/static')
@@ -361,8 +427,13 @@ def test_not_found(self):
361 427 request = self._makeRequest()
362 428 request.subpath = ('notthere.html',)
363 429 context = DummyContext()
364   - response = inst(context, request)
365   - self.assertEqual(response.status, '404 Not Found')
  430 + from pyramid.httpexceptions import HTTPNotFound
  431 + try:
  432 + response = inst(context, request)
  433 + except HTTPNotFound as e:
  434 + self.assertEqual(e.code, 404)
  435 + else:
  436 + self.assertEqual(response.status, '404 Not Found')
366 437
367 438 class DummyContext:
368 439 pass

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.