Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Lib/rlcompleter.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,14 @@ def attr_matches(self, text):
if (word[:n] == attr and
not (noprefix and word[:n+1] == noprefix)):
match = "%s.%s" % (expr, word)
if isinstance(getattr(type(thisobject), word, None),
property):
# bpo-44752: thisobject.word is a method decorated by
# `@property`. What follows applies a postfix if
# thisobject.word is callable, but know we know that
# this is not callable (because it is a property).
# Also, getattr(thisobject, word) will evaluate the
# property method, which is not desirable.

class_attr = getattr(type(thisobject), word, None)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also run arbitrary code because the descriptor protocol also gets run on access through the class. To be fully safe we should probably use something like inspect.getattr_static. Still, your change is a strict improvement so I'm OK with landing it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I experimented with this but it breaks another test create_expected_for_none and changes behavior elsewhere. Upstream behavior and this PR:

>>> None.__class__<TAB>
None.__class__()

Changing this line to inspect.getattr_static(type(thisobject), word, None):

>>> None.__class__<TAB>
None.__class__

I could argue this either way, but I think I will merge this as-is as it doesn't change any existing tested behavior.

if isinstance(
class_attr,
(property, types.GetSetDescriptorType, types.MemberDescriptorType)
) or (hasattr(class_attr, '__get__') and not callable(class_attr)):
# Avoid evaluating descriptors, which could run
# arbitrary code or raise exceptions.
matches.append(match)
continue

Expand Down
52 changes: 52 additions & 0 deletions Lib/test/test_rlcompleter.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import unittest
from unittest.mock import patch
import builtins
import types
import rlcompleter
from test.support import MISSING_C_DOCSTRINGS

Expand Down Expand Up @@ -135,6 +136,57 @@ def bar(self):
self.assertEqual(completer.complete('f.b', 0), 'f.bar')
self.assertFalse(f.property_called)

def test_released_memoryview_completion_works(self):
mv = memoryview(b"abc")
mv.release()

self.assertIsInstance(type(mv).shape, types.GetSetDescriptorType)
self.assertIsInstance(type(mv).strides, types.GetSetDescriptorType)

completer = rlcompleter.Completer(dict(mv=mv))
matches = completer.attr_matches('mv.')

# These are getset descriptors on memoryview and should be completed
# without evaluating the released-memoryview getters.
self.assertIn('mv.shape', matches)
self.assertIn('mv.strides', matches)

def test_member_descriptor_not_evaluated(self):
class Foo:
__slots__ = ("boom",)
boom_accesses = 0

def __getattribute__(self, name):
if name == "boom":
type(self).boom_accesses += 1
raise RuntimeError("boom access should be skipped")
return super().__getattribute__(name)

self.assertIsInstance(Foo.boom, types.MemberDescriptorType)

completer = rlcompleter.Completer(dict(f=Foo()))
matches = completer.attr_matches('f.')
self.assertIn('f.boom', matches)
self.assertEqual(Foo.boom_accesses, 0)

def test_raising_descriptor_completion_works(self):
class ExplodingDescriptor:
def __init__(self):
self.instance_get_calls = 0

def __get__(self, obj, owner):
if obj is None:
return self
self.instance_get_calls += 1
raise RuntimeError("descriptor getter exploded")

class Foo:
boom = ExplodingDescriptor()

completer = rlcompleter.Completer(dict(f=Foo()))
matches = completer.attr_matches('f.')
self.assertIn('f.boom', matches)
self.assertEqual(Foo.boom.instance_get_calls, 0)

def test_uncreated_attr(self):
# Attributes like properties and slots should be completed even when
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
In the REPL, autocompletion might run arbitrary code in the getter of a
descriptor. If that getter raised an exception, autocompletion would fail to
present any options for the entire object. Autocompletion now works as
expected for these objects.
Loading