From bd7977eed79bd8fc4d9e73b6d848b0f0cd5a72ec Mon Sep 17 00:00:00 2001 From: Marc Tamlyn Date: Wed, 6 Feb 2013 13:05:17 +0000 Subject: [PATCH 1/2] Purge naked excepts. Most of these had obvious exceptions which would be thrown. Some I'm not sure about but they should at least catch only Exception so as not to ignore SystemExit and other inappropriate Error classes. --- rest_framework/compat.py | 6 +++--- rest_framework/fields.py | 2 +- rest_framework/relations.py | 20 +++++++++---------- rest_framework/renderers.py | 4 ++-- rest_framework/templatetags/rest_framework.py | 9 +++++---- rest_framework/tests/urlpatterns.py | 4 ++-- rest_framework/views.py | 2 +- 7 files changed, 24 insertions(+), 23 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 0d512342b..9636b9c17 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -10,13 +10,13 @@ import django # Try to import six from Django, fallback to included `six`. try: from django.utils import six -except: +except ImportError: from rest_framework import six # location of patterns, url, include changes in 1.4 onwards try: from django.conf.urls import patterns, url, include -except: +except ImportError: from django.conf.urls.defaults import patterns, url, include # Handle django.utils.encoding rename: @@ -35,7 +35,7 @@ except ImportError: # django-filter is optional try: import django_filters -except: +except ImportError: django_filters = None diff --git a/rest_framework/fields.py b/rest_framework/fields.py index d6e8539d4..2c3e59b58 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -228,7 +228,7 @@ class ModelField(WritableField): def __init__(self, *args, **kwargs): try: self.model_field = kwargs.pop('model_field') - except: + except KeyError: raise ValueError("ModelField requires 'model_field' kwarg") self.min_length = kwargs.pop('min_length', diff --git a/rest_framework/relations.py b/rest_framework/relations.py index ae4ef6b32..48946e218 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -1,5 +1,5 @@ from __future__ import unicode_literals -from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError, NoReverseMatch from django.core.urlresolvers import resolve, get_script_prefix from django import forms from django.forms import widgets @@ -59,7 +59,7 @@ class RelatedField(WritableField): self.queryset = manager.related.model._default_manager.all() else: # Reverse self.queryset = manager.field.rel.to._default_manager.all() - except: + except Exception: raise msg = ('Serializer related fields must include a `queryset`' + ' argument or set `read_only=True') @@ -290,7 +290,7 @@ class HyperlinkedRelatedField(RelatedField): def __init__(self, *args, **kwargs): try: self.view_name = kwargs.pop('view_name') - except: + except KeyError: raise ValueError("Hyperlinked field requires 'view_name' kwarg") self.slug_field = kwargs.pop('slug_field', self.slug_field) @@ -317,7 +317,7 @@ class HyperlinkedRelatedField(RelatedField): kwargs = {self.pk_url_kwarg: pk} try: return reverse(view_name, kwargs=kwargs, request=request, format=format) - except: + except NoReverseMatch: pass slug = getattr(obj, self.slug_field, None) @@ -328,13 +328,13 @@ class HyperlinkedRelatedField(RelatedField): kwargs = {self.slug_url_kwarg: slug} try: return reverse(view_name, kwargs=kwargs, request=request, format=format) - except: + except NoReverseMatch: pass kwargs = {self.pk_url_kwarg: obj.pk, self.slug_url_kwarg: slug} try: return reverse(view_name, kwargs=kwargs, request=request, format=format) - except: + except NoReverseMatch: pass raise Exception('Could not resolve URL for field using view name "%s"' % view_name) @@ -360,7 +360,7 @@ class HyperlinkedRelatedField(RelatedField): try: match = resolve(value) - except: + except Exception: raise ValidationError(self.error_messages['no_match']) if match.view_name != self.view_name: @@ -434,7 +434,7 @@ class HyperlinkedIdentityField(Field): try: return reverse(view_name, kwargs=kwargs, request=request, format=format) - except: + except NoReverseMatch: pass slug = getattr(obj, self.slug_field, None) @@ -445,13 +445,13 @@ class HyperlinkedIdentityField(Field): kwargs = {self.slug_url_kwarg: slug} try: return reverse(view_name, kwargs=kwargs, request=request, format=format) - except: + except NoReverseMatch: pass kwargs = {self.pk_url_kwarg: obj.pk, self.slug_url_kwarg: slug} try: return reverse(view_name, kwargs=kwargs, request=request, format=format) - except: + except NoReverseMatch: pass raise Exception('Could not resolve URL for field using view name "%s"' % view_name) diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 74c7e2c98..960d48497 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -217,7 +217,7 @@ class TemplateHTMLRenderer(BaseRenderer): try: # Try to find an appropriate error template return self.resolve_template(template_names) - except: + except Exception: # Fall back to using eg '404 Not Found' return Template('%d %s' % (response.status_code, response.status_text.title())) @@ -303,7 +303,7 @@ class BrowsableAPIRenderer(BaseRenderer): try: if not view.has_permission(request, obj): return # Don't have permission - except: + except Exception: return # Don't have permission and exception explicitly raise return True diff --git a/rest_framework/templatetags/rest_framework.py b/rest_framework/templatetags/rest_framework.py index a1db65bc6..1c962798e 100644 --- a/rest_framework/templatetags/rest_framework.py +++ b/rest_framework/templatetags/rest_framework.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals, absolute_import from django import template +from django.core.exceptions import NoReverseMatch from django.core.urlresolvers import reverse from django.http import QueryDict from django.utils.html import escape @@ -31,7 +32,7 @@ try: # Django 1.5+ def do_static(parser, token): return StaticFilesNode.handle_token(parser, token) -except: +except ImportError: try: # Django 1.4 from django.contrib.staticfiles.storage import staticfiles_storage @@ -43,7 +44,7 @@ except: """ return staticfiles_storage.url(path) - except: # Django 1.3 + except ImportError: # Django 1.3 from urlparse import urljoin from django import template from django.templatetags.static import PrefixNode @@ -137,7 +138,7 @@ def optional_login(request): """ try: login_url = reverse('rest_framework:login') - except: + except NoReverseMatch: return '' snippet = "Log in" % (login_url, request.path) @@ -151,7 +152,7 @@ def optional_logout(request): """ try: logout_url = reverse('rest_framework:logout') - except: + except NoReverseMatch: return '' snippet = "Log out" % (logout_url, request.path) diff --git a/rest_framework/tests/urlpatterns.py b/rest_framework/tests/urlpatterns.py index 41245ad15..82cd6cdbe 100644 --- a/rest_framework/tests/urlpatterns.py +++ b/rest_framework/tests/urlpatterns.py @@ -23,14 +23,14 @@ class FormatSuffixTests(TestCase): factory = RequestFactory() try: urlpatterns = format_suffix_patterns(urlpatterns) - except: + except Exception: self.fail("Failed to apply `format_suffix_patterns` on the supplied urlpatterns") resolver = urlresolvers.RegexURLResolver(r'^/', urlpatterns) for test_path in test_paths: request = factory.get(test_path.path) try: callback, callback_args, callback_kwargs = resolver.resolve(request.path_info) - except: + except Exception: self.fail("Failed to resolve URL: %s" % request.path_info) self.assertEquals(callback_args, test_path.args) self.assertEquals(callback_kwargs, test_path.kwargs) diff --git a/rest_framework/views.py b/rest_framework/views.py index ef2b5f920..fd6b4313f 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -252,7 +252,7 @@ class APIView(View): try: return conneg.select_renderer(request, renderers, self.format_kwarg) - except: + except Exception: if force: return (renderers[0], renderers[0].media_type) raise From 11610e7c3c4330b863c9da5d843b6d874c2958e9 Mon Sep 17 00:00:00 2001 From: Marc Tamlyn Date: Wed, 6 Feb 2013 13:10:54 +0000 Subject: [PATCH 2/2] Try the correct NoReverseMatch location. --- rest_framework/relations.py | 4 ++-- rest_framework/templatetags/rest_framework.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 48946e218..53fd646d0 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from django.core.exceptions import ObjectDoesNotExist, ValidationError, NoReverseMatch -from django.core.urlresolvers import resolve, get_script_prefix +from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django.core.urlresolvers import resolve, get_script_prefix, NoReverseMatch from django import forms from django.forms import widgets from django.forms.models import ModelChoiceIterator diff --git a/rest_framework/templatetags/rest_framework.py b/rest_framework/templatetags/rest_framework.py index 1c962798e..c21ddcd7b 100644 --- a/rest_framework/templatetags/rest_framework.py +++ b/rest_framework/templatetags/rest_framework.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals, absolute_import from django import template -from django.core.exceptions import NoReverseMatch -from django.core.urlresolvers import reverse +from django.core.urlresolvers import reverse, NoReverseMatch from django.http import QueryDict from django.utils.html import escape from django.utils.safestring import SafeData, mark_safe