guarded_import does not support 5-args form

Bug #659968 reported by Vincent Pelletier
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zope 2
Invalid
Medium
Unassigned

Bug Description

Environment:
- python 2.6.5
- Zope 2.12.9

Symptom:
  TypeError: guarded_import() takes at most 4 arguments (5 given)

I triggered this problem indirectly, when calling xrange with a float argument.
This triggers this error, because xrange tries internally (disclaimer: I didn't check exactly what does the import) to import __doc__ from _warnings module using the 5-arguments form, and it uses ZopeGuards' version to do so, which fails with above error.

This bug report is just about guarded_import parameter count, but for completeness I must also say that, after adding a dummy fifth parameter to guarded_import, calling "xrange(0.1)" failed with
  Unauthorized: You are not allowed to access '__doc__' in this context

For the ones who are about to question the sanity of calling xrange with a float value:
- given error makes diagnostic non-straightforward (this is the most important point to me as I didn't intend to give it a float value)
- it does work in "pure" python (this is secondary to me, I won't use this - but I don't see a reason to prevent this from working)

Regards,
Vincent Pelletier

description: updated
Revision history for this message
Hanno Schlichting (hannosch) wrote :

I've seen the same problem.

I added the first workaround in r117566 in the 2.12 branch (and merged to the standalone AccessControl distribution). The guared_import function now accepts the level argument, but only supports level 0 at the moment. You get an explicit Unauthorized message if a different level is specified.

I couldn't find a good explanation of the semantics of the level argument, so I couldn't easily figure out how to support it in full.

Changed in zope2:
importance: Undecided → Medium
status: New → In Progress
Revision history for this message
Vincent Pelletier (vincent-nexedi) wrote :

Thanks.

Do you really want to use 0 here ? From python documentation[1], default level is -1.

I did a few tests to see how level affects __import__ behaviour, see attached tarball for the source, and below is the output I get here:

$ python2.5 test.py
absolute keyword: success urllib2 some_module.urllib2
relative keyword: success
relative built-in failed
absolute built-in, level -1 : success urllib2 some_module.urllib2
absolute built-in, level 0 : success urllib2
absolute built-in, level 1 : success urllib2 some_module.urllib2
absolute built-in, level 2 : failed

[1] http://docs.python.org/release/2.5/lib/built-in-funcs.html

Regards,
Vincent Pelletier

Revision history for this message
Vincent Pelletier (vincent-nexedi) wrote :

Woops, just noticed a mistake:
  relative keyword: success
This should be:
  relative keyword: failed

Cause: I forgot to "del urllib2" in the "else:" blocks using keyword, so urllib2 was still existing in some_module.

Revision history for this message
Vincent Pelletier (vincent-nexedi) wrote :

Sorry, I just found one more thing about your commit: don't you want to pass level parameter to
  return __import__(mname, globals, locals, fromlist)
also ?

Regards,
Vincent Pelletier

Revision history for this message
Hanno Schlichting (hannosch) wrote :

I fixed the default value to be -1 now. Thanks for noticing.

I don't understand enough about the semantics of the level argument and the way the code checks for allowed modules to change the code any further.

The only purpose of my change is to give a better error message for this case. This doesn't try to actually support the level argument in any way. I hope you might be able to come up with tests and a patch for this.

Revision history for this message
Vincent Pelletier (vincent-nexedi) wrote :

I started thinking about implementing this support, and here are my conclusion:
Basically, relative import means "relative to current package". So implementing "level" argument support means restricted_import would be expected to be called as part of a package.
I there some sort of "Restricted Package" ?
If there is such a thing, then it would make sense to implement full support (and this would probably provide with exampled).
Otherwise, I guess it's ok the way it is now, and this bug could be closed as resolved.

And I should probably open another bug for the specific xrange case I mentioned.

Revision history for this message
Maurits van Rees (maurits-vanrees) wrote :

I see the same, so I will add some notes here. I run into this when I am trying to trigger an error by writing some tests for the old unicodeTestIn python script in Products.Archetypes. This is with the current plone-coredev/4.0 which uses a checkout of AccessControl from the 2.12. branch of Zope2.

It can be reproduced with a Script (Python) with this admittedly silly line:

    return u'\xeb' in [u'\xeb'.encode('utf-8')] and 'yes' or 'no'

This raises an Unauthorized exception:

     Module AccessControl.ZopeGuards, line 283, in guarded_import
    Unauthorized: Using import with a level specification isn't supported by AccessControl: _warnings

When I comment out the 'if level != -1:' test in AccessControl that would raise the above exception, the hidden problem in the silly line becomes visible as the code wants to show this warning:

    UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal

I would very much prefer to see this last UnicodeWarning as it helps me to see the problem in my code, instead of the cryptic Unauthorized error.

From the python 2.6 docstring for __import__ it does not look dangerous to allow level=0 as argument as that simply means doing an absolute import:

    Level is used to determine whether to perform
    absolute or relative imports. -1 is the original strategy of attempting
    both absolute and relative imports, 0 is absolute, a positive number
    is the number of parent directories to search relative to the current module.

But AccessControl has some scary code so I can very much imagine that you are hesitant to change the code any further. Feels like opening Pandora's box... Still, allowing level=0 looks safe to me; in our case it seems to have the same effect as level=-1 because relative imports probably do not have much meaning in a RestrictedPython context. With a pdb at the end of the guarded_import method, this is what I see:

    (Pdb) __import__('_warnings', globals, locals, fromlist, level=-1)
    <module '_warnings' (built-in)>
    (Pdb) __import__('_warnings', globals, locals, fromlist, level=0)
    <module '_warnings' (built-in)>
    (Pdb) __import__('_warnings', globals, locals, fromlist, level=1)
    *** ValueError: Attempted relative import in non-package

So I suggest changing the added test to 'if level > 0: raise Unauthorized(...)'.

For completeness sake, if I try that same silly Script (Python) on Plone 3.3.5 (python 2.4, Zope 2.10) I get the more familiar
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 0: ordinal not in range(128)

Changed in zope2:
status: In Progress → Triaged
Revision history for this message
Colin Watson (cjwatson) wrote :

The zope2 project on Launchpad has been archived at the request of the Zope developers (see https://answers.launchpad.net/launchpad/+question/683589 and https://answers.launchpad.net/launchpad/+question/685285). If this bug is still relevant, please refile it at https://github.com/zopefoundation/zope2.

Changed in zope2:
status: Triaged → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.