issue python-jsonschema#158: Use try-finally to ensure resolver scopes_stack empty when

iteration breaks (no detectable performance penalty).

* Replace non-python-2.6 DefragResult with named-tuple.
* Add test-case checking scopes_stack empty.

Author: ankostis

jsonschema/compat.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+from collections import namedtuple
 import operator
 import sys
 
@@ -16,7 +17,6 @@
     from io import StringIO
     from urllib.parse import (
         unquote, urljoin, urlunsplit, SplitResult, urlsplit as _urlsplit,
-        DefragResult
     )
     from urllib.request import urlopen
     str_types = str,
@@ -27,7 +27,6 @@
     from StringIO import StringIO
     from urlparse import (
         urljoin, urlunsplit, SplitResult, urlsplit as _urlsplit, # noqa
-        DefragResult
     )
     from urllib import unquote  # noqa
     from urllib2 import urlopen  # noqa
@@ -44,6 +43,7 @@ def urlsplit(url):
     return SplitResult(scheme, netloc, path, query, fragment)
 
 
+DefragResult = namedtuple('DefragResult', 'url fragment')
 def urldefrag(url):
     if "#" in url:
         s, n, p, q, frag = urlsplit(url)

jsonschema/tests/test_validators.py

@@ -558,6 +558,25 @@ def test_is_type_raises_exception_for_unknown_type(self):
         with self.assertRaises(UnknownType):
             self.validator.is_type("foo", object())
 
+    def test_resolver_scope_stack_is_empty_after_exception(self):
+        instance ={
+            'foo': 1
+        }
+        schema = {
+            'id':'/a/b',
+            'type': ['object'],
+            'properties': {
+                'foo': {
+                    'id':'c',
+                    'type': ['string']
+                }
+            }
+        }
+        with self.assertRaises(ValidationError):
+            self.validator.validate(instance, schema)
+
+        self.assertEqual(len(self.validator.resolver.scopes_stack), 0)
+
 
 class TestDraft3Validator(ValidatorTestMixin, unittest.TestCase):
     validator_class = Draft3Validator

jsonschema/validators.py

@@ -83,34 +83,33 @@ def iter_errors(self, instance, _schema=None):
             scope = _schema.get(u"id")
             if scope:
                 self.resolver.push_scope(scope)
-
-
-            ref = _schema.get(u"$ref")
-            if ref is None:
-                validators = iteritems(_schema)
-            else:
-                validators = [(u"$ref", ref)]
-
-            for k, v in validators:
-                validator = self.VALIDATORS.get(k)
-                if validator is None:
-                    continue
-
-                errors = validator(self, v, instance, _schema) or ()
-                for error in errors:
-                    # set details if not already set by the called fn
-                    error._set(
-                        validator=k,
-                        validator_value=v,
-                        instance=instance,
-                        schema=_schema,
-                    )
-                    if k != u"$ref":
-                        error.schema_path.appendleft(k)
-                    yield error
-
-            if scope:
-                self.resolver.pop_scope()
+            try:
+                ref = _schema.get(u"$ref")
+                if ref is None:
+                    validators = iteritems(_schema)
+                else:
+                    validators = [(u"$ref", ref)]
+
+                for k, v in validators:
+                    validator = self.VALIDATORS.get(k)
+                    if validator is None:
+                        continue
+
+                    errors = validator(self, v, instance, _schema) or ()
+                    for error in errors:
+                        # set details if not already set by the called fn
+                        error._set(
+                            validator=k,
+                            validator_value=v,
+                            instance=instance,
+                            schema=_schema,
+                        )
+                        if k != u"$ref":
+                            error.schema_path.appendleft(k)
+                        yield error
+            finally:
+                if scope:
+                    self.resolver.pop_scope()
 
         def descend(self, instance, schema, path=None, schema_path=None):
             for error in self.iter_errors(instance, schema):
@@ -250,7 +249,7 @@ def __init__(
         self.handlers = dict(handlers)
 
 
-        self.old_scopes = []
+        self.scopes_stack = []
         self.store = _utils.URIDict(
             (id, validator.META_SCHEMA)         ## IDs assumed pure urls (no fragments).
             for id, validator in iteritems(meta_schemas)
@@ -272,7 +271,7 @@ def from_schema(cls, schema, *args, **kwargs):
 
     def push_scope(self, scope, is_defragged=False):
         old_scope = self.resolution_scope
-        self.old_scopes.append(old_scope)
+        self.scopes_stack.append(old_scope)
         if not is_defragged:
             scope = urldefrag(scope)
         self.resolution_scope = DefragResult(
@@ -282,7 +281,7 @@ def push_scope(self, scope, is_defragged=False):
         )
 
     def pop_scope(self):
-        self.resolution_scope = self.old_scopes.pop()
+        self.resolution_scope = self.scopes_stack.pop()
 
     @contextlib.contextmanager
     def resolving(self, ref):
@@ -309,11 +308,11 @@ def resolving(self, ref):
 
         uri = DefragResult(url, ref.fragment)
         old_base_uri, self.base_uri = self.base_uri, uri
+        self.push_scope(uri, is_defragged=True)
         try:
-            self.push_scope(uri, is_defragged=True)
             yield self.resolve_fragment(document, ref.fragment)
-            self.pop_scope()
         finally:
+            self.pop_scope()
             self.base_uri = old_base_uri
 
     def resolve_fragment(self, document, fragment):