Code

Opened 9 years ago

Closed 7 years ago

Last modified 3 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: UI/UX:

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.

Attachments (0)

Change History (7)

comment:1 Changed 7 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

comment:2 Changed 7 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Design decision needed

c.f. also #730

comment:3 Changed 7 years ago by anonymous

  • Cc achambers.home@… added

comment:4 Changed 7 years ago by anonymous

  • Cc achambers.home@… removed

comment:5 Changed 7 years ago by lukeplant

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 Changed 7 years ago by ubernostrum

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #730.

comment:7 Changed 3 years ago by jacob

  • milestone 1.0 beta deleted

Milestone 1.0 beta deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.