Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixed #16383 #2979

Closed
wants to merge 1 commit into from

3 participants

@coder9042

No description provided.

@coder9042

@timgraham
Is this approach fine?

tests/template_tests/tests.py
@@ -810,6 +814,10 @@ def get_template_tests(self):
'filter-syntax23': (r'1{{ var.noisy_fail_key }}2', {"var": SomeClass()}, (SomeOtherException, SomeOtherException)),
'filter-syntax24': (r'1{{ var.noisy_fail_attribute }}2', {"var": SomeClass()}, (SomeOtherException, SomeOtherException)),
+ # When a property raises AttributeError, it resulted in silent failure. Now it gives original AttributeError
+ # Refs. 16383
+ 'filter-syntax24': ('{{ var.attribute_error_attribute }}', {"var": SomeClass()}, (AttributeError)),

This 'filter-syntax24' key is duplicated. seems good to use 'filter-syntax25'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
django/template/base.py
@@ -773,7 +773,10 @@ def _resolve_lookup(self, context):
if isinstance(current, BaseContext) and getattr(type(current), bit):
raise AttributeError
current = getattr(current, bit)
- except (TypeError, AttributeError):
+ except (TypeError, AttributeError) as e:
+ if isinstance(e, AttributeError):
+ if(not isinstance(current, BaseContext) and bit in dir(current)):

this parenthesis is redundant and white space after 'if' is required.
like 'if not isinstance ... dir(current):'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@hirokiky

Commented about only problems on the code (not specification, not regression).

django/template/base.py
@@ -773,7 +773,10 @@ def _resolve_lookup(self, context):
if isinstance(current, BaseContext) and getattr(type(current), bit):
raise AttributeError
current = getattr(current, bit)
- except (TypeError, AttributeError):
+ except (TypeError, AttributeError) as e:
+ if isinstance(e, AttributeError):
+ if not isinstance(current, BaseContext) and bit in dir(current):
+ return getattr(current, bit)
@timgraham Owner

can we change this to raise e to just re-raise the original exception rather than re-evaluating it?

Things are easier to understand this way as the line which actually raised exception is seen...if we do what you suggest, we might not get that.

@timgraham Owner

Sorry, I should have said simply raise without the e.

Ok that seems better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
django/template/base.py
@@ -773,7 +773,10 @@ def _resolve_lookup(self, context):
if isinstance(current, BaseContext) and getattr(type(current), bit):
raise AttributeError
current = getattr(current, bit)
- except (TypeError, AttributeError):
+ except (TypeError, AttributeError) as e:
+ if isinstance(e, AttributeError):
@timgraham Owner

This code needs comments.
Also, seems like we can combine the if statements instead of nesting them?

Will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@coder9042 coder9042 Fixed #16383 -- Raised the AttributeError raised in property of an ob…
…ject when used in a template.

Thanks Hiroki and Tim Graham for review.
381bf37
@timgraham
Owner

merged in 0dd05c9 with minor edits.

@timgraham timgraham closed this
@coder9042

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 30, 2014
  1. @coder9042

    Fixed #16383 -- Raised the AttributeError raised in property of an ob…

    coder9042 authored
    …ject when used in a template.
    
    Thanks Hiroki and Tim Graham for review.
This page is out of date. Refresh to see the latest.
Showing with 14 additions and 1 deletion.
  1. +6 −1 django/template/base.py
  2. +8 −0 tests/template_tests/tests.py
View
7 django/template/base.py
@@ -773,7 +773,12 @@ def _resolve_lookup(self, context):
if isinstance(current, BaseContext) and getattr(type(current), bit):
raise AttributeError
current = getattr(current, bit)
- except (TypeError, AttributeError):
+ except (TypeError, AttributeError) as e:
+ # If bit is a property method of current which raises AttributeError
+ # inside itself, it returns that property and hence the error is raised
+ # refs #16383
+ if isinstance(e, AttributeError) and not isinstance(current, BaseContext) and bit in dir(current):
+ raise
try: # list-index lookup
current = current[int(bit)]
except (IndexError, # list index out of range
View
8 tests/template_tests/tests.py
@@ -117,6 +117,10 @@ def noisy_fail_attribute(self):
raise SomeOtherException
noisy_fail_attribute = property(noisy_fail_attribute)
+ def attribute_error_attribute(self):
+ raise AttributeError
+ attribute_error_attribute = property(attribute_error_attribute)
+
class OtherClass:
def method(self):
@@ -810,6 +814,10 @@ def get_template_tests(self):
'filter-syntax23': (r'1{{ var.noisy_fail_key }}2', {"var": SomeClass()}, (SomeOtherException, SomeOtherException)),
'filter-syntax24': (r'1{{ var.noisy_fail_attribute }}2', {"var": SomeClass()}, (SomeOtherException, SomeOtherException)),
+ # When a property raises AttributeError, it resulted in silent failure. Now it gives original AttributeError
+ # Refs. 16383
+ 'filter-syntax25': ('{{ var.attribute_error_attribute }}', {"var": SomeClass()}, (AttributeError)),
+
### COMMENT SYNTAX ########################################################
'comment-syntax01': ("{# this is hidden #}hello", {}, "hello"),
'comment-syntax02': ("{# this is hidden #}hello{# foo #}", {}, "hello"),
Something went wrong with that request. Please try again.