On 23/05/11 18:33, Carl Meyer wrote: > On 05/23/2011 11:31 AM, Luke Plant wrote: >> Putting all these things together, I've uploaded a new patch to #16004: >> >> http://code.djangoproject.com/attachment/ticket/16004/16004.fix.alternative.diff >> >> It is not nearly as invasive as the other changes you suggested. >> However, it does fix the failing tests in my project (i.e. fixes >> csrf_protect), and leaves TemplateResponse objects unrendered as long as >> possible, without having to change one line of my own code/tests. And >> the implementation doesn't make me puke either.
Just re-read this, and realised it might come over as suggesting that your idea *did* make me puke, which isn't what I was saying at all - I was saying that this (limited) implementation of what was essentially your idea turns out quite nicely. >> I agree that we ought to be doing something about middleware ordering >> and things like that, but perhaps that is something for Django 2000 - >> when we can sit down and work out how everything should fit together. > > This does seem like a relatively nice solution for TemplateResponse and > decorator_from_middleware. Isn't the patch missing a modification to > BaseHandler to do "response = response.render()" rather than just > "response.render()"? Yep, good catch. > Unfortunately, this doesn't do anything for the problem with cache_page > and Vary headers, which isn't related to TemplateResponse. I'm not sure > we can wait for Django 2000 to find a fix for that. OK, but I think it is less pressing. #16004 is significant regression introduced in 1.3, which is now causing more bugs in trunk due to actual use of TemplateResponse. #15855 has existed since the dawn of time (0.91 at least, as far as I can tell from eye-balling the code). I'm happy to wait around for a fuller solution to be thrashed out, but I worried that the pursuit of an ideal might make it actually more fragile than it is. I think long term we should move towards a single processing phase (e.g. decorators always just mark for action later). But to attempt that in a backwards compatible way requires making functions that are eager now lazy (in effect). That addition of laziness is very likely to add more bugs - as the addition of TemplateResponse highlighted. decorator_from_middleware is broken, but it has been broken for a long time, and its very likely that a lot is depending on its eager behaviour. >> There was also the general >> assumption of just one view being involved, one response object. We >> don't want to break code that is slightly more inventive, like this: >> >> http://docs.djangoproject.com/en/dev/ref/contrib/csrf/#view-needs-protection-for-one-path > > I don't like seeing that recommended in the documentation. For one > thing, it's highly dependent on detailed knowledge of the implementation > of csrf_protect and csrf_exempt. The response is still going to pass > through both of those decorators; a naive reader would be as likely to > assume "last-one-wins" as anything else. And I find it quite hard to > read, compared to just having a function available that takes the > request as argument and performs the CSRF check directly and immediately. It doesn't depend on anything in the implementation, as far as I can see. csrf_protect protects a view from CSRF attacks. csrf_exempt exempts a view from the checks the CSRF middleware performs. That is how they are documented, and that's all you need to know. That section of the documentation is pointing out a solution that you could deduce simply from the documentation of the parts. I guess whether it is obvious depends on what your model is of how these work. I'd argue that by far the most obvious model is that the csrf_protect does what it says eagerly i.e. immediately, especially given that is supposed to work when the middleware is absent. Regards, Luke -- "The first ten million years were the worst. And the second ten million, they were the worst too. The third ten million, I didn't enjoy at all. After that I went into a bit of a decline." (Marvin the paranoid android) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
