Edgewall Software
Modify

Opened 11 years ago

Closed 10 years ago

Last modified 8 years ago

#7823 closed defect (fixed)

import obfuscation

Reported by: tac@… Owned by: osimons
Priority: normal Milestone: 0.12
Component: web frontend/mod_python Version: 0.11.2
Severity: normal Keywords:
Cc: kasper@…, osimons, Jonas Borgström Branch:
Release Notes:
API Changes:

Description

I use in my external app

from trac.admin.console import TracAdmin

In trac 0.11.2 i get the errors: IRequestHandler not found.

I solved this issue by fixing

  • trac/attachment.py
      - from trac.web import HTTPBadRequest, IRequestHandler
      + from trac.web.api import HTTPBadRequest, IRequestHandler
    
  • trac/wiki/intertrac.py
      - from trac.web import IRequestHandler
      + from trac.web.api import IRequestHandler
    

I am open for every solution - i won't touch trac source. But trac.admin.console is otherwise not usable for me.

Attachments (1)

t7823-genshi_modpython-r8333-011.diff (747 bytes ) - added by osimons 10 years ago.
Only enter the workaround code if Genshi is actually installed as a zipped egg.

Download all attachments as: .zip

Change History (29)

comment:1 by Remy Blank, 11 years ago

Component: admin/webadmin/console
Version: none0.11.2

Could you please post the whole traceback for the "IRequestHandler not found" error, and possibly the part of your application where the exception happens?

comment:2 by osimons, 11 years ago

Do you have mod_python installed and on Python path? I'm quite certain that is related to the current egg-trickery done in trac.web.__init__.py in order to handle zipped Genshi eggs through mod_python. I've seen that too, but according to jonas that code will be needed at least for a while still and it has seemed like the 'path of least problems'.

I suppose the faulty logic we both have hit here is that a) mod_python is installed b) we've arrived at trac.web outside regular request dispatching (trac.web.modpython_frontend never loaded). In that case the logic reads like the api classes will never actually get imported to their expected location in trac.web.

I don't have a solution to this as I haven't been involved in the workaround code. Hopefully it can be adjusted to accomodate more situations than just the plain request handling.

comment:3 by osimons, 11 years ago

Component: admin/consoleweb frontend/mod_python
Owner: set to Christopher Lenz

comment:4 by osimons, 11 years ago

Owner: Christopher Lenz removed

I thought we got rid of automatic component owners, but cmlenz was still on the mod_python component. Just been in Admin and removed him - please revert if there is some arrangement I'm unaware of.

comment:5 by kasper@…, 10 years ago

Cc: kasper@… added

I am experiencing this too. I am creating a Django-based website and have been using presenting some of Trac's data which I have been using the Trac API's to present (wiki format etc.) but this is an absolute show stopper :-(

comment:6 by osimons, 10 years ago

Cc: osimons added

Adding myself to cc list.

comment:7 by Christian Boos, 10 years ago

Milestone: 0.11.40.11.3

comment:8 by Christian Boos, 10 years ago

Well, even if I don't have the time to actually test this and find out the cause of the errors, changing the imports to be more specific (the way suggested in the description above) seems fine by me.

comment:9 by osimons, 10 years ago

Cc: Jonas Borgström added

I don't agree. There are countless lines of from trac.web import ... in Trac source and plugins. The imports need to work as intended. And also, the problem arises from a limited-time workaround - jonas says it stays until a new Genshi version is released that fixes egg-extract issues.

I think my description of the actual problem above is correct: from trac.web.api import * will never get evaluated if first trac.web code access is in a mod_python context (ie. no import exception), but not via the regular modpython_frontend code (ie. fails the if test of module being loaded in sys.modules).

comment:10 by Christian Boos, 10 years ago

Milestone: 0.11.30.11.4

I myself tend to write the imports by always specifying the full path to the actual module defining the symbol, that's why I was OK to change them the way suggested. As you pointed out however, there is actually a bunch of from trac.web import... throughout the code, so perhaps it's not worth changing only a few of them. So I leave that ticket to someone else.

comment:11 by osimons, 10 years ago

Milestone: 0.12.10.12
Owner: set to osimons

Did some digging, and found genshi:ticket:252 with genshi:changeset:928 being applied in trunk almost 1 year ago, and ported to 0.5.x stable as part of genshi:changeset:947 for 0.5.2dev.

That means:

  • For trunk, this code workaround can be removed based on current requirement on 'Genshi>=0.6dev-r960'.
  • For 0.11-stable, we still just require 'Genshi>=0.5', and that makes it look unlikely that it will be fixed soon - we would need a new Genshi 0.5.2 release, and even with that I'm not sure what the policy would be to change Trac dependencies in a stable branch.

I'll remove it for trunk at least - but would really like a way to drop this code for 0.11-stable as well… This code is really distracting for any kind of integrated use of Trac code, and at least it should only fire if Genshi is actually installed as an egg (personally I never install zipped eggs).

How about something like this (untested):

from pkg_resources import get_provider, ZipProvider
if isinstance(get_provider('genshi'), ZipProvider):
    # The current workaround code here...
else:
    from trac.web.api import *

See setuptools-pkgresources docs - at least my interactive testing of various test install modules seems to verify this logic.

by osimons, 10 years ago

Only enter the workaround code if Genshi is actually installed as a zipped egg.

comment:12 by osimons, 10 years ago

Priority: lownormal

Tried it, and works like a charm :-) With this patch, ensuring that Genshi is installed unzipped will mean no worries regardless. As this code is unlikely to disappear from 0.11-stable, at least this means that there is a well-working workaround for the workaround…

Please, please: 'OK' to apply to 0.11-stable (and remove workaround completely from trunk)?

comment:13 by Christian Boos, 10 years ago

I'm not sure I understand what the workaround (r7439) is supposed to do.

I just tried to install Genshi-0.5.1 as a zipped egg, and I get the following error when accessing any page:

  File ".../mod_python/importer.py", line 1537, in HandlerDispatch\n    default=default_handler, arg=req, silent=hlist.silent)
  File ".../mod_python/importer.py", line 1202, in _process_target\n    module = import_module(module_name, path=path)
  File ".../mod_python/importer.py", line 304, in import_module\n    return __import__(module_name, {}, {}, ['*'])
ImportError: No module named trac.web.modpython_frontend

Regardless whether the patch (well, now r8335) was applied or not. However this was with SetEnv PYTHON_EGG_CACHE ... not PythonOption.

Retrying with PythonOption PYTHON_EGG_CACHE ..., I get a different error:

  File ".../Trac-0.11.5stable_r8333-py2.4.egg/trac/web/modpython_frontend.py", line 148, in handler\n    from trac.web.main import dispatch_request
  File ".../Trac-0.11.5stable_r8333-py2.4.egg/trac/web/main.py", line 49, in ?\n    from trac.web.chrome import Chrome
  File ".../Trac-0.11.5stable_r8333-py2.4.egg/trac/web/chrome.py", line 53, in ?\n    from trac.wiki import IWikiSyntaxProvider
  File ".../Trac-0.11.5stable_r8333-py2.4.egg/trac/wiki/__init__.py", line 3, in ?\n    from trac.wiki.intertrac import *
  File ".../Trac-0.11.5stable_r8333-py2.4.egg/trac/wiki/intertrac.py", line 26, in ?\n    from trac.web import IRequestHandler
ImportError: cannot import name IRequestHandler

In the log file, I also see:

  File ".../mod_python/importer.py", line 1537, in HandlerDispatch\n    default=default_handler, arg=req, silent=hlist.silent)
  File ".../mod_python/importer.py", line 1229, in _process_target\n    result = _execute_target(config, req, object, arg)
  File ".../mod_python/importer.py", line 1128, in _execute_target\n    result = object(arg)
  File ".../Trac-0.11.5stable_r8333-py2.4.egg/trac/web/modpython_frontend.py", line 142, in handler\n    pkg_resources.set_extraction_path(egg_cache)
  File ".../setuptools-0.7a1dev_r66608-py2.4.egg/pkg_resources.py", line 1005, in set_extraction_path\n    raise ValueError(
ValueError: Can't change extraction path, files already extracted

Maybe that explains the issue.

I'm using mod_python 3.3.1, the configuration is:

<Location /trac>
  SetHandler mod_python
  PythonHandler trac.web.modpython_frontend
  PythonOption TracEnvParentDir /srv/trac
  PythonOption TracUriRoot /trac
##  SetEnv PYTHON_EGG_CACHE /var/lib/nobody
  PythonOption PYTHON_EGG_CACHE /var/lib/nobody
</Location>

Is there anything I missed?

comment:14 by osimons, 10 years ago

Jonas can no doubt correct me, but the linked genshi changesets is supposed to ensure that with compiled speedups it should always install Genshi unzipped. If not, a chicken & egg problem arises due to the fact that it needs to unzip before the trac.web.mod_python.handler function is hit where the cache is actually set (the from trac.web.api import * is the start of a long chain of module loading, including genshi).

This workaround is messy, and I've hit the issue over and over again for a long time regardless of how Genshi was installed - to the extent that I have been patching every Trac install I do… The purpose of latest patch is just to contain the code to the circumstance where Genshi is actually installed as an egg. However, a different matter (as you also found out) is if the import workaround code actually works according to purpose…

comment:15 by osimons, 10 years ago

Actually, the load order is perhaps uncertain - and it sort of 'depends'. Just to elaborate, the problem that I always have been getting (before now) is of the ImportError: cannot import name IRequestHandler kind.

The reason is that my 'coderesort' plugin among other things contain a custom authentication handler built using various parts of the Trac codebase. As 98% of my service is behind authentication-required paths, the first request will hit the authentication handler before it hits the regular request handler. So whenever a periodic restart happens, the server would only come up again if the first request did not require authentication… As mentioned by others, any other such tight coupling with Trac would also fail in similar fashion if used in any other way than through the regular Trac mod_python handler (as from trac.web.api import * would not get evaluated).

comment:16 by osimons, 10 years ago

Just some testing as follow-up: For any genshi 0.5.x egg built before revision 947, easy_install of the egg will just install the zipped egg. As of 947 and later, the egg will always be extracted when installed if it containes compiled speedups. Try building + installing 946 and 947 and notice the difference. So, if 0.5.2 ever gets released and we bump Trac requirement to 0.5.2 (up from 0.5) this will be a non-issue for 0.11 and workaround code can be removed (and current requirement in trunk is already passed the 928 revision needed to work properly, and that's why I'll also remove it there).

Again, [8335] does not touch the workaround code itself - it just bypasses it when the egg isn't zipped.

comment:17 by osimons, 10 years ago

This apparently needs more work. A user came on IRC today, where things worked in 0.11.4 but failed now with 0.11.5rc1. I'm including the full error here:

Mod_python error: "PythonHandler trac.web.modpython_frontend"
Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/mod_python/apache.py", line 287, in HandlerDispatch
    log=debug)
  File "/usr/lib/python2.4/site-packages/mod_python/apache.py", line 464, in import_module
    module = imp.load_module(mname, f, p, d)
  File "/usr/lib/python2.4/site-packages/Trac-0.11.5rc1-py2.4.egg/trac/web/__init__.py", line 7, in ?
    if isinstance(get_provider('genshi'), ZipProvider):
  File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 119, in get_provider
    __import__(moduleOrReq)
  File "build/bdist.linux-i686/egg/genshi/__init__.py", line 32, in ?
  File "build/bdist.linux-i686/egg/genshi/core.py", line 544, in ?
  File "build/bdist.linux-i686/egg/genshi/_speedups.py", line 7, in ?
  File "build/bdist.linux-i686/egg/genshi/_speedups.py", line 4, in __bootstrap__
  File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 799, in resource_filename
    return get_provider(package_or_requirement).get_resource_filename(
  File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 1228, in get_resource_filename
    self._extract_resource(manager, self._eager_to_zip(name))
  File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 1249, in _extract_resource
    real_path = manager.get_cache_path(
  File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 880, in get_cache_path
    self.extraction_error()
  File "/usr/lib/python2.4/site-packages/pkg_resources.py", line 846, in extraction_error
    raise err
ExtractionError: Can't extract file(s) to egg cache
The following error occurred while trying to extract file(s) to the Python egg
cache:
  [Errno 13] Permission denied: '/var/www/.python-eggs/Genshi-0.5.1-py2.4-linux-i686.egg-tmp'
The Python egg cache directory is currently set to:
  /var/www/.python-eggs
Perhaps your account does not have write access to this directory?  You can
change the cache directory by setting the PYTHON_EGG_CACHE environment
variable to point to an accessible directory.

Other interesting artifacts:

  • Setuptools version was 0.6c3
  • PythonOption PYTHON_EGG_CACHE has no effect regardless of where in apache config it was set. mod_python still went with its own default even when set at config root outside virtual hosts (as our docs recommends).

Here is what I read into the traceback above: the test for isinstance(get_provider('genshi'), ZipProvider) actually triggers an extract of the egg. That was certainly not my intention. So, the test needs to be changed to some reliable method of detecting if the egg is zipped without triggering an extract. Good ideas are most welcome. I'll check with the Distutils-SIG first.

comment:18 by osimons, 10 years ago

Right, more testing with pkg_resources:

>>> import sys, os, pkg_resources
>>> pkg_resources.get_distribution('genshi')
Genshi 0.5.2dev-r0 (/path/to/Genshi-0.5.2dev_r0-py2.4-macosx-10.5-i386.egg)
>>> 'genshi' in sys.modules
False
>>> pkg_resources.get_provider('genshi')
<pkg_resources.ZipProvider instance at 0x68b20>
>>> 'genshi' in sys.modules
True

So, from that it seems querying a Distribution is a safer option.

I'd like to change the test in [8335] to:

if not os.path.isdir(pkg_resources.get_distribution('genshi').location):

comment:19 by osimons, 10 years ago

The patch:

  • trac/web/__init__.py

    a b  
    33# PYTHON_EGG_CACHE variable is set from there
    44#
    55# TODO: Remove this once the Genshi zip_safe issue has been resolved.
    6 from pkg_resources import get_provider, ZipProvider
    7 if isinstance(get_provider('genshi'), ZipProvider):
     6import os
     7from pkg_resources import get_distribution
     8if not os.path.isdir(get_distribution('genshi').location):
    89    try:
    910        import mod_python.apache
    1011        import sys

comment:20 by osimons, 10 years ago

Or, we can push cmlenz to release Genshi 0.5.2 and drop all the workaround code for 0.11.5…

in reply to:  19 ; comment:21 by dirk@…, 10 years ago

Replying to osimons:

The patch:

works for me. Many thanks!

in reply to:  21 comment:22 by osimons, 10 years ago

Replying to dirk@…:

works for me. Many thanks!

Good! Applied the fix in [8354].

comment:23 by Christian Boos, 10 years ago

What about trunk? From the commit comment of r8335, it seems that the workaround wouldn't be needed. Does that mean we can have a single line from trac.web.api import * in source:trunk/trac/web/__init__.py?

in reply to:  23 comment:24 by osimons, 10 years ago

Replying to cboos:

What about trunk? From the commit comment of r8335, it seems that the workaround wouldn't be needed. Does that mean we can have a single line from trac.web.api import * in source:trunk/trac/web/__init__.py?

Yes it does. I was going to get around to that as part of merging to trunk after 0.11.5 ([8335] and [8354]). However, I have a feeling it also implies being able to drop all the workaround code in modpython_frontend.py that sets egg_cache and reloads. I would like jonas to confirm that first. Jonas?

comment:25 by Christian Boos, 10 years ago

Owner: changed from osimons to Christian Boos

I'm going to implement the changes mentioned in the comment:24 above, we'll see during the testing period if this works in all situations.

comment:26 by Christian Boos, 10 years ago

Resolution: fixed
Status: newclosed

I finally merged the workaround to trunk r8809, learning from cmlenz that he intends to drop the _speedups C extension (which at this date doesn't speed up Genshi as much as it used to do).

So the workaround is still there, is still ugly, and we really should (one day) find a better solution, but for now it should just work.

If there are still issues with that, I'd suggest opening a new ticket.

comment:27 by Christian Boos, 10 years ago

Owner: changed from Christian Boos to osimons

comment:28 by steeni, 8 years ago

I had this same issue because I have mod_python for trac in /trac and for authentication in /svn. When I get a request to /svn, it extracts _mysql.so. Next time request comes (in same apache process) to /trac, it fails with internal server error because there are files extracted. Maybe this is also my configuration issue but it really did help to solve this by placing this in modpython_frontend.py.

    try:
        pkg_resources.set_extraction_path(egg_cache)
    except:
        req.log_error("Can't set extraction path. These files were extracted before: %s" % str(pkg_resources.cached_files.keys()))
        raise

Aslo, Maybe there could be if that checks pkg_resources.extraction_path != egg_cache and set it only when needed?

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain osimons.
The resolution will be deleted.
to The owner will be changed from osimons to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.