Hi Luke, On 05/23/2011 05:14 PM, Luke Plant wrote: > On 23/05/11 18:33, Carl Meyer wrote: >>> 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 didn't read it that way, but thanks for the clarification ;-) >> 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. Yes, fair enough. I think your current patch is the best plan we have for now, and it wouldn't make sense to hold it; I'll keep noodling on the longer-term options. >>> 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. Yes, you're right here as well. I still think that pattern is ugly and hard to read, but given the documented behavior of those decorators, its function is clear. Carl -- 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.
