decorator_from_middleware can cause middleware hooks to run out of correct order.
|Reported by:||jdunck||Owned by:|
|Severity:||Normal||Keywords:||cache middleware decorator gzip conditional http|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||yes|
cache_page, gzip_page, and conditional_page all use decorator_from_middleware.
decorator_from_middleware wraps the view function to simulate middleware being run on a per-view basis.
Here's the interesting bit:
def _wrapped_view(request, *args, **kwargs): if hasattr(middleware, 'process_request'): result = middleware.process_request(request) if result is not None: return result if hasattr(middleware, 'process_view'): result = middleware.process_view(request, view_func, *args, **kwargs) if result is not None: return result try: response = view_func(request, *args, **kwargs) except Exception, e: if hasattr(middleware, 'process_exception'): result = middleware.process_exception(request, e) if result is not None: return result raise if hasattr(middleware, 'process_response'): result = middleware.process_response(request, response) if result is not None: return result return response
The problem is that middleware's order is important. Suppose two middleware decorators, A and B, are applied to the same view.
The normal flow of middleware processing would be: A.process_request, B.process_request, A.process_view, B.process_view, view, B.process_response, A.process_response.
The flow of the *decorated execution* is this: A.process_request, A.process_view, B.process_request, B.process_view, view, B.process_response, A.process_response.
This is not yet a real-world issue as far as I know, but seems to be a lurking problem. I think the only reason it hasn't surfaced is that view decorators are fairly uncommon. I don't see a simple way to fix this.
Change History (12)
comment:3 Changed 7 years ago by thejaswi_puthraya
- Component changed from Uncategorized to HTTP handling
comment:7 Changed 4 years ago by hirokiky
- Cc hirokiky@… added
- Has patch set
- Needs documentation set
- Needs tests set
comment:8 Changed 4 years ago by hirokiky
- Owner changed from nobody to hirokiky
- Status changed from new to assigned
comment:10 follow-up: ↓ 12 Changed 2 years ago by hirokiky
- Has patch unset
- Owner hirokiky deleted
- Status changed from assigned to new