Skip to content

Commit bdf6f1b

Browse files
committed
issue #590: rework ParentEnumerationMethod to recursively handle bad modules
In the worst case it will start with sys.path and resolve everything from scratch.
1 parent 4caca80 commit bdf6f1b

File tree

3 files changed

+140
-42
lines changed

3 files changed

+140
-42
lines changed

mitogen/master.py

Lines changed: 95 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -535,18 +535,20 @@ def find(self, fullname):
535535
Find `fullname` using its :data:`__file__` attribute.
536536
"""
537537
module = sys.modules.get(fullname)
538-
LOG.debug('_get_module_via_sys_modules(%r) -> %r', fullname, module)
539-
if getattr(module, '__name__', None) != fullname:
540-
LOG.debug('sys.modules[%r].__name__ does not match %r, assuming '
541-
'this is a hacky module alias and ignoring it',
542-
fullname, fullname)
543-
return
544-
545538
if not isinstance(module, types.ModuleType):
546539
LOG.debug('%r: sys.modules[%r] absent or not a regular module',
547540
self, fullname)
548541
return
549542

543+
LOG.debug('_get_module_via_sys_modules(%r) -> %r', fullname, module)
544+
alleged_name = getattr(module, '__name__', None)
545+
if alleged_name != fullname:
546+
LOG.debug('sys.modules[%r].__name__ is incorrect, assuming '
547+
'this is a hacky module alias and ignoring it. '
548+
'Got %r, module object: %r',
549+
fullname, alleged_name, module)
550+
return
551+
550552
path = _py_filename(getattr(module, '__file__', ''))
551553
if not path:
552554
return
@@ -573,43 +575,57 @@ def find(self, fullname):
573575
class ParentEnumerationMethod(FinderMethod):
574576
"""
575577
Attempt to fetch source code by examining the module's (hopefully less
576-
insane) parent package. Required for older versions of
577-
:mod:`ansible.compat.six`, :mod:`plumbum.colors`, and Ansible 2.8
578-
:mod:`ansible.module_utils.distro`.
578+
insane) parent package, and if no insane parents exist, simply use
579+
:mod:`sys.path` to search for it from scratch on the filesystem using the
580+
normal Python lookup mechanism.
581+
582+
This is required for older versions of :mod:`ansible.compat.six`,
583+
:mod:`plumbum.colors`, Ansible 2.8 :mod:`ansible.module_utils.distro` and
584+
its submodule :mod:`ansible.module_utils.distro._distro`.
585+
586+
When some package dynamically replaces itself in :data:`sys.modules`, but
587+
only conditionally according to some program logic, it is possible that
588+
children may attempt to load modules and subpackages from it that can no
589+
longer be resolved by examining a (corrupted) parent.
579590
580591
For cases like :mod:`ansible.module_utils.distro`, this must handle cases
581592
where a package transmuted itself into a totally unrelated module during
582-
import and vice versa.
593+
import and vice versa, where :data:`sys.modules` is replaced with junk that
594+
makes it impossible to discover the loaded module using the in-memory
595+
module object or any parent package's :data:`__path__`, since they have all
596+
been overwritten. Some men just want to watch the world burn.
583597
"""
584-
def find(self, fullname):
598+
def _find_sane_parent(self, fullname):
585599
"""
586-
See implementation for a description of how this works.
600+
Iteratively search :data:`sys.modules` for the least indirect parent of
601+
`fullname` that is loaded and contains a :data:`__path__` attribute.
602+
603+
:return:
604+
`(parent_name, path, modpath)` tuple, where:
605+
606+
* `modname`: canonical name of the found package, or the empty
607+
string if none is found.
608+
* `search_path`: :data:`__path__` attribute of the least
609+
indirect parent found, or :data:`None` if no indirect parent
610+
was found.
611+
* `modpath`: list of module name components leading from `path`
612+
to the target module.
587613
"""
588-
if fullname not in sys.modules:
589-
# Don't attempt this unless a module really exists in sys.modules,
590-
# else we could return junk.
591-
return
592-
593-
pkgname, _, modname = str_rpartition(to_text(fullname), u'.')
594-
pkg = sys.modules.get(pkgname)
595-
if pkg is None or not hasattr(pkg, '__file__'):
596-
LOG.debug('%r: %r is not a package or lacks __file__ attribute',
597-
self, pkgname)
598-
return
599-
600-
pkg_path = [os.path.dirname(pkg.__file__)]
601-
try:
602-
fp, path, (suffix, _, kind) = imp.find_module(modname, pkg_path)
603-
except ImportError:
604-
e = sys.exc_info()[1]
605-
LOG.debug('%r: imp.find_module(%r, %r) -> %s',
606-
self, modname, [pkg_path], e)
607-
return None
608-
609-
if kind == imp.PKG_DIRECTORY:
610-
return self._found_package(fullname, path)
611-
else:
612-
return self._found_module(fullname, path, fp)
614+
path = None
615+
modpath = []
616+
while True:
617+
pkgname, _, modname = str_rpartition(to_text(fullname), u'.')
618+
modpath.insert(0, modname)
619+
if not pkgname:
620+
return [], None, modpath
621+
622+
pkg = sys.modules.get(pkgname)
623+
path = getattr(pkg, '__path__', None)
624+
if pkg and path:
625+
return pkgname.split('.'), path, modpath
626+
627+
LOG.debug('%r: %r lacks __path__ attribute', self, pkgname)
628+
fullname = pkgname
613629

614630
def _found_package(self, fullname, path):
615631
path = os.path.join(path, '__init__.py')
@@ -638,6 +654,47 @@ def _found_module(self, fullname, path, fp, is_pkg=False):
638654
source = source.encode('utf-8')
639655
return path, source, is_pkg
640656

657+
def _find_one_component(self, modname, search_path):
658+
try:
659+
#fp, path, (suffix, _, kind) = imp.find_module(modname, search_path)
660+
return imp.find_module(modname, search_path)
661+
except ImportError:
662+
e = sys.exc_info()[1]
663+
LOG.debug('%r: imp.find_module(%r, %r) -> %s',
664+
self, modname, [search_path], e)
665+
return None
666+
667+
def find(self, fullname):
668+
"""
669+
See implementation for a description of how this works.
670+
"""
671+
#if fullname not in sys.modules:
672+
# Don't attempt this unless a module really exists in sys.modules,
673+
# else we could return junk.
674+
#return
675+
676+
fullname = to_text(fullname)
677+
modname, search_path, modpath = self._find_sane_parent(fullname)
678+
while True:
679+
tup = self._find_one_component(modpath.pop(0), search_path)
680+
if tup is None:
681+
return None
682+
683+
fp, path, (suffix, _, kind) = tup
684+
if modpath:
685+
# Still more components to descent. Result must be a package
686+
if fp:
687+
fp.close()
688+
if kind != imp.PKG_DIRECTORY:
689+
LOG.debug('%r: %r appears to be child of non-package %r',
690+
self, fullname, path)
691+
return None
692+
search_path = [path]
693+
elif kind == imp.PKG_DIRECTORY:
694+
return self._found_package(fullname, path)
695+
else:
696+
return self._found_module(fullname, path, fp)
697+
641698

642699
class ModuleFinder(object):
643700
"""

tests/module_finder_test.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,52 @@ def test_ansible_module_utils_distro_succeeds(self):
180180
'pkg_like_ansible.module_utils.distro._distro'
181181
)
182182

183+
# ensure we can resolve the subpackage.
183184
path, src, is_pkg = self.call('pkg_like_ansible.module_utils.distro')
184185
modpath = os.path.join(MODS_DIR,
185186
'pkg_like_ansible/module_utils/distro/__init__.py')
186187
self.assertEquals(path, modpath)
187188
self.assertEquals(src, open(modpath, 'rb').read())
188189
self.assertEquals(is_pkg, True)
189190

191+
# ensure we can resolve a child of the subpackage.
192+
path, src, is_pkg = self.call(
193+
'pkg_like_ansible.module_utils.distro._distro'
194+
)
195+
modpath = os.path.join(MODS_DIR,
196+
'pkg_like_ansible/module_utils/distro/_distro.py')
197+
self.assertEquals(path, modpath)
198+
self.assertEquals(src, open(modpath, 'rb').read())
199+
self.assertEquals(is_pkg, False)
200+
201+
def test_ansible_module_utils_system_distro_succeeds(self):
202+
# #590: a package that turns itself into a module.
203+
# #590: a package that turns itself into a module.
204+
import pkg_like_ansible.module_utils.sys_distro as d
205+
self.assertEquals(d.I_AM, "the system module that replaced the subpackage")
206+
self.assertEquals(
207+
sys.modules['pkg_like_ansible.module_utils.sys_distro'].__name__,
208+
'system_distro'
209+
)
210+
211+
# ensure we can resolve the subpackage.
212+
path, src, is_pkg = self.call('pkg_like_ansible.module_utils.sys_distro')
213+
modpath = os.path.join(MODS_DIR,
214+
'pkg_like_ansible/module_utils/sys_distro/__init__.py')
215+
self.assertEquals(path, modpath)
216+
self.assertEquals(src, open(modpath, 'rb').read())
217+
self.assertEquals(is_pkg, True)
218+
219+
# ensure we can resolve a child of the subpackage.
220+
path, src, is_pkg = self.call(
221+
'pkg_like_ansible.module_utils.sys_distro._distro'
222+
)
223+
modpath = os.path.join(MODS_DIR,
224+
'pkg_like_ansible/module_utils/sys_distro/_distro.py')
225+
self.assertEquals(path, modpath)
226+
self.assertEquals(src, open(modpath, 'rb').read())
227+
self.assertEquals(is_pkg, False)
228+
190229

191230
class ResolveRelPathTest(testlib.TestCase):
192231
klass = mitogen.master.ModuleFinder

tests/responder_test.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,16 @@ def test_ansible_six_messed_up_path(self):
158158
self.assertEquals(1, len(router._async_route.mock_calls))
159159

160160
self.assertEquals(1, responder.get_module_count)
161-
self.assertEquals(0, responder.good_load_module_count)
162-
self.assertEquals(0, responder.good_load_module_size)
163-
self.assertEquals(1, responder.bad_load_module_count)
161+
self.assertEquals(1, responder.good_load_module_count)
162+
self.assertEquals(7642, responder.good_load_module_size)
163+
self.assertEquals(0, responder.bad_load_module_count)
164164

165165
call = router._async_route.mock_calls[0]
166166
msg, = call[1]
167167
self.assertEquals(mitogen.core.LOAD_MODULE, msg.handle)
168-
self.assertIsInstance(msg.unpickle(), tuple)
168+
169+
tup = msg.unpickle()
170+
self.assertIsInstance(tup, tuple)
169171

170172

171173
class ForwardTest(testlib.RouterMixin, testlib.TestCase):

0 commit comments

Comments
 (0)