admin/docs/filters|tags __file__ attribute errors for egg extensions

[Welborn Productions]

Accessing hostname.com/admin/doc/filters and hostname.com/admin/doc/tags causes an Internal Server Error on Django 1.7. The cause is modules that have no __file__ attribute.

In django/contrib/admindocs/views.py, the function load_all_installed_template_libraries() already gracefully fails on OSError when finding python files. However, when the module being checked has no __file__ attribute the error bubbles up and causes an Internal Server Error.

Someone on django-users suggested that it may be because some extensions are installed as eggs. I've attached a naive patch that simply adds AttributeError to the caught exceptions, causing the function to fail gracefully instead of letting it bubble up.

The code that triggers the error:

try:
    libraries = [
        os.path.splitext(p)[0]
        for p in os.listdir(os.path.dirname(upath(mod.__file__)))
        if p.endswith('.py') and p[0].isalpha()
    ]
except OSError:
    libraries = []

I've simply added another error to that block:

try:
   # ...same code from above.
except (OSError, AttributeError):
    libraries = []

I thought about refactoring this and maybe expanding the for-loop so AttributeError is only caught where needed, but chose instead to make the least invasive change possible.

[Welborn Productions]

Fixed patch for file AttributeError (naive fix).

[Aymeric Augustin]

Django is attempting to find all possible modules, even those that aren't imported yet, which is technically incompatible with Python's import machinery (loaders and importers). ​pkgutil.iter_modules won't help. So there won't be a perfect solution.

That said, I would find it better to split the try/except in two parts:

1. attempt to compute os.path.dirname(upath(mod.__file__)), continue if that fails,
2. attempt to list files within that directory, continue if that fails,

[Welborn Productions]

Newer patch with suggested changes.

[Welborn Productions]

Here is a new patch with the suggested changes. It catches AttributeError when calculating the directory, if it fails then continue is used.

On success it will try to list the files in that directory.

I used an else on the this try, so the libraries list is used only when it is successfully initialized. The old method to skip/continue on OSError was to set libraries to an empty list which would then be skipped automatically with the for loop. The new method uses an actual continue.

[Tim Graham]

Is it possible to add tests for this?

[Welborn Productions]

To be honest I'm not sure how I would test for this. The function has no arguments. It depends on template.get_templatetags_modules() to gather module names, import them, determine the directory for each module, and list the directory contents. How could I introduce a module with no __file__ to test with?

This patch doesn't change the way template libraries are loaded. It just lets it fail with a little more grace. If anyone knows how I might go about testing this I would be glad to put in the work.

[Tim Graham]

That's what I thought regarding tests.

After looking at your patch, I would take a slightly different approach; please see the attached patch. If it looks fine (maybe you'd like to test it), I'll go ahead and commit it.

[Welborn Productions]

@timgraham, sorry it has taken me so long to reply. Your approach works fine. I tested it under my setup and it allowed me to use the filters documentation perfectly. I would love to see this fix included in Django.

[Tim Graham <timograham@…>]

In 01ab84c61330ffa5ac87c637249611c5e5343e57:

Fixed #23525 -- Fixed admindocs crash on apps installed as eggs.

Thanks welbornprod for report and initial patch.

[Tim Graham <timograham@…>]

In ac098867c004b7336aac30d0689e70fbd73a7306:

[1.7.x] Fixed #23525 -- Fixed admindocs crash on apps installed as eggs.

Thanks welbornprod for report and initial patch.

Backport of 01ab84c61330ffa5ac87c637249611c5e5343e57 from master