Make QuotaImageTagsProxy deep-copyable

Infinite recursion is possible if you try to deepcopy a
QuotaImageTagsProxy object, which is something that happens when using
oslo.policy 1.43.1 and beyond.

By adding a __gettattribute__ within the __getattr__ method we can break
the loop when the name of the attribute is 'tags'.

The added test will fail (with infinite recursion) if the new code is
removed.

For completeness a test is added checking what happens when the tags
attribute is removed.

The thing that is not immediately clear from the __getattr__ code is
that it is not for getting at the tags themselves, it is for getting
at the methods on the set object at self.tags. The non-obviousness
of this led to rather a lot of confusion while trying to get this
code right. It isn't really "right" now, but rather: safe in the
context of its use.

Change-Id: Ic47c9bf8e9b97de5a5a49a35f631753c54e0e2af
Related-Bug: #1808063
This commit is contained in:
Chris Dent 2018-12-13 20:56:39 +00:00
parent 044d6b45f9
commit deffb09871
2 changed files with 25 additions and 14 deletions

View File

@ -165,14 +165,15 @@ class QuotaImageTagsProxy(object):
return self.tags.__len__(*args, **kwargs)
def __getattr__(self, name):
# Use TypeError here, not AttributeError, as the latter is how we
# know a tag is not present. TypeError says "this object is not
# what it claims to be".
try:
tags = self.__getattribute__('tags')
except AttributeError:
raise TypeError('QuotaImageTagsProxy has no tags.')
return getattr(tags, name)
# Protect against deepcopy, which calls getattr. __getattr__
# is only called when an attribute is not "normal", so when
# self.tags is called, this is not.
if name == 'tags':
try:
return self.__getattribute__('tags')
except AttributeError:
return None
return getattr(self.tags, name)
class ImageMemberFactoryProxy(glance.domain.proxy.ImageMembershipFactory):

View File

@ -12,6 +12,7 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import copy
import uuid
import mock
@ -596,15 +597,24 @@ class TestQuotaImageTagsProxy(test_utils.BaseTestCase):
items.remove(item)
self.assertEqual(0, len(items))
def test_tags_attr_exception(self):
def test_tags_attr_no_loop(self):
proxy = glance.quota.QuotaImageTagsProxy(None)
self.assertRaises(AttributeError, lambda: proxy.foo)
self.assertEqual(set([]), proxy.tags)
# Remove tags to cause the object to be broken. If tags
# is not there and we weren't raising TypeError, we'd
# get an infinite loop when calling 'proxy.foo'.
def test_tags_deepcopy(self):
proxy = glance.quota.QuotaImageTagsProxy(set(['a', 'b']))
proxy_copy = copy.deepcopy(proxy)
self.assertEqual(set(['a', 'b']), proxy_copy.tags)
self.assertIn('a', proxy_copy)
# remove is a found via __getattr__
proxy_copy.remove('a')
self.assertNotIn('a', proxy_copy)
def test_tags_delete(self):
proxy = glance.quota.QuotaImageTagsProxy(set(['a', 'b']))
self.assertEqual(set(['a', 'b']), proxy.tags)
del proxy.tags
self.assertRaises(TypeError, lambda: proxy.foo)
self.assertIsNone(proxy.tags)
class TestImageMemberQuotas(test_utils.BaseTestCase):