Exceptions in middleware loading fail silently
|Reported by:||jaylett||Owned by:||nobody|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||yes|
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.
Change History (6)
Changed 5 years ago by jaylett
comment:1 Changed 5 years ago by jaylett
- Cc james@… added
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
comment:2 Changed 5 years ago by jacob
- milestone set to 1.1
- Patch needs improvement set
- Triage Stage changed from Unreviewed to Accepted