Exceptions in middleware loading fail silently

[James Aylett]

If the loading of a piece of middleware fails (either due to failure of the import of the middleware module, or an ImproperlyConfigured exception raised by Django itself), the exception will be pushed up to the handler (wsgi, modpython or whatever), which will generally speaking interpret it as an exception from within *request processing*, rather than framework initialisation. However the handlers simply check that self._response_middleware is not None to decide whether to load middleware, which it will no longer be. So on the *next* request, middleware loading will not be performed, and the request will continue as normal -- but without the failing middleware or any middleware further down the list having been loaded or registered. It's not difficult to imagine security issues coming out of this, although they should only bite in the absence of rigorous testing and review; however it can also cause subtle bugs when dealing with complex middleware which cause considerable pain in development.

One solution would be to ensure that on an exception being raised from within core.handlers.base.BaseHandler.load_middleware() self._response_middleware is set back to None, and the exception logged usefully. This would allow reloading to work.

I've attached a patch which is more aggressive than this: it bombs out (using os._exit) having printed the stack trace. In a development situation this probably doesn't make a huge amount of difference, but it may trigger more obvious warnings in a deployed system. Changing this patch to set self._response_middleware to None would be trivial in any case.

[James Aylett]

Patch to bomb out on exception during middleware loading

[Jacob]

The attached patch isn't the correct approach at all -- there's almost never a good reason to use os.exit, and never a good reason to use os._exist -- but this is indeed a bug.

[Jacob]

Actually, upon further investigation, this bug report is incorrect. The bug report states that "if the loading of a piece of middleware fails ... the exception will be pushed up to the handler [which will] simply check that self._response_middleware is not None .. which it will no longer be.

However, looking at the code, that's not true; load_middleware() essentially does:

        self._request_middleware = None
        request_middleware = []

        for middleware_path in settings.MIDDLEWARE_CLASSES:
            # ... load the middleware in middleware_path

            if hasattr(mw_instance, 'process_request'):
                request_middleware.append(mw_instance.process_request)

        self._request_middleware = request_middleware

So if the loading raises an exception the loop will never complete and self._request_middleware will remain None.

[James Aylett]

The original report wasn't invalid, because that code only went into place in [10036]. However on visual inspection it looks like this bug is no longer be present in trunk, and hence will not be present in the 1.1 release.

[Jacob]

Milestone 1.1 deleted