Opened 18 years ago

Closed 17 years ago

Last modified 12 years ago

#749 closed defect (duplicate)

Middleware should work as a 'stack' or allow custom orderings

Reported by: L.Plant.98@… Owned by: nobody
Component: Core (Other) Version:
Severity: normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This ticket is the summary/result of a discussion on django-developers, especially near the end of this thread:

http://groups.google.com/group/django-developers/browse_thread/thread/2865c3f1e8780bf6/999616351a520e67?lnk=arm#999616351a520e67

The current behaviour of a set of middleware is that they are used in the order in which they are listed for process_request and process_view, and in reverse order for process_exception and process_response.

If one middleware returns an HttpResponse in process_request, then the remaining ones in the list are skipped as far as process_request is concerned, but they all still contribute to process_response. Instead, they should be skipped.

The current behaviour makes the CacheMiddleware buggy. Consider this stack

MIDDLEWARE_CLASSES = (
    "django.middleware.cache.CacheMiddleware",
    "somemodule.SomePostProcessingMiddleware",
    "django.middleware.session.SessionMiddleware",
)

The first time a page is accessed, it will go through all middleware both 'down' (process_request) and 'up' (process_response), and the page is cached after it has been post-processed by SomePostProcessingMiddleware. The second time we have a cache hit, and the CacheMiddleware returns the response. However, that response still goes through all the process_response middleware, so SomePostProcessingMiddleware is applied twice, which is bad. In theory this could be bad for any of the middleware (adding headers twice etc), it just depends what they do.

If CacheMiddleware returns a response, the ones beneath it should be skipped for process_response.

There are alternative fixes, including:
1) allow different ordering for request and response middleware to specified
2) change the CacheMiddleware so that it creates a deep copy of the cached response, attaches it to the request object, and then in process_response ignores the passed in response object, and returns the one it stashed earlier.
3) split CacheMiddleware into two, to allow different ordering

I think the proposed solution is better than these, but needs to be thought about carefully, especially the effect it might have on other middleware functionality, and what exactly should happen with process_view. One idea I had for process_view was that it be made to work equivalently to process_response (i.e. it must return a view function, instead of optionally returning an HttpResponse object). But I don't know where/how/why process_view is currently used.

I think it's quite important to get this right/standardised (or at least documented) before version 1.0.

Change History (7)

comment:1 by (none), 17 years ago

milestone: Version 1.0

Milestone Version 1.0 deleted

comment:2 by Simon G. <dev@…>, 17 years ago

Triage Stage: UnreviewedDesign decision needed

c.f. also #730

comment:3 by anonymous, 17 years ago

Cc: achambers.home@… added

comment:4 by anonymous, 17 years ago

Cc: achambers.home@… removed

comment:5 by Luke Plant, 17 years ago

Another idea: allow middleware to add additional arbitrary information to the response objects so that they can see whether they have already modified the response (and require implementations to use this facility where necessary). This data would have to survive serialisation, and we would need to think about how it would work for response objects that are not subclasses of 'HttpResponse'.

comment:6 by James Bennett, 17 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #730.

comment:7 by Jacob, 12 years ago

milestone: 1.0 beta

Milestone 1.0 beta deleted

Note: See TracTickets for help on using tickets.
Back to Top