Re: CSRF proposal and patch ready for review
Can I have some feedback on this please? I've now addressed everything outstanding (tested under HTTPS and updated the tutorials), and I've included a friendly summary at the top of http://code.djangoproject.com/wiki/CsrfProtection For those wanting to see the patch, for once Trac hasn't barfed on it, so you can see it with nice formatting here: http://code.djangoproject.com/attachment/ticket/9977/csrf_template_tag_r11477_1.diff As far as I'm concerned, this is ready for checkin, except that I haven't had *any* recent feedback or thumbs up etc. from the list or other core devs. This is a breaking change (i.e. there are required changes to some settings for things to continue to work), so it needs some attention, and it's security related as well (which justifies the breakage as well as more attention IMO). I really don't want this to sit around bitrotting or eventually get postponed to Django 1.3. It's best going in ASAP, so that we can iron out any problems with the upgrade instructions before releasing 1.2. Thanks, Luke -- "I'm at peace with the world. I'm completely serene. I know why I was put here and why everything exists. I am here so everybody can do what I want. Once everybody accepts it, they'll be serene too." (Calvin and Hobbes) 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
Every form template needs to import the CSRF > module. Every view with a form _must_ use a RequestContext. The > upgrade path for existing apps involves a lot of little changes. > Yes, you've documented the transition, but every extra step in a > transition document is one more thing that can (and will) be > misinterpreted or misunderstood by somebody. There's just too many > moving parts involved here for this to be a painless transition for > everyone. > > I completely agree that CSRF protection is important, and it's a > crying shame that the CSRF middleware is off by default. As Simon > mentioned early on, this probably means that the CSRF middleware > isn't getting used anywhere near as often as it should. > > I also follow the logic that lead from the SafeForm to the template > tag approach. > > However, I'm just not convinced that what we're left with is > something we want to impose as a project default. IMHO, the major > flaws of the middleware insertion approach are almost preferable to > the complexities introduced by the tag approach. This is kind of ironic - I pretty much argued this here: http://groups.google.com/group/django-developers/msg/1afbe3a79db00b71 But I've done my very best to minimize the concerns I listed there, and I do think that the tag approach is better, despite its flaws. One major reason is that if you have two POST forms in a page, one which is external and the other internal (a reasonable possibility on sites that have a login box on every page), then the automatic tag insertion method will silently give you a vulnerability, because it will send the CSRF token to an external site, and it's not possible to fix it. This is highlighting the fact that control of when the token is inserted should be on a per-form basis. Given that requirement, I don't see how you can get round needing something like a template tag in each form. (Disadvantages of alternatives like SafeForm have been discussed already). Even without two POST forms, i.e. if you have just one POST form that targets an external URL, while you can fix it, you still have to remember to add the csrf_response_exempt decorator to the view function. In fact, I've just discovered that there is a view in current Django that, if you have the current CSRF protection enabled, will leak the CSRF token to an external site -- the technical 500 debug view in django/views/debug.py has a POST form to dpaste.com. (I'll try to fix that soon, I don't think dpaste.com is likely to be malicious). This highlights what happens when you try to automatically and silently do something that you don't actually have enough information to do. With the template tag method, you are safe from this kind of accident. > A litmus test of this - the tutorial, which is pretty much the > simplest semi-real application you can build in Django, should be > CSRF protected without needing to explain at length what CSRF is. > The template tag solution current fails this test pretty > comprehensively. I agree that the tutorials are a litmus test, and it is a big shame that they are affected. On the other hand, I think the changes I've made to the tutorials, while probably not exactly polished, are adequate, and they don't explain at length what CSRF is, and I don't think they need to. The updated tutorial does highlight the problem of CSRF, and it's a problem that unfortunately every web developer needs to know *something* about. It's becoming more exploited in the real world, and just like developers need to know *something* about XSS and SQL injection, they do need to know about CSRF. IMO, the only time when it's justified to completely hide an issue like this from developers is when we can guarantee that it won't be a problem. To use examples from Django, the docs mention nothing about SQL-injection, because the ORM and the API for raw SQL (via cursor.execute) do not open up vulnerabilities. The template docs *do* talk about XSS under the 'Automatic HTML escaping' section, because that is a bit of 'magic' that the developer may need control over, and so will need to know about. For CSRF, we can't automatically fix it, so we can't pretend it doesn't exist. Thanks again for your feedback. Like you, I'm far from being 100% happy with this solution, but it's the lesser of two evils IMO. Regards, Luke -- "Ineptitude: If you can't learn to do something well, learn to enjoy doing it poorly." (despair.com) 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
I wrote: > In fact, I've just discovered that there is a view in > current Django that, if you have the current CSRF protection > enabled, will leak the CSRF token to an external site -- the > technical 500 debug view in django/views/debug.py has a POST form > to dpaste.com. (I'll try to fix that soon, I don't think dpaste.com > is likely to be malicious). Actually, I've realised I would need to import a contrib function (csrf_response_exempt) into core to fix this. That's a bad thing to do, so I'm thinking of not fixing this unless anyone screams. Luke -- I never hated a man enough to give him his diamonds back. (Zsa Zsa Gabor) 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
al request-related things accessible to the templates is exactly the reason RequestContext exists, so it seems strange to use another mechanism. > However, as I said, these ideas are mere zygotes of real thoughts. > I'll let you know when they've gestated a little more. I replied to them anyway, hopefully to some benefit. > By any chance, are you coming to DjangoCon? This would be a > fantastic subject for a sprint discussion. No, I'm afraid not (though I'd love to go). I have a full time job which is not programming related, and it would be too far for me anyway (I'm in the UK). Luke -- I never hated a man enough to give him his diamonds back. (Zsa Zsa Gabor) 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 -~--~~~~--~~--~--~---
Re: proposal: decouple admin index page from INSTALLED_APPS
On Friday 04 September 2009 10:00:37 patrickk wrote: > e.g, when I´m using djangos auth-app and I´m extending the user-model > with a user-profile, I´m having "auth" (with users and groups) and > "user" (with user profile) on my admin index page. orderd by names, > auth is very much on the top of my page while user is at the bottom. > for an editor, this is probably hard to understand (because the editor > doesn´t know anything about apps). for an editor, it´d more > comfortable having a headline "user management" with the apps "users", > "groups" and "user profiles". this re-arrangement could be defined in > admin.py. You can also define the arrangement by overriding the admin template for the index and hard coding in your own order. It's not ideal, but it's perhaps preferable to adding another place for configuring the admin. If you want this kind of flexibility for the index page, you might also want to add extra notes etc onto the page, which makes customising the template a reasonably good way to do it. Having an admin.py for every project is a bit vague, because 'projects' don't really exist as far as Django is concerned, only 'apps'. Luke -- I teleported home one night With Ron and Sid and Meg, Ron stole Meggie's heart away And I got Sidney's leg (THHGTTG) 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
On Monday 31 August 2009 15:26:42 Russell Keith-Magee wrote: > 1. Is there anything we can do to fix the problems with the > ResponseMiddleware? I know there are problems, but at the core there > is something really good - it seems a waste to throw it all away. I just stumbled across a new argument against the CsrfResponseMiddleware: http://code.djangoproject.com/ticket/9163 To me, this confirms my gut instinct (or rather, Jacob's), that post- processing the response is really nasty. Luke -- Life is complex. It has both real and imaginary components. 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
On Monday 31 August 2009 15:26:42 Russell Keith-Magee wrote: > 3. CSRF is currently a contrib app. Why? CSRF control is the very > model of a feature that shouldn't be decoupled from the base > framework. If we're aiming to make CSRF support like XSS support, > surely it should be baked into the core, not kept isolated as a > contrib app. Is there anything else that gets easier if we sink this > into the core? At the very least, the tag loading issues go away - are > there other possible benefits? For the sake of recording my thoughts, one advantage of keeping it as a contrib app is that developers can completely replace the CSRF mechanism if they don't like the bundled one. Simply by doing s/django.contrib.csrf/thirdparty.csrf/ to their INSTALLED_APPS, MIDDLEWARE_CLASSES and TEMPLATE_CONTEXT_PROCESSROS settings, they would replace the CSRF mechanism (including the templatetag library) with their own. Their own {% csrf_token %} might return an empty string (e.g. if they were just using Referer checking or Origin checking or something), but that's fine. The admin and all other apps would then seamlessly use the different CSRF mechanism. So keeping it as contrib and not in core might be an advantage if some websites have special requirements, or the bundled CSRF mechanism becomes outdated. Luke -- "Mistakes: It could be that the purpose of your life is only to serve as a warning to others." (despair.com) 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
On Tuesday 15 September 2009 12:28:51 Russell Keith-Magee wrote: > The CSRF tag approach you have implemented didn't win a lot of fans > whenever I described it, and for pretty much the same reasons I have > expressed previously - too many moving parts, and a little too much > manual intervention required. > > The discussions I had with Simon and Andrew revolved around trying to > improve the interface to SafeForm, and I think we may have a workable > solution. I've only just arrived home, so I'm fairly jet lagged at the > moment; I'll try to put the result of our discussions into the form of > a formal proposal over the next day or so. OK, I'll wait and see. To clear things up, the template tag approach really has just two 'moving parts': - use RequestContext in your view (often you will be covered already e.g. generic views, and lots of people already using RequestContext everywhere) - use the csrf tag in the template. Everything else is global settings i.e. a one time change, which you will be prompted about *once* if you forget them, and then you can forget them. I'd be very surprised if SafeForm was a better result - it has major flaws that can't be addressed by API changes, AFAIC: * it requires using Django's form system. I've got plenty of views that don't (e.g. anything with a dynamic number of controls). People not using django.Forms are left out in the cold, or having to learn another way to do things. * it's off by default. Turning it on by default will require making django.forms.Form subclass SafeForm, which will almost certainly require a huge and immediate upgrade effort, because SafeForm cannot have the same API as Form. If it's off by default, I see no point at all in this entire exercise. If we don't arrive at a point where the POST form created in the tutorial is safe from CSRF, I think we've failed. And it will still require changes to templates, just like the template tag, and it will also require changes to views, only significantly more complex than simply using RequestContext. It's got at least as many and at least as intrusive 'moving parts' AFAICS. It also has the disadvantage that it is much more intrusive - you can't just switch out the implementation of your CSRF protection mechanism like you can with the template tag and middleware. But I'll wait 'til I see your proposal. Luke -- "My capacity for happiness you could fit into a matchbox without taking out the matches first." (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 -~--~~~~--~~--~--~---
Re: #10436 status
On Wednesday 16 September 2009 12:21:54 Tom Evans wrote: > On Wed, 2009-09-16 at 04:09 -0700, Aljosa Mohorovic wrote: > > could somebody check ticket #10436? > > is there some reason why patch for ticket #10436 is not in trunk? > > > > Aljosa Mohorovic > > It was committed 6 months ago? > > http://code.djangoproject.com/changeset/10030 That would be #10463, not #10436 Luke -- "My middle name is 'Organised'! My first name is 'Poorly'." 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
they won't do so loudly, or with an obvious cause of the error, or with a workaround to buy them time to update their code. OK, now onto a reply, an alternative, and some comparisons: > > * it's off by default. Turning it on by default will require making > > django.forms.Form subclass SafeForm, which will almost certainly require > > a huge and immediate upgrade effort, because SafeForm cannot have the > > same API as Form. If it's off by default, I see no point at all in this > > entire exercise. If we don't arrive at a point where the POST form > > created in the tutorial is safe from CSRF, I think we've failed. > > My priority for this is a little different: while I would dearly love > to see all Django applications secure against CSRF by default, I can't > see a way of doing it that doesn't break existing apps. More important > for me though is that the Django admin (and other reusable apps, such > as Pinax stuff) MUST be secure against CSRF out of the box, no matter > what the user puts in their settings.py file. If developers fail to > protect themselves (especially when the solution is well documented) > then at least it isn't our fault. Given the massive (IMO) disadvantages to the SafeForm solution I've described above, I think it's pretty disingenuous to say "well we provided you with the tools, it's your fault if you're not protected". The solution needs to be much better than well documented -- it needs to be easy to use, and easy to change to from existing code. You also need to update all the existing Forms documentation to tell people that really they shouldn't be using this. With the amount of new stuff people need to learn to use SafeForm, they are just not going to bother. I guess the biggest thing here is we've got different priorities, as you said -- I want to make it easy for developers to create apps that are secure against CSRF, while you want to make the contrib apps secure, and want to ensure no breakage even if people don't read release notes. The SafeForm solution doesn't actually achieve that (see 6.3 and 6.4 above), and even if it did, it is at the massive cost of baking in a CSRF solution which has such big usability problems that I doubt anything outside contrib apps will adopt it. In fact, if you simply want to fix contrib apps, SafeForm is a massively over- engineered solution, as there are *much* less invasive ways to do it. All you need to do is: 1) add the CSRF template tag to builtins and use it in templates. 2) add a @csrf_protect decorator on views. 3) hack RequestContext so that it always includes the CSRF context processor. All contrib apps use RequestContext anyway, so the changes needed for that method would be tiny, with no changes to settings.py required. This would handle errors like the current middleware, but the error messages would only occur when there is a genuine CSRF attack. (The reason I don't favour this solution is because of criticisms 4 and 5 above). The best test case for which solution is easier to use is to start by patching all the contrib apps - forms, views, templates and tests. Then you have to remember that you are expecting every developer out there to do the same with their code base if the solution is really going to be a viable 'blessed' solution. I don't particularly want you to write that patch for SafeForm, because I think it would be a waste of time, but I think that you'd be convinced of the points above if you did. For comparison, the template tag required the following changes to contrib apps: View functions: 0 changes (because they all used RequestContext already, as will much existing code - all that use generic views etc.) Tests: a single 1 line change to a test that counted the number of s in an admin page (which was a pretty fragile test that has been updated multiple times already). Templates: 18 templates - added {% load csrf %} and {% csrf_token %} To give another example, here is the changeset for one real site (about 12,000 LOC) to switch to the template tag method: http://bitbucket.org/spookylukey/cciw-website/changeset/f2edd690fd9e/ It has changes to settings and templates, but zero changes to view functions, and zero changes to tests. That's better than some projects, because I obviously must have already been using RequestContext for all POST forms, but I don't think that will be atypical. OK, I'm done! Wondering-if-I-overdid-it-ly :-) Luke -- "Oh, look. I appear to be lying at the bottom of a very deep, dark hole. That seems a familiar concept. What does it remind me of? Ah, I remember. Life." (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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
Hi Simon, > 1. SafeForm has no dependency on Django's session framework. This is A > Good Thing as I personally avoid sessions in my projects (no need to > access state on every request if you can get away with not doing it). ... > It's a bit more than that - I also want something that works > independently of sessions, middleware and RequestContext, but that's > mainly due to my own personal Django development preference. Just one thing to say in reply - the new version of CsrfViewMiddleware is also independent of sessions. It sets its own cookie just like SafeForm does. The references to the session framework still in there are purely for backwards compatibility -- if a form is created and sent to the user with Django 1.1, and the site is then upgraded to 1.2, when the user submits the form the request will have a session cookie and a CSRF token, but no CSRF cookie, which would produce a 403 error. So for that tiny little window, we allow a fall-back to using the 1.1 method by allowing a session cookie for a CSRF cookie. The fall-back doesn't actually trigger any session activity - it just looks at request.COOKIES[settings.SESSION_COOKIE_NAME]. I also had one thing to add to my previous e-mail - on the backwards compatibility front, there are also the tests that people may have written against customised contrib views. I have examples of these in my projects -- where I have added significant customisations to the admin, and I want to test certain post-conditions etc. In these cases, we also have potential for breakage - the tests are going to fail until they are updated in the same way that core tests need to be updated to get and use the CSRF token. This will happen even if people have used 'best practice' (using super() and inheritance so they don't duplicate any actual contrib code or templates). The examples I have in my code actually would not break, because I happen to have used twill for those tests, but I think the point stands. (You could argue that 'best practice' for tests means using twill-like tests, but Django tests have not provided either a good example or helpful utilities on this front, so it's an unreasonable expectation IMO). Broken tests are not as bad as breaking the actual site, but any responsible developer is going to fix rather than ignore failing tests, so it definitely adds to the upgrade cost. Regards, and thanks for taking my (lengthy) criticisms in good spirit! Luke -- OSBORN'S LAW Variables won't, constants aren't. 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
On Saturday 19 September 2009 16:56:52 Russell Keith-Magee wrote: > On Fri, Sep 18, 2009 at 6:09 AM, Luke Plant wrote: > > If the target of a is internal: > > * add {% load csrf %} to the template and {% csrf_token %} to the form > > * use RequestContext in the corresponding view. > > > > And that's it. > > ... once you've also got your INSTALLED_APPS, CONTEXT_PROCESSORS and > MIDDLEWARE_CLASSES set up correctly - and make sure you have, because > if you haven't you're going to get weird errors - or worse still, > accidentally disable CSRF protection. Fair point - I was explicitly talking about the *day-to-day* usage at that point. For the other things, the errors aren't quite so inscrutable any more - the 403 page looks like this now: http://lukeplant.me.uk/uploads/django_csrf_403.html The only way to *accidentally* disable the protection and not know is if you remove the middleware - other changes will result in an error. In reality, this is going to happen when people comment it out for the sake of debugging, and forget to put it back in again, or just decide they don't care. I can see that will happen more than you'd hope... ...but is it really our fault if people disable the protection? I guess to the extent that we've made it difficult to get right, or more tempting to do it wrong, it *is* our fault. So I don't think we necessarily need to make it *impossible*, just minimise the incentive. > So - is there anything we do to make the template tag more palatable? > > I think we can - and you've already shown how: > > In fact, if you simply want to fix contrib apps, SafeForm is a massively > > over- engineered solution, as there are *much* less invasive ways to do > > it. All you need to do is: > > 1) add the CSRF template tag to builtins and use it in templates. > > 2) add a @csrf_protect decorator on views. > > 3) hack RequestContext so that it always includes the CSRF context > > processor. > > > > All contrib apps use RequestContext anyway, so the changes needed for > > that method would be tiny, with no changes to settings.py required. OK, I agree that this solution has some nice features, but let's be realistic about its failings: 1) changing RequestContext so it always includes the CSRF context processor is ... 'cowboy', as I think Simon would put it :-) I wasn't actually suggesting that as a real proposal. 2) We would still have to fix all the tests, because we can't just turn the decorator off. There are also the tests that we can't fix, i.e. where other people have customised contrib apps in various ways and written tests against the views. I mentioned this in my most recent reply to Simon. I'm not so bothered about having to fix ours, it's other people's that worry me, and the DRY violation - baking it in like this means that every test of a POST view is now concerned with CSRF. I can think of some hacks to avoid fixing tests (ours and other peoples): * make the decorator turn itself off: * on the basis of some setting. You can be sure that this will suffer from the same problem as the middleware setting - people will turn it off for production. * on the basis of some other way of detecting test suite mode, whether 'runtests.py' or 'django-admin.py test' * hacking test.Client.post() to always include a CSRF cookie and token. Easy to do, but fairly 'cowboy'. I'm guessing it could also break things that depend on the presence/absence/number of cookies. I think the lazy instinct which makes me not want to fix even our own tests is a healthy instinct at root...but I don't know how to address it best. > A middleware is really the only way to ensure that every view is > protected by default - which is a certainly a desirable goal. However, > if adding CSRF protection requires _any_ changes on the part of the > end user, then I can guarantee that somewhere, a > lazy/time-malnourished developer will take the "just disable the > middleware and make it work" approach. As soon as this happens, > contrib.admin becomes insecure. On this point, I completely agree with > Simon: End users should be allowed to be as lazy as they like, but > their laziness shouldn't open security holes in an app that Django > ships, since the contrib apps (and admin in particular) are the > obvious first port of call for any systematic CSRF attack. Perhaps we could add some code to the template tag that checks MIDDLEWARE_CLASSES for the CSRF middleware and if it's not there, and it's on a production site, posts some information to a public web site to name and shame the developer, and e-mails their boss as well... :-) Serio
Re: CSRF proposal and patch ready for review
On Saturday 19 September 2009 16:56:52 Russell Keith-Magee wrote: > So - let's have both. A middleware enabled by default protects the > rest of the views. However, we can also have a view decorator lets us > protect admin (and other contrib apps) explicitly. If users disable > the middleware, admin remains secure. If users fail to follow the > migration instructions correctly, admin remains secure. Users like > Simon that were tormented by middlewares when they were younger ( :-) > ) can disable the middleware and use the decorator liberally > throughout their own projects; the worst possible outcome here is that > one of their own views is insecure - admin is unaffected. > > The middleware and view can easily be made to play nicely together - > there's no need to check the token twice, so make the middleware set a > flag on the response object that marks the content as CSRF-checked; if > the decorator sees that flag, it doesn't need to duplicate the check. Another advantage of the middleware + decorator solution is that sometimes there might be a legitimate case for turning off the middleware globally e.g.: - if for some reason there are no POST forms, and never will be. - if all POST interaction is done using AJAX - if the user has implemented their own solution for CSRF for their own apps. In this situation, it's nice if the developer can turn off the middleware and still have the admin protected. In trying to implement this, I came across the problem that the same decorator can't be used for methods and functions, and I need to decorate some methods of AdminSite. (AdminSite uses never_cache directly on methods quite a bit, but this only works by accident - never_cache doesn't use the view function arguments (e.g. the request object), otherwise it would fail). Does anyone have a nice solution to this? Regards, Luke -- "Outside of a dog, a book is a man's best friend... inside of a dog, it's too dark to read." 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 -~--~~~~--~~--~--~---
decorator_from_middleware change
Hi all, I need to fix #6371 [1], and I've found a nice way to do it (something like [2]). However, decorator_from_middleware is a pain, since it doesn't always return a an actual decorator, for "historical reasons". I need to change this to fix the bug. Is anyone against this? decorator_from_middleware isn't actually documented anywhere, but I'm guessing it might be used outside of the Django code base. I want to change it as follows. For decorators like gzip_page, everything works as normal: gzip_page = decorator_from_middleware(GzipMiddleware) The only difference is that gzip_page is now a true decorator. For decorators which require arguments given to the middleware, you will now need to use decorator_from_middleware_with_args. I will manually fix the one decorator this breaks (cache_page) to provide backwards compatibility with the published documentation. Does anyone object to this change? It makes fixing #6371 a *lot* easier. In fact I can't actually get my head around how to fix it at all without the change. Luke [1] http://code.djangoproject.com/ticket/6371 [2] http://stackoverflow.com/questions/1288498/using-the-same-decorator-with- arguments-with-functions-and-methods/1288936#1288936 -- Parenthetical remarks (however relevant) are unnecessary 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 -~--~~~~--~~--~--~---
Re: decorator_from_middleware change
On Monday 21 September 2009 20:27:50 Jacob Kaplan-Moss wrote: > On Mon, Sep 21, 2009 at 1:21 PM, Luke Plant wrote: > > However, decorator_from_middleware is a pain, since it doesn't always > > return a an actual decorator, for "historical reasons". I need to change > > this to fix the bug. Is anyone against this? > > No, I think this is precisely correct. I've been meaning to do exactly > what you're proposing for a while myself; just haven't gotten around > to it. > > > decorator_from_middleware isn't actually documented anywhere > > I actually avoided documenting it because it's broken. Once you fix > it, we should (i.e. I will, if you don't have time) document it. I've committed my change [1], and also replaced _CheckLogin with my method [2] (it was essentially the same method, just generalised). I haven't added any docs, because I'm not sure it's perfect yet (and also because I am lazy and I was mainly working on something else, this just got in the way). In the 'normal' case where you don't need to pass any args to the middleware, it is exactly the same (but works for decorating methods now). In the abnormal case, you have to use decorator_from_middleware_with_args, which is a pretty ugly name, if explicit. I don't know if you had other ideas about what to do with this. Changing that name is easy, it's only used by cache_page. Luke [1] http://code.djangoproject.com/changeset/11586 [2] http://code.djangoproject.com/changeset/11587 -- Parenthetical remarks (however relevant) are unnecessary 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
OK, you convinced me. I really would rather this wasn't baked in, but given the migration issues and the fact that it is security related, I guess I can stomach it. I've updated the patch [1] to move things to builtin functionality. I also had to fix some bugs to get the csrf_protect decorator working for methods, which are in trunk already. I've left most of the code itself under django/contrib/csrf because: 1) backwards compatibility with people importing the middleware means we have to leave django/contrib/csrf for some things anyway. 2) In this case, I don't see any great advantage in having stub modules which just import other stuff for backwards compatibility 3) I really can't be bothered to change all the imports at this point in time! I moved the template tag itself to core code, because it was causing import cycles otherwise, and there are no backwards compatibility issues, nor does it add any actual imports of contrib code to core. I think the patch now addresses all your/our concerns: - decorator on all contrib view functions that need it. I'm pretty sure I got them all, but I only decorated the ones that actually need it (i.e. use POST), and I only added it on the 'source' function (e.g. auth.views.password_change and not AdminSite.password_change) because otherwise you end up decorating many times. I also added the decorator in AdminSite.admin_view, which will be a catch-all for all admin views (provided users are using AdminSite.urls) - the view middleware is still on by default. - csrf_token is in builtin templatetags - csrf context_processor is hard coded into RequestContext - tests 'fixed' with very little work. I fixed the tests by a custom attribute on request objects that tells the middleware/decorator to not actually reject requests. This is better than disabling completely, because it means that the middleware will still send cookies etc., and it's always good to have tests as close as possible to the real code. The test client adds the custom attribute to HttpRequests after it has created them. I had to add the attribute in one other place in the code as well - a test that was manually calling a view function that had csrf_protect applied. This method of fixing tests was also the best for testing the CSRF middleware - globally mocking the middleware out would have made it hard to test the middleware itself! Docs are all updated, all tests passing etc. If people are happy for this to go in, it would be very helpful if other people could have a go updating their apps and give the general docs/upgrade instructions/tutorials a good check after I commit it. I can't easily do checks like that, because I just won't spot the holes after having the code in my head for so long. The only thing left is a nicer render_to_response shortcut for using RequestContext, which is a refinement we can add later. Regards, Luke [1] http://code.djangoproject.com/attachment/ticket/9977/csrf_template_tag_r11587_1.diff -- Parenthetical remarks (however relevant) are unnecessary 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
On Tuesday 22 September 2009 13:12:51 Russell Keith-Magee wrote: > On Tue, Sep 22, 2009 at 10:34 AM, Luke Plant wrote: > > I've left most of the code itself under django/contrib/csrf because: > > > > 1) backwards compatibility with people importing the middleware > >means we have to leave django/contrib/csrf for some things > >anyway. > > 2) In this case, I don't see any great advantage in having stub modules > >which just import other stuff for backwards compatibility > > This isn't just a "moving deckchairs on the Titantic" thing - I can > think of at least three good reasons we should be moving the code into > core modules: > > * Maintaining the basic contract of django.contrib - that you should > be able to delete the contrib directory and have Django still work. If > all the CSRF code is in contrib, this won't be the case. That doesn't happen to be true at the moment - django.core.context_processors depends on django.contrib.auth. (Not saying that's a good thing, just highlighting a fact). But on the other hand, you can at least disable the auth context processor, and you can't disable the CSRF one. The other reason for leaving it as it is at the moment is for the sake of the patch - it's much harder to see what's going if you are moving code as well as changing it. If the other devs agree it needs to move to core, I'd prefer to do two commits, the first which is just functionality changes - for the sake of a comprehensible changelog - and the second that moves the files and changes all the imports etc. To do this properly, we'd also need to reorganise docs, fix all the :ref:s etc. Another problem with moving it all is that is makes upgrade instructions a bit more complex (which had just got a bit simpler), but I guess we may as well do it all at once. > I'm reasonably happy with the testing approach you've taken - or, at > least, I can't think of anything substantially better. > > > My only comment is that we might want to put an underscore on the > magic variable (i.e., request._dont_enforce_csrf_checks) to reinforce > the fact that this really isn't public API. Good call, I'll fix that. I have mentioned the existence of the attribute in the docs in the 'testing' section, but not its name, precisely because it's not a public API, but some people might need to know about it in order to fix their tests. > At this point, I'm convinced, mod the minor things I've flagged. > However, I'd like to see Jacob and Malcolm chime in before this is > committed. Agreed. Luke -- "Pessimism: Every dark cloud has a silver lining, but lightning kills hundreds of people each year trying to find it." (despair.com) 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
On Tuesday 22 September 2009 12:57:16 Simon Willison wrote: > The main reason I really like preserving form data is that it means > CSRF failures are less of a problem, allowing us to be much more > strict about how they work (setting form tokens to expire after a few > hours, tying tokens to individual forms etc). This means we can reduce > the damage caused in the case of a token leaking somehow. This becomes > particularly important if tokens are going to be used to protect GET > requests, which some applications may want to do (Flickr have CSRF- > protected GETs for their logout and change-language links, for > example) - GET tokens are more likely to leak in other people's > referrer logs or to intermediate proxies. I'm not convinced that token leakage is going to be a problem that can easily be fixed with timeouts. Having someone's CSRF token isn't going to be useful in an attack unless you know whose it is. (An attacker could possibly log IP addresses against CSRF tokens for later attacks, but that's not likely to be very successful). The obvious and easy way to do any kind of systematic attack is to immediately send some attack javascript back in response to any incoming request that contains a Referer from an external target along with a csrf token. Timeouts are not going to help you there. > I'd be enormously happy to see that go in as django.shortcuts.render - > the name may not be as descriptive as render_to_response but for > something used in practically every view I think terseness should beat > descriptiveness. +1. Your TemplateResponse class sounds cool, but perhaps too clever to be default way of doing things that we would put in documentation. > 2. I'm not at all keen on the implementation as a middleware > (especially a view middleware, which doesn't play well with generic > views and redispatching to other view functions, both patterns I like > a lot). Could you explain a bit more about the difficulties with generic views? AFAICS, decorators seem slightly worse than a middleware, because you end up using them twice e.g.: @csrf_protect def some_generic_view(request, **kwargs): # POST processing here. @csrf_protect def my_view(request): # some stuff here, then return some_generic_view(request, blah='blah') With a middleware, you don't have this duplication. What are the disadvantages of the middleware? > I'd be perfectly happy with a decorator that is also available > as a middleware. As it stands, there is a middleware and a decorator created using decorator_from_middleware. > 3. I'd like to include support for CSRF form tokens to expire and be > locked down to individual forms, as seen in SafeForm. Whether or not > we actually go for that depends on how likely we are to find a > solution to the next point... > 4. I'd love it if we could find a way for developers who care (such as > myself) to opt-in to CSRF-failing gracefully at the form validation > level. I'm confident this should be possible, but I don't think it's > necessary to make it the default behaviour (the CSRF 403 screen is > probably fine for most people). If I'm thinking correctly, this isn't too hard: 1) Implement your csrf_protect_form method. That could easily add your requirement to lock down to individual forms and timeouts. It would need cooperation from a new template tag, or additional optional arguments to the current template tag. It might also need an additional context processor in settings, but as an opt-in solution that's OK. I think the solution that manipulates request.POST sounds OK - yes a hack, but providing this method is not the default, most people won't have to deal with any problems with it. 2) Get the view to be exempted from the normal CSRF checks done by the middleware. Thankfully, we already have not one but two ways of doing this - the manual @csrf_exempt decorator on views, and the internal mechanism that allows the decorator and middleware to avoid duplicate checking. Automatically doing the latter in csrf_protect_form is probably the way ahead. So, if I'm thinking straight, it should just be a matter of this: -- views.py --- @csrf_protect_form(name="myform", timeout=60*60*24) def my_view(request): # ... render(request, 'my_template.html', ...) -- my_template.html -- {% csrf_token "myform" %} The only question mark in my mind is what happens with multiple forms on a page (e.g. when you have a login box on every page). It might not be an issue - the target of the login box will be another view anyway - but it needs thought. Luke -- "Pessimism: Every dark cloud has a silver lining, but lightning kills hundreds of people each year trying to find it." (despair.com) Luke Plant |
Re: decorator_from_middleware change
On Tuesday 22 September 2009 21:31:08 James Bennett wrote: > On Mon, Sep 21, 2009 at 9:04 PM, Luke Plant wrote: > > I've committed my change [1], and also replaced _CheckLogin > > with my method [2] (it was essentially the same method, just > > generalised). > > The decorator_from_middleware change appears to have broken > cache_page; I'm now getting "AttributeError: 'int' object has no > attribute '__module__'" from views which use cache_page. Bummer, I tried hard not to break it - I added backwards compatibility tests for the basic different uses. Could you produce a test case? Cheers, Luke -- "Pessimism: Every dark cloud has a silver lining, but lightning kills hundreds of people each year trying to find it." (despair.com) 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 -~--~~~~--~~--~--~---
Re: decorator_from_middleware change
On Tuesday 22 September 2009 20:37:05 kmike wrote: > cache_page decorator previously used to have optional > 'key_prefix' argument, not only timeout. Is it gone? Can I use > > @cache_page(3600, key_prefix='vasia') > def my_func(request) >... > That wasn't documented anywhere as far as I can see, so now it's gone. I had to make some decision about how far the backwards compatibility went, and current policy is to go with documented API, which explicitly says "cache_page takes a single argument: the cache timeout, in seconds." [1] If there is some documentation/tutorial about the key_prefix argument, or if this change is going to cause lots of breakage, perhaps we'll have to revisit this. For the time being, you can create a 'cache_page' decorator that does what you want very easily, as in the example below. James B - do we have a place to list things like this i.e. things that probably should go in release notes? > Another question: in 'decorator_from_middleware_with_args' > docstring example stated: > Use like:: > > cache_page = decorator_from_middleware(CacheMiddleware) > > Is it correct? Nope, should be decorator_from_middleware_with_args of course, thanks, will fix. Luke [1] http://docs.djangoproject.com/en/dev/topics/cache/#the-per-view- cache -- "Pessimism: Every dark cloud has a silver lining, but lightning kills hundreds of people each year trying to find it." (despair.com) 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 -~--~~~~--~~--~--~---
Re: CSRF proposal and patch ready for review
On Tuesday 22 September 2009 21:24:48 Luke Plant wrote: > 2) Get the view to be exempted from the normal CSRF checks done > by the middleware. Thankfully, we already have not one but two > ways of doing this - the manual @csrf_exempt decorator on views, > and the internal mechanism that allows the decorator and > middleware to avoid duplicate checking. Automatically doing the > latter in csrf_protect_form is probably the way ahead. Hmm, had a thinko there. The middleware is run *before* decorators have had a chance to modify the request object. So only the first of these will work I think. That plays badly with your method of dispatching from your own view code. You will have to manually csrf_exempt your top level view code, and manully apply csrf_protect_form as needed. Luke -- "Pretension: The downside of being better than everyone else is that people tend to assume you're pretentious." (despair.com) 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 -~--~~~~--~~--~--~---
Re: decorator_from_middleware change
On Wednesday 23 September 2009 23:40:25 James Bennett wrote: > So, I've worked out what the problem is. > > Previously either of these worked: > > cache_page(timeout, view) > cache_page(view, timeout) > > Now, cache_page assumes that the first positional argument will be > the timeout. So what we're seeing when running some of our code on > trunk is cache_page treating the timeout value (an integer) as the > view to be cached, and failing when trying to find things like > __module__ attached to it. > > I don't know for certain how many people may be using this idiom, > since it was never documented (AFAICT cache_page always documented > itself as putting the timeout first and the view function second), > so I'm not really sure what (if anything) we should do about it. > > There may also be deeper issues with the fact that it's not > possible to call cache_page with explicit keyword args, since it > accepts **kwargs but doesn't do anything with it, always assuming > that the timeout and view can be pulled from positional args. I deliberately wrote it the way it is documented, and only that way. So it accepts these forms: cache_page(timeout, view) cache_page(timeout)(view) It shouldn't accept **kwargs though, that's a mistake, because it does nothing with them, and it would be better to fail loudly in that case. I've fixed that, and also added some asserts to get a better error message for now unsupported ways of calling it. Do you think this is enough? Luke -- "Pretension: The downside of being better than everyone else is that people tend to assume you're pretentious." (despair.com) 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 -~--~~~~--~~--~--~---
Re: decorator_from_middleware change
On Monday 21 September 2009 20:27:50 Jacob Kaplan-Moss wrote: > No, I think this is precisely correct. I've been meaning to do > exactly what you're proposing for a while myself; just haven't > gotten around to it. > > > decorator_from_middleware isn't actually documented anywhere > > I actually avoided documenting it because it's broken. Once you fix > it, we should (i.e. I will, if you don't have time) document it. OK, it's committed now. (r11586, r11593) Technically this is a bug fix (#6371), so it ought to get backported to 1.1.X. However, it does actually introduce backwards incompatibilities with cache_page (cache_page still works exactly as documented, but various people were using various undocumented features of it). Also, you could argue it is a new feature - "these decorators now work with methods". So, in light of those things, should it be backported to 1.1.X or not? Luke -- "Pretension: The downside of being better than everyone else is that people tend to assume you're pretentious." (despair.com) 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 -~--~~~~--~~--~--~---
Re: Adding signing (and signed cookies) to Django core
On Thursday 24 September 2009 18:18:56 Simon Willison wrote: > SECRET_KEY considerations > = Can I add some other things I've been worrying about while we're on the topic? In other web apps (I think Wordpress?), there have been problems associated with use of secret keys when the same key is used for different purposes throughout the application. Suppose one part of an app signs an e-mail address for the purpose of an account confirmation link sent in an e-mail. The user won't be able to forge the link unless they know HMAC(SECRET_KEY, email). However, suppose another part of the website allows a user to set their e-mail address (merely for convenience), and stores it in a signed cookie. That means an attacker can now easily get hold of HMAC(SECRET_KEY, email), and forge the link. There are many places in Django that use SECRET_KEY. I'm not currently aware of any vulnerability, because in most cases the attacker has only *limited* control over manipulating the message that is being signed. But I may have missed some, and without some systematic method, it would be easy for one place to open up vulnerabilities for all the others. So I propose: - we review all the Django code involving md5/sha1 - we switch to HMAC where appropriate - we add unique prefixes to the SECRET_KEY for every different place it is used. So for the e-mail confirmation link, we use HMAC("email-confirmation" + SECRET_KEY, message) - also add the ability to do SECRET_KEY rotation, as Simon suggested. This suggests we want a utility wrapper around hmac that looks like hmac(unique_key_prefix, key, message) and handles all the above details for us. The main difficulty is the way this could break compatibility with existing signed messages, especially persistent ones like those stored in password files etc. Luke -- "Smoking cures weight problems...eventually..." (Steven Wright) 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 -~--~~~~--~~--~--~---
Re: Adding signing (and signed cookies) to Django core
On Friday 25 September 2009 12:27:53 Simon Willison wrote: > Do you have any further information on the WordPress problems? No, I can't find it. It might not have been WordPress. All I remember is that it was along the lines of what I outlined in my previous e- mail -- one part of the application was essentially allowing the user to retrieve MD5(secret_key + user_supplied_data) (might have been HMAC or SHA1), which allowed them to get past another bit of security. Luke -- "Smoking cures weight problems...eventually..." (Steven Wright) 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 -~--~~~~--~~--~--~---
CSRF - next step?
Hi all, I just want to know what the status is before committing the CSRF stuff: * Jacob am I waiting for a thumbs up? I think you said you were going to try out the code. * Simon am I waiting for your patch? If I'm not waiting for either, my plan would be: * Commit what I've got pretty much as is. * Then move it all from contrib to core * Question: where should the docs (currently ref/contrib/csrf.txt) be moved to? ref/csrf.txt seems like a sensible place, but I don't know. * Then add a render_to_response_with_request shortcut (once we can decide on a colour...) and adjust the tutorials. Simon: * I could wait for you to write your csrf_protect_form code before doing any of this, merge it and then do the above. * You could write it against my lp-csrf_rework branch (which is not going to change substantially), and I can move your patch over to core (or you could move it). * You could wait 'til I'm done so that we don't have to worry about the fact that everything is going to move. Personally, I think your patch would be better going in separately, as it is additional functionality that is not used by default anywhere. Also, I have written a Python script that attempts to help people find all the s and view code that needs attention. It has a whole bunch of limitations and caveats, but I think it's pretty useful 90% solution and automates a lot of what people would have done using grep etc. Where should it go? It's currently in 'extras/' which seems to be the right place. Luke -- "Smoking cures weight problems...eventually..." (Steven Wright) 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 -~--~~~~--~~--~--~---
Re: CSRF - next step?
On Saturday 26 September 2009 19:44:24 Alex Gaynor wrote: > So I'm still a little unclear on what this shortcut does that > direct_to_template doesn't already? It just has a slightly different and simpler API, and an import that does not involve generic views. It also *doesn't* do some things that direct_to_template *does* do, like the params dictionary. I think it needs to be simple and obvious enough to be used early in the tutorials, before generic views have even been introduced. But it's certainly an optional part of the CSRF stuff - just something to make using RequestContext easier and simpler. Luke -- "Smoking cures weight problems...eventually..." (Steven Wright) 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 -~--~~~~--~~--~--~---
Re: decorator_from_middleware change
On Saturday 26 September 2009 20:10:59 kmike wrote: > Not true. cache_page decorator is now documented as putting view > first and timeout second. Take a look at > http://docs.djangoproject.com/en/dev/topics/cache/#the-per-view-ca > che Doh, I don't know how I managed to read those several times and not see that! I've fixed it now, thanks! Alex Gaynor reckoned both ways round were possible with previous code. I haven't checked that, but I'll believe him, and I've added compatibility for that. > Another thought. If key_prefix is deprecated in cache_page > decorator maybe it should be also deprecated in CacheMiddleware? It's not 'deprecated' - it was never documented, and therefore never a feature in the first place, and so I didn't see the need to implement it. We could of course re-add it, and document it. As it happens, doing so looks easy. I'm not sure why those arguments are not documented anywhere, either for cache_page or CacheMiddleware, that's why I'm reluctant to add this -- we would then be committing ourselves to supporting something that we might have wanted to remove. Putting specific support for it in cache_page would make it harder to remove. Does anyone else know if we want to expose all the arguments of CacheMiddleware in cache_page? I can easily add them if so. Regards, Luke -- "Smoking cures weight problems...eventually..." (Steven Wright) 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 -~--~~~~--~~--~--~---
Re: Proposal for 1.2: built-in logging with django.core.log
On Friday 09 October 2009 10:33:28 Simon Willison wrote: > For anyone interested in following this, I've just started a > (currently experimental) branch on GitHub: > > http://github.com/simonw/django/tree/logging Is there some easy way to see the patch? Preferably a link which will just show the diff between the latest of your branch and trunk, but failing that, a recipe of commands using git (for those of us who haven't bothered to learn git properly yet). Cheers, Luke -- "We may not return the affection of those who like us, but we always respect their good judgement." -- Libbie Fudim 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 -~--~~~~--~~--~--~---
Re: Proposal for 1.2: built-in logging with django.core.log
On Saturday 10 October 2009 07:31:31 Simon Willison wrote: > master: 314.442s > logging: 317.096s > > Since there's nothing at all in my logging code that will speed > anything up, I'm putting that down to chance (I did run the suite a > few times first to "warm up" the VM I used, but like I said, this > is pretty much the opposite of science). Those numbers are the time in seconds, right? In which case your branch is slower, not faster, as I think you are implying. It could still be down to chance, but its only 1% which would be an acceptable hit for me anyway. Luke -- "Where a person wishes to attract, they should always be ignorant." (Jane Austen) 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 -~--~~~~--~~--~--~---
lazy auth context processor
I want to fix #6552 (also #12031), and I've attached a patch that makes the auth context processor lazy. I'm pretty sure this is the right thing to do, but I'd like to check for any gotchas people might be aware of. Some behavioural changes: * user.get_and_delete_messages() will not be run if the page template doesn't iterate over 'messages'. If you had a view which set some messages, followed by a template (that used RequestContext) which didn't display messages, it would silently delete messages. Now, those messages will be displayed at a later point. I think this is a bug fix, not a problem. * the output of {{ user }} in a template will be different. I think this is acceptable, because only things like {{ user.username }} are actually useful, apart from when debugging. Other eyeballs on the patch would be welcome. Luke -- "You'll be glad to know, I'm going to donate all the snot I sneeze to hospitals for mucus transfusions." (Calvin and Hobbes) 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 -~--~~~~--~~--~--~---
Re: lazy auth context processor
On Wednesday 14 October 2009 15:32:11 Jacob Kaplan-Moss wrote: > On Wed, Oct 14, 2009 at 7:24 AM, Luke Plant wrote: > > I want to fix #6552 (also #12031), and I've attached a patch that > > makes the auth context processor lazy. > > The patch looks good to me. The approach is a bit hard to follow > what with the multiple types of lazyness involved, but it makes > enough sense to me. I'm fine with this going in. Did you see the second patch or the first? It now uses 'lazy' for PermWrapper, 'lazy + memoize' for messages (because User.get_and_delete_messages() is not idempotent), and ContextLazyObject for the User object. Still a bit of a mess, but it's the best I can come up with so far. For the moment, I think I'm going to keep ContextLazyObject private (I'll move it), because there are probably too many limitations and gotchas. If it gets used again internally and turns out to be useful/robust enough for public use, fine. Luke -- "You'll be glad to know, I'm going to donate all the snot I sneeze to hospitals for mucus transfusions." (Calvin and Hobbes) 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 -~--~~~~--~~--~--~---
Re: lazy auth context processor
On Wednesday 14 October 2009 15:54:25 Russell Keith-Magee wrote: > > * the output of {{ user }} in a template will be different. I > > think this is acceptable, because only things like {{ > > user.username }} are actually useful, apart from when debugging. > > I disagree. User.__unicode__() returns self.username, so {{ user }} > is a potentially useful (albeit non-specific) rendering technique > for user. OK, I'll fix that. I had thought that it returned "", but that's repr(), not unicode(). > My only other comment is about ContextLazyObject - is this really > something that is specific to Context objects? Although we don't > have any other use for it right now, it strikes me as a more > general functional utility for deferring access to an object, > rather than something specific for templates. At the moment it's pretty general - it's just a thin layer on top of LazyObject that allows a callable to be passed in. But I don't want to advertise it as a general solution for deferred access to attributes because: * LazyObject was only created to support dynamic settings really. (i.e. where you are not using a settings file). It might have lots of corner cases where it's really not appropriate to be used. * There might be custom fixes we need for this use case. For example, so far there is the need to add __unicode__. I'd like to be sure I can fix these bugs, which might be harder if people start using it for different things. Adding __unicode__ to LazyObject, for instance, might make debugging of dynamic settings more difficult * I think 'lazy' is a better solution in general. It just unfortunately is not easy to use in this case. If people find other uses for it, almost all the functionality is in LazyObject anyway, which is as generic as it sounds. Luke -- "You'll be glad to know, I'm going to donate all the snot I sneeze to hospitals for mucus transfusions." (Calvin and Hobbes) 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 -~--~~~~--~~--~--~---
Re: lazy auth context processor
On Wednesday 14 October 2009 16:39:31 Luke Plant wrote: > On Wednesday 14 October 2009 15:54:25 Russell Keith-Magee wrote: > > > * the output of {{ user }} in a template will be different. I > > > think this is acceptable, because only things like {{ > > > user.username }} are actually useful, apart from when > > > debugging. > > > > I disagree. User.__unicode__() returns self.username, so {{ user > > }} is a potentially useful (albeit non-specific) rendering > > technique for user. > > OK, I'll fix that. I had thought that it returned > "", but that's repr(), not unicode(). Just to point out, to anyone who cares or gets confused by my patch: There was a whole bunch of confusion in my original post. The output of {{ user }} was never actually threatened. I was mixing up unicode/repr/str in my testing, and assuming that they would work the same way. LazyObject as it currently stands [1] will proxy __unicode__ methods automatically, but not __repr__ and __str__, although it does not have any special casing for any of them. This is due to the fact that 'object' does not provide an implementation of __unicode__, but does for the other two, and __getattr__ is only triggered when that implementation is not found. We should probably add that special casing for at least str, possible unicode. Luke [1] http://code.djangoproject.com/browser/django/trunk/django/utils/functional.py?rev=9945#L255 -- You meet a lot of smart guys with stupid wives, but you almost never meet a smart woman with a stupid husband. (Erica Jong) 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 -~--~~~~--~~--~--~---
LazyObject fix - tiny backwards incompatible change
Hi all, There is a bit of cruft in LazyObject that I'd like to fix - 'get_all_members'. This seems to be left over from before it got separated from LazySettings. The method is *only* needed to support introspection, which is usually only used from the interactive prompt. There is a much better way of doing it - implement the __members__ property and the __dir__() method. This is the standard and future compatible way. I'll need to fix the few instances of where LazyObject is subclassed in Django itself. If anyone else has subclassed LazyObject, from now on the 'get_all_members' method of wrapped objects will be ignored. In most cases, this will have no effect, because the default __dir__() will do the right thing. In exceptional cases (where the wrapped object implements __getattr__()), introspection won't return all the attributes that it should. Upgrading is easy - if your wrapped object does not implement __getattr__(), just remove 'get_all_members()'. If it does, replace 'get_all_members' with '__dir__', and if using Python < 2.6, add: __members__ = property(lambda self: self.__dir__()) Any objections to this? I can only imagine that it will affect interactive usage at the Python prompt in a few obscure cases, if anything. But I could have missed something. Luke -- You meet a lot of smart guys with stupid wives, but you almost never meet a smart woman with a stupid husband. (Erica Jong) 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 -~--~~~~--~~--~--~---
shortcut proposal
Hi all, As a consequence of the proposed CSRF changes, we brought up wanting to add a shortcut like render_to_response that uses RequestContext, as a refinement, but didn't decide on anything. Ideally, we would have something as short as this: return render(request, "template_name.html", {'foo':'bar'}) Currently you can do this: return render_to_response("template_name.html", {'foo':'bar'}, context_instance=RequestContext(request)) This is a bit cumbersome, especially with the need to import RequestContext as well as render_to_response. My proposal: add 'request' as a keyword argument to this same function: return render_to_response("template_name.html", {'foo':'bar'}, request=request) It's slightly longer than the 'ideal' could be, but has some quite big advantages: * you only have one import whether you want to use RequestContext or Context or both. And it's an import most people will have already in existing code. * you don't have to *learn* a new function name or import * in the context of the tutorial, it works very well - no new import to add, just a small change which can be explained without even going into how RequestContext works or that it even exists. * 'render_to_response' is nice and explicit, and it's very difficult to shorten it without losing that. Given where we are right now, I think it's the best option so far. Comments? Luke -- You meet a lot of smart guys with stupid wives, but you almost never meet a smart woman with a stupid husband. (Erica Jong) 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 -~--~~~~--~~--~--~---
Re: Session/cookie based messages (#4604)
On Friday 16 October 2009 02:04:35 Tobias McNulty wrote: > On Thu, Oct 15, 2009 at 7:03 PM, SmileyChris wrote: > > If we are to slowly deprecate this functionality, I think that we > > should do effectively the same thing. > > Leave user messages as-is and use something like this as the > > default message storage: > > Just to make sure I understand this correctly, let me try to > summarize what this would mean: > > * the proposed app controls {{ messages }} and is responsible for > retrieving anything set in the Message model > > * old apps that call user.message_set.create will still get their > messages to the screen, assuming the project uses {{ messages }} in > the template > > * old apps that call get_and_delete_messages or iterate through > the Message.objects.all() manually will NOT see any messages that > were stored in the session or a cookie, but they will continue to > see messages created the old way > > * judging from your code, the new app will NOT store any messages > in the old Message model, it will only read them > > * the standard template code: > > {% if messages %} > > {% for message in messages %} > {{ message }} > {% endfor %} > > {% endif %} > > will continue to work untouched after the update. For those that > choose to leverage it, a {{message.level}} (or otherwise named > attribute) will also be available. > > * since we are tying ourselves to the {{ messages }} variable, > this probably means that the app should be called 'messages'. > There has been some hesitation about this. Are we in the clear > now that we have a potential upgrade path from the old API? Or is > there still concern about naming? I've been catching up on this and watching this evolve, and this proposal now looks solid IMO. I think you should call this 'messages' consistently throughout, in terms of the names of modules/variables/settings. If you need to changed some documentation regarding the existing message functionality, call that 'user messages' to distinguish it. With regard to the deprecation schedule, normally we add a PendingDeprecationWarning before the DeprecationWarning. If code in core/contrib continues to use user messages for 1.2, it will emit a PendingDeprecation, which is bad practice really - it's not fair to users to emit warnings that aren't their fault and that they can't fix, and it's not good for core code to be using something that we are saying everyone else shouldn't. I think this means that either the deprecation cycle would have to pushed back one (i.e. pending deprecation warning in 1.3, deprecation in 1.4, removed in 1.5), or core/contrib should be fixed by 1.2. I would strongly prefer the latter, and this would affect my vote: I don't want two messaging systems in Django, and if user messages are not deprecated, then we do have two systems. Luke -- You meet a lot of smart guys with stupid wives, but you almost never meet a smart woman with a stupid husband. (Erica Jong) 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 -~--~~~~--~~--~--~---
Re: shortcut proposal
On Friday 16 October 2009 11:01:16 Yuri Baburov wrote: > Well, generally, I use to think that everything related to "django > views in general" constantly lacks devs attention. > Like it's perfect since long time ago. Or it is just too boring > topic, or API stability contract tied their hands, or just > everyone get used to it... > Or everyone code their own renderer once on project start, and then > use that renderer everywhere, like I do. > I also think direct_to_response was one of these personal > renderers. And I believe no one really care of render_to_response > weirdness ;) Russell answered the direct_to_template() history, so I'll skip that bit... Your other ideas about views are interesting, and remind me of ideas of Simon Willison - see 'TemplateResponse' about half way down this message: http://groups.google.com/group/django-developers/msg/b1b3f8854b9ae2b1 To respond to your specific suggestion, there are plenty of problems adopting it as a global way of doing things. For example, what if a view function returns HttpResponses with different templates depending on the path through, or returns a different kind of response, like a HttpRedirectResponse? Your simple wrapping function will now break. What if it decides not to use a template at all? Or a completely different templating engine? By adopting LazyHttpResponse everywhere, you would effectively be saying that a HttpResponse is "a Django-like template and some context variables, plus some headers". But at the moment, a HttpResponse is not that, it is some content and some headers, nothing more. Yes, your LazyHttpResponse obeys the HttpResponse contract, but not vice-versa. If people rely on view functions returning LazyHttpResponse, any view which does not will be regarded as broken, or any decorator/wrapper that assumes a LazyHttpResponse will be considered broken. The Django devs don't think that everything to do with view functions is perfect, but I for one would be against changing the whole way they work. The basic definition of a view function: a callable that takes a HttpRequest (and possibly more arguments), and returns a HttpResponse of some kind. ...is, as I see it, exactly as it should be. There are no requirements about using our template engine, or using templates at all. How you return the value, and the exact type etc, is entirely up to you. Starting with that, you can always build other abstractions on top, and it isn't hard to do so. You immediately have the ability to wrap existing views that take keyword arguments. If you want a view function that has bits you need to customize, you can use a class based solution, with methods that can be overridden, like the admin does for instance. There are an infinite number of other things you can do, including your LazyHttpResponse etc., TemplateResponse etc. Some of these may be good enough to make it into core patterns. But I think it would be a very bad idea build these into the way Django works and make everyone pay for the complexity and overhead that only some people might need, or tie the definition of a view function to some other technology (like templates). Luke -- The probability of someone watching you is proportional to the stupidity of your action. 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 -~--~~~~--~~--~--~---
Re: shortcut proposal
On Friday 16 October 2009 13:39:10 Russell Keith-Magee wrote: > Interesting idea. I think the advantages you list are all on the > money. My only question is on the pathological case: > > render_to_response("template_name.html", {'foo','bar'}, > context_instance=Context(), request=request) > > What is the interpretation here? An error? ignore the request? > ignore the context? > > Introducing an API where there is a usage that is legal syntax but > ambiguous argument usage makes me slightly nervous - although, I > will admit the potential for accidental misuse here is small. I implemented this the other day, and it seemed like the only sensible thing to do was raise a ValueError if they try to supply both, rather than silently overwrite a value in **kwargs. The patch is essentially the following lines inserted into render_to_response: ... request = kwargs.pop('request', None) if request is not None: if 'context_instance' in kwargs: raise ValueError("blah blah blah") else: kwargs['context_instance'] = RequestContext(request) ... > 4) Add a completely new shortcut: > > bikeshed(request, *args, **kwargs) > > which does exactly what render_to_response does, but instantiates a > RequestContext. I've deliberately chosen a nonsense name - at the > moment, deciding if this is the solution we want is more important > than the actual name. I am inclined towards 4) as well. I would just *slightly* quibble that the name is not important - a name that makes sense and is memorable is very important for a shortcut. If I have to look it up, it's not a shortcut any more. That's why I think that modifying render_to_response rather than adding a new shortcut has a slight edge here - because people already know it. Also, it means that I can just use "M-x grep" in emacs to find all instances that can be updated, and I don't have to worry about import lines :-) Not that I don't have a complete test suite on all my projects that would catch any missing imports, of course... ;-) Hmm, for me this boils down to whether I'm feeling lazy or feeling like a perfectionist, option 4) is definitely cleaner... Luke -- The probability of someone watching you is proportional to the stupidity of your action. 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 -~--~~~~--~~--~--~---
Re: LazyObject fix - tiny backwards incompatible change
On Friday 16 October 2009 12:55:24 Russell Keith-Magee wrote: > What exactly is the motivation for this change? Is it addressing a > specific bug? Or just general cleanup? It's mainly general cleanup. I must have slipped through the net when LazyObject was refactored out -- a generic proxy class shouldn't be depending on a public method called 'get_all_members' Also, because it is a public method like that, and with a name like that, I can easily envisage someone with an object that has a method called 'get_all_members' that does something entirely different. If they wanted to wrap it with LazyObject, they now have a problem, and possibly a rather difficult to debug problem. They will file an angry bug report asking "why on earth isn't Django using the standard way of doing this?". Rather than deal with that problem in the future, it seems much better to deal with it now. A Google code search does not return any results for our LazyObject being used outside of Django itself (which may not mean much, I guess, but it's some indication). Personally, I think this is a much smaller backwards incompatibility than some of the others we've been happy to make (e.g. TransactionTestCase in 1.1) in terms of the likelihood of it affecting people. I think it's quite possible that *no-one* would even come across this issue if we did nothing, but it's probably much more likely that they would be bit as described above, than bit by me changing it, especially if I document it in release notes. Luke -- The probability of someone watching you is proportional to the stupidity of your action. 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 -~--~~~~--~~--~--~---
Re: shortcut proposal
Hi Yuri, I'm slightly confused about what were debating. In your first response, you said: > I strongly believe any provided django views (django contrib views, > your own views or 3rd-party views) should allow enhancing with > "logic-reusability-fu": > ... In the more recent one: You wrote: > Luke Plant wrote: > > The Django devs don't think that everything to do with view > > functions is perfect, but I for one would be against changing the > > whole way they work. The basic definition of a view function: > > > > a callable that takes a HttpRequest (and possibly more > > arguments), and returns a HttpResponse of some kind. > > > > ...is, as I see it, exactly as it should be. There are no > > requirements about using our template engine, or using templates > > at all. How you return the value, and the exact type etc, is > > entirely up to you. > > Lack of requirements is not a feature here, but IMHO a bug ;) (i.e. it would be best to *require* use of Django's template system for all views) But also you wrote: > I'm talking about providing more data *where* these views *were > tied already*, not *tying to template renderer*. > And please don't forget, that Django does the last with > render_to_response *now*. So is your proposal that all views must return an enhanced HttpResponse, or that builtin shortcuts like render_to_response, which already used Django's template system, should return an enhanced HttpResponse? The first proposal, IMO, is a non-starter, unless we are talking "Django 2000", but I still think it would be really bad idea even then. The second proposal is worth talking about I think. But I think you need a really concrete proposal. It opens up huge issues - the 'contract' of a view would then include the names of all variables in its output context. Some views already have that contract in effect - the ones that are designed to allow you to supply your own template - but many do not. We then have to think about whether this is the best pattern for allowing selective overriding of parts of what a view does - I suspect that messing around with the context dictionary afterwards is of very limited use, not to mention fragile, compared to a full class based method that splits the view up into methods that can be overridden as required. Luke -- The probability of someone watching you is proportional to the stupidity of your action. 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 -~--~~~~--~~--~--~---
Re: Session/cookie based messages (#4604)
On Thursday 22 October 2009 04:41:03 Tobias McNulty wrote: > In sticking with the staged deprecation policy, my vote would be: > * use compat_add_message() internally in 1.2. There is no need to > document this function as request.message_set.create will continue > to work. > * in 1.3, remove compat_add_message and raise an error if > 'messages' isn't enabled when you try to use the admin or > create_update generic views, etc. I think this is the right approach. The second branch of compat_add_message should raise a DeprecationWarning that indicates that the 'messages' app will be a dependency of the admin and generic views, since the user's code is going to break if they update to 1.3 without fixing their settings. Obviously the release notes for 1.2 and 1.3 need to detail this - you can start to add things to doc/releases/1.2-alpha.txt I've started to look at the code, and have some other minor suggestions, but not significant things to do with approach, so I'll e-mail you personally with them at some point. On the basis of what I've seen so far, I'm willing to change my vote on this to +1, which at the moment would make me the 'champion' for this feature, as no-one has voted -1. At the moment, the big areas that are lacking are: - user docs - code docs (I'm curious about things like EOFMessage, there is nothing to explain why it exists). - changes and complete tests for the auth context processor, including the different configurations that are allowed (with and without the messages app, for instance). I think the auth context processor should call the messages one, so that you get the same result whether you use one or the other or both. Luke -- Carpe post meridian! (I'm not a morning person.) 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 -~--~~~~--~~--~--~---
Re: default Model.all()?
On Thursday 22 October 2009 12:45:13 Waldemar Kornewald wrote: > Hi, just a small issue we always wondered about: > Why doesn't Model provide a simple .all() function which just maps > to self.objects.all() or self._default_manager.all()? Only few > models provide multiple managers and it's annoying to write > .objects everywhere. Is there a good reason to force people to > write ".objects" all the time? Remember that methods on Model will be available to instances of Model. Therefore it's really important not to pollute the namespace, which is shared with model field attributes. Therefore, we decided on a single standard way to distinguish 'table level' attributes from 'row level', and that is ".objects". Adding more methods to Model would only cause more potential conflicts with field names. Luke -- "Some people says that if you play a Windows XP install CD backwards you will hear demon voices commanding you to worship Satan. But that's nothing. If you play it forwards it will install Windows XP." 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 -~--~~~~--~~--~--~---
Re: default Model.all()?
On Thursday 22 October 2009 18:16:23 Philippe Raoult wrote: > While we're talking about that, is there a reason the default > manager isn't iterable (default mapping to .all()) ? > > This is not as trivial as it sounds, because it means template > writers need to be aware of which member of the class are managers > and which are plain methods returning lists or querysets. For > example: > > {% for comment in blogpost.comments_set.all %} > vs > {% for comment in blogpost.custom_comments_method %} > > There might be a good reason why it's like that, otherwise I'll > propose a patch... The reason is caching: if you made it iterable, you would have to throw away the results rather than store in a cache, because otherwise it would always return the same results; Model.objects.all() always returns a fresh QuerySet. It also wouldn't be thread safe. Luke -- "Some people says that if you play a Windows XP install CD backwards you will hear demon voices commanding you to worship Satan. But that's nothing. If you play it forwards it will install Windows XP." 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 -~--~~~~--~~--~--~---
Re: default Model.all()?
On Friday 23 October 2009 10:37:11 Philippe Raoult wrote: > I have the following shorcut in my code: > > class IterableManager(models.Manager): > use_for_related_fields = True > > def __iter__(self): > return self.all().__iter__() > > class CustomModel(models.Model): > objects = IterableManager() > > > I'm honestly not familiar with caching and threading and thus fail > to see how that breaks anything (vs using blogpost.comment_set.all > in a template). Maybe you could you enlighten me ? I think the above code is thread safe, but isn't very performance friendly. Suppose you write: mylist = CustomModel.objects and iterate over mylist twice in one request — it will call the DB twice. Had you written: mylist = CustomModel.objects.all() then when you iterate over mylist more than once, or evaluate it other ways (such as take its length), you only hit the DB once. If you try to fix that by adding a cache, then the cache will end up being permanent (i.e. until the Python process terminates), and it (probably) won't be thread safe either. Luke -- Clothes make the man. Naked people have little or no influence on society. -- Mark Twain 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 -~--~~~~--~~--~--~---
Re: Django 1.2 feature voting
On Saturday 24 October 2009 14:32:58 rjc wrote: > Schema migration is not an option, it is required for any > production code (we ship > a lot of code to out customer's site and regularly publish patches > that include schema > changes). You cannot make a site without ORM or schema migration, I > see both at the > same level. You mean that *you* cannot make a site without schema migration :-) Other people may get on just fine without it, or have strategies other than a Python based solution for dealing with it. And as other people have mentioned, there is nothing stopping you from having schema migration. > BTW, we use django evolution since south doesn't support python 2.3 > (again a lot of > enterprise code is stuck at RHEL4 which is py2.3) Python 2.3 support will be dropped for Django 1.2, so it sounds like you will be stuck with Django 1.1.X for other reasons. I was going to add that this was documented somewhere, but I can't find it anywhere. I think we did agree that it would be in the 1.1 release notes: http://groups.google.com/group/django-developers/msg/0888b1c8f2518059 But it's not there, which is an unfortunate oversight. Nonetheless, I don't think that oversight will change the plan. Python 2.4 was released nearly 5 years ago, so it's hardly unreasonable for us to drop support for 2.3. Luke -- Noise proves nothing. Often a hen who has merely laid an egg cackles as if she laid an asteroid. -- Mark Twain 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 -~--~~~~--~~--~--~---
Buildbot failure
Latest trunk is failing: http://buildbot.djangoproject.com/builders/django-trunk%20debian- lenny-python2.5-sqlite3/builds/220/steps/test/logs/stdio But I'm pretty sure that it's a bug in buildbot - the log refers to two files which shouldn't even exist in that revision of trunk (.../contrib/csrf/tests.py and .../contrib/csrf/context_processors.py), suggesting that there is a failure to clean out files that have been deleted in subversion. I've also run the complete test suite on my own machine (against sqlite) and don't see any failures. I don't know what to do about this, does anyone else? Luke -- "If we could just get everyone to close their eyes and visualise world peace for an hour, imagine how serene and quiet it would be until the looting started" -- Anon 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 -~--~~~~--~~--~--~---
CSRF changes - backwards incompatible
Hi all, For those following trunk, the CSRF changes have now landed (apart from Simon's proposed refinements). At first I thought this would be perfectly seamless, not requiring any immediate action, and it therefore didn't warrant a note to django- devs. However, there are circumstances when developers MUST update their code immediately to avoid breakage. If you have supplied custom templates to contrib views that accept POST requests (e.g. auth login etc.), the template may need updating. The steps needed are fully described in the docs, but in short: Inside all elements, add {% csrf_token %} That's it. Apologies to anyone who has already experience breakage because of this. This brings up another question - I don't know what our policy for backwards compatibility with respect to templates is. Obviously if you completely override an entire template for something like an admin page, you cannot expect everything to keep working — otherwise it would be impossible to add any new features to the admin. Also there is the problem of inheritance etc. — do we guarantee that the inheritance/include chain of all blocks will stay the same? Sometimes you need to move anything around — 'refactor' if you will — and a commitment not to change anything here would be painful (and it's not what we've done so far, although we have minimized unnecessary changes). How much do we consider those who have overridden templates by setting TEMPLATE_DIRS so that a custom template (e.g. admin/base_site.html) is chosen over the default one? It's a common pattern, although in some ways it could be considered a hack. Is it different for views that have explicit hooks for customisation (like a template_name keyword argument)? For the latter, I think the view function docstring should document everything that will be in the template context if we are making guarantees about compatibility. But that would not have avoided the current problem of needing to insert something extra. What about CSS/javascript? I know there have been a number of changes here which have caused breakage if people have overridden templates in certain ways, but it seems unreasonable to require the admin CSS to remain organized as it is now. The docs for this are in slight disarray — I can only find some obsolete docs which say that they don't apply any more. It's actually quite hard to define backwards compatibility in this area, much more so than with Python code. Luke -- Environmentalists are much too concerned with planet earth. Their geocentric attitude prevents them from seeing the greater picture -- lots of planets are much worse off than earth is. 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 -~--~~~~--~~--~--~---
Re: CSRF changes - backwards incompatible
On Tuesday 27 October 2009 13:07:14 rebus_ wrote: > And there are also some typos in guide: Cheers! Fixed now. After this patch, I won't be sad if I never have to type 'csrf' (or 'crsf') ever again :-) But unfortunately I will... Luke -- Environmentalists are much too concerned with planet earth. Their geocentric attitude prevents them from seeing the greater picture -- lots of planets are much worse off than earth is. 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 -~--~~~~--~~--~--~---
Re: CSRF changes - backwards incompatible
On Tuesday 27 October 2009 13:30:42 TheMaTrIx wrote: > I don't understand something here. csrf is stated to be a option > that needs to be enabled if you wish to use it for views, yet I > just ran a trunk sync and boom, django-pages-cms is busted, > without me enabling anything. The CSRF protection is enabled by default in MIDDLEWARE_CLASSES, for the unlikely scenario that you haven't set that setting. This is as documented. It is also enabled for all contrib views, as documented. That can break if you using views from contrib apps, and have supplied custom templates which don't have the CSRF token. Beyond that, I'll need more details! Luke -- Environmentalists are much too concerned with planet earth. Their geocentric attitude prevents them from seeing the greater picture -- lots of planets are much worse off than earth is. 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 -~--~~~~--~~--~--~---
Re: CSRF changes - backwards incompatible
On Tuesday 27 October 2009 13:03:14 Luke Plant wrote: > If you have supplied custom templates to contrib views that accept > POST requests (e.g. auth login etc.), the template may need > updating. The steps needed are fully described in the docs, but in > short: > > Inside all elements, add {% csrf_token %} > > That's it. It looks like this doesn't work for templates that are rendered by inclusion_tag(). That's because they don't inherit the context — something I didn't realise (probably because I haven't used inclusion_tag). There is a patch on http://code.djangoproject.com/ticket/12095 that tries to address this. I think the best solution is to fix inclusion_tag, rather than require any complex changes for developers — otherwise they will get fed up and decide "Django's CSRF sucks" and not use it. A nicer and more general way of fixing inclusion_tag would be welcome. Adding tests for this is a bit of a pain due to the way custom template tag libraries need to be in an 'app'. Luke -- Environmentalists are much too concerned with planet earth. Their geocentric attitude prevents them from seeing the greater picture -- lots of planets are much worse off than earth is. 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 -~--~~~~--~~--~--~---
Re: Advocacy for Email-01 (email backends)
On Sunday 25 October 2009 07:22:27 Russell Keith-Magee wrote: > I'm keen to see a resolution to this problem. To that end, I'm > interested in hearing specific criticisms or concerns with the > current backend proposal. I'm also interested in any alternate > approaches to this problem. I'm completely fine on this proposal. Apparently I voted '-0', I'm not sure precisely why as I didn't leave any specific comment, but I would change that to +0 now (I have no specific need for it myself, but can see that it's an important improvement for lots of people, and it can be implemented cleanly). I presume that the only built-in email backends would be the existing SMTP one, one for testing (which is just a code shuffle really), and a 'write to standard out' one for debugging (which is trivial and completely un-critical anyway), so this doesn't really add any maintenance burden to Django. Regards, Luke -- A common mistake that people make when trying to design something completely foolproof is to underestimate the ingenuity of complete fools. -- Douglas Adams 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 -~--~~~~--~~--~--~---
Re: ImproperlyConfigured: Error importing middleware django.middleware.csrf: "No module named csrf"
On Saturday 31 October 2009 01:04:09 vl4dt wrote: > I get this output from a newly created project with the latest > trunk: > > > Any suggestions? I suspect the python path is missing something, > also the server stops responding, just sits there and no further > requests are served. I started a new project in the normal way, and it works fine for me. It sounds like you've configured something wrong, or you're not actually looking at latest trunk. Maybe your command to start the project used a 'django-admin.py' that was from trunk, but when running the server it's looking at different Python sources. Luke --~--~-~--~~~---~--~~ 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 -~--~~~~--~~--~--~---
Re: CSRF changes - backwards incompatible
On Saturday 31 October 2009 03:04:28 Kegan Gan wrote: > How does the csrf_token affect TestCase.client.post() ? The token doesn't affect it directly. The HttpRequest class that TestCase uses is hacked so that the middleware and csrf_protect decorator don't actually reject requests which have the token. Basically, it disables the protection for your tests, to make testing of views much easier. All the cookies and tokens work just the same. Luke -- A common mistake that people make when trying to design something completely foolproof is to underestimate the ingenuity of complete fools. -- Douglas Adams 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 -~--~~~~--~~--~--~---
Documentation of backwards incompatible changes
Hi all, The online docs can easily give the impression that we have perfect compatibility with Django 1.0 — the 'Django over time' section on [1] has a link to 'Backwards-incompatible changes' [2], which contains pre-1.0 changes only, and a notice at the top saying you don't need to read it if your code is post 1.0. Also, the archive of release notes [3] incorrectly says that BackwardIncompatibleChanges is the place to look for those following trunk. It's a bit of a mess actually. The truth about backwards incompatibilities is slightly more complex, as the release notes of 1.1 show. But we don't currently have a good place to point people to. Release notes are a bit awkward, because it can be confusing trying to work out which you should read. If I'm upgrading from 1.0 to 1.2, do I need to dig out the notes for 1.1 as well? What about 1.1 RC1, beta 1, alpha 1? We also need a place for people following trunk to refer to. I'm proposing UpgradingNotes [4], which I've just started throwing together. It has everything you need to know in one place. It has notes for trunk at the top, and links to all the release notes you need to read, newest release first. This way, the contents of the top section of this page will be edited (mainly by committers, but possibly by other users) while there is any flux in trunk, and it will become part of the release notes when the next version is released. Also, the current FutureBackwardsIncompatibleChanges [5] is really just duplication of part of the 'Deprecation Timeline' docs [6]. I think the former should be removed. Thoughts? If other people think this is the right idea, links from the official docs to BackwardsIncompatibleChanges just need to be changed to UpgradingNotes. Luke [1] http://docs.djangoproject.com/en/dev/ [2] http://code.djangoproject.com/wiki/BackwardsIncompatibleChanges [3] http://docs.djangoproject.com/en/dev/releases/ [4] http://code.djangoproject.com/wiki/UpgradingNotes [5] http://code.djangoproject.com/wiki/FutureBackwardsIncompatibleChanges [6] http://docs.djangoproject.com/en/dev/internals/deprecation/ -- As Ralph Waldo Emerson once said, "I hate quotations." 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-develop...@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=.
Re: Documentation of backwards incompatible changes
On Saturday 14 November 2009 04:15:26 Russell Keith-Magee wrote: > We can add a stub set of 1.2-final release notes right now, with a > big header that reads "1.2 currently in development". Whenever > someone in the core adds a new feature, we add a section to the > 1.2 release docs. If that means backwards incompatibilities, then > make the appropriate notes. > > The home page would then link to the release notes index page; this > page will need a bit of a cleanup to make the notes of past > alpha/beta releases less prominent, but that page could do with a > cleanup anyway (since it has a reference to the backwards > incompatible wiki, too). That's fine with me, if we can reformat the releases index in a helpful way. My motivation was to have a single place that definitively lists all changes that people upgrading need to know about. For projects in maintenance mode, developers won't care about new features, and I don't think they should have to expend mental effort picking through a list of releases to work out which ones are relevant, or checking all of them just in case, or worrying that they missed some. Perhaps we could have two sections on that page — all release notes, and upgrading notes. Or, next to 'final' releases there could be an additional link to the 'backwards incompatible changes' section, along with some notes at the top explaining the page. Luke -- As Ralph Waldo Emerson once said, "I hate quotations." 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-develop...@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=.
Re: Doc building piece missing?
On Saturday 21 November 2009 11:54:40 Tim Chase wrote: > Just noticed this in [1] > > -- > csrf_tokenś > > New in Django 1.1.2: > > System Message: WARNING/2 > (/home/djangodocs/en/dev/ref/templates/builtins.txt) > > undefined label: releases-1.1.2 > -- > > > Looks like some tag/label needs to be created for the doc build > process to find it. If this needs a bug report to be filed, I > can do that too, but it may just be a fast "whoops" fix. I'm > guessing it's not the only instance in the docs, but I'd also > guess that the same fix should solve all of them. It is supposed to be 1.1.2, and at the moment it is the only instance in the docs. The circumstances which created the need for this are rare — we added a new 'feature' to the 1.1 branch, which in reality is a no-op, but will help to ease the transition from the 1.1.X branch to the 1.2.X branch. There is a comment on this bug: http://code.djangoproject.com/ticket/12130#comment:6 The problem is there are no 1.1.2 release notes yet. Also, if you put "versionchanged:: 1.2", our docs are set up to parse that as referring to the next version of Django, so it says "Development version". The same isn't true of 1.1.2. I don't know how this should be handled. Perhaps the easiest way is to start the tentative 1.1.2 release notes as has been done with the 1.2 notes. Luke -- "DO NOT DISTURB. I'm disturbed enough already." 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-develop...@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=.
smart if tag
Hi all, I started work replacing Django's if with the "smart-if" template tag by Chris Beaven ( http://www.djangosnippets.org/snippets/1350/ ) Of course, it is not as simple as it looks... 4 issues: 1) Handling non-existent variables == The current smart-if does not handle the case of non-existent variables like Django does. (For example, something like {% if foo or bar %} when one of foo or bar is not defined in the context.) To do this properly, I think the best approach would be something like "almost three valued logic", which would work as follows: 1) variables that don't exist are represented by Null 2) all operators return either True or False, never Null 3) for 'not', 'and' and 'or', Null effectively acts like False (so "not Null" == True, "Null or True" == True etc) 4) for all comparison operators, if either argument is Null then the result is False e.g. "Null == Null" is False "Null > 1" is False, "Null <= 1" is False, except "!=", which should return true if either value is Null. Any comments on this approach? I've started to implement this, and it works OK so far, just needs a bit more work. 2) TEMPLATE_STRING_IF_INVALID breaks everything === The smart 'if' tag allows for filters: {% if articles|length >= 5 %} To achieve this, it puts its arguments through FilterExpression. If settings.TEMPLATE_STRING_IS_INVALID is not an empty string, this means that: {% if foo %}blah{% endif %} will render 'blah' instead of '' if 'foo' is not in the context. This causes test failures at the moment. We could change the tests, but that is a backwards incompatibility, and it highlights a fundamental change in the behaviour of {% if %}, which basically makes TEMPLATE_STRING_IF_INVALID useless for even its stated purpose of debugging, because the logic of a template will now be changed. Personally, I care nothing for TEMPLATE_STRING_IF_INVALID, as there are other things that it breaks, and I never use it. But obviously I can't just think of myself here. If there was an easy solution that didn't change the behaviour of the if tag in this case that would be great, but I can't see one. 3) Behaviour of 'x and b or y' == Previously this was a syntax error. In Chris's code, it is fine and works like Python. I would vote for keeping the smart-if as it is, as I don't see the value of complicating the implementation to disallow something that could be useful sometimes. 4) Behaviour of 'not' = Current Django will interpret 'not' as a variable if there is no ambiguity e.g. {% if not %}blah{% endif %} This is not documented, but the behaviour is specified in detail in the tests, so changing it would be a backwards incompatibility. The smart-if is different (the above is a syntax error), and I suspect that making the smart-if behave like current Django could complicate things a lot. IMO, current Django behaviour here is complete misfeature! I don't know of any other language where keywords can be treated as variables if the keyword doesn't make sense in that position... Regards, Luke -- "He knows the way I take: when he has tried me, I shall come forth as gold" (Job 23:10). 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-develop...@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.
Re: Composite Primary/Foreign Keys
On Saturday 28 November 2009 07:30:46 Russell Keith-Magee wrote: > However - with that said, we're currently about 1 month out from > the deadline for adding new features to Django 1.2. The features > that are scheduled for that release were chosen back in October, > following several months of discussion and design plans. The core > team is currently focussed on developing the features that we want > in 1.2. Isn't this in our "Medium priority" list for 1.2 ? http://code.djangoproject.com/wiki/Version1.2Features Malcolm is down at a committer for this, but no-one as lieutenant, and it looks like someone voted -1. As far as I understand it, that means this is definitely a possibility for 1.2, but would require a lot of work in terms of a good design and implementation that all the core devs are happy with. Regards, Luke -- "He knows the way I take: when he has tried me, I shall come forth as gold" (Job 23:10). 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-develop...@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.
Re: smart if tag
On Monday 30 November 2009 11:18:13 Russell Keith-Magee wrote: > > 1) Handling non-existent variables > > == ... > > To do this properly, I think the best approach would be something > > like "almost three valued logic", which would work as follows: > > > > 1) variables that don't exist are represented by Null > > 2) all operators return either True or False, never Null > > 3) for 'not', 'and' and 'or', Null effectively acts like False > > (so "not Null" == True, "Null or True" == True etc) > > 4) for all comparison operators, if either argument is Null then > > the result is False e.g. "Null == Null" is False > > "Null > 1" is False, "Null <= 1" is False, except > > "!=", which should return true if either value is Null. > > > > Any comments on this approach? I've started to implement this, > > and it works OK so far, just needs a bit more work. > > I was with you right up until the equality comparisons (Null == > Null -> False etc). As noted by Alex, it conflicts with the answer > given by {% ifequal %}; it also differs with Python's behavior. Yeah, I hadn't thought of that. I don't think it's a case of differing with Python's behaviour, as we are making a deliberate choice to substitute a missing variable with None (or Null or whatever). You could argue that Python throws a NameError in this case. I was trying to think about what is intuitive. e.g. what should happen here if foo has not been supplied: {% if foo < 1 %} yes {% endif %} I think it is fairly counter-intuitive for a template author that you get 'yes' here. However, on further reflection, the "almost 3 valued logic" can be equally counter-intuitive, say with the following case: {% if foo < 1 %} yes {% else %} no {% endif %} It would render 'no' if foo was missing, but it might be quite surprising that if you decided to re-arrange the template by inverting the logical condition: {% if foo >= 1 %} no {% else %} yes {% endif %} then you get different behaviour. (i.e. 'yes' if foo was missing). So, I'm *now* suggesting that we convert everything missing to None. In fact, I've found that doing anything apart from this would be hard. (Alex, you were right about what Malcolm had done with FilterExpression, which resolves the problem with TEMPLATE_STRING_IF_INVALID, but this gives me None instead of '', and not information about whether the variable actually exists, which is a slightly different question. I didn't find this out earlier because most of the tests are against the 'IfParser' which is a subclass of 'TemplateIfParser', and works slightly differently, and because I wasn't thinking properly). Behaviour of 'not' - revisited == I think this leaves just the behaviour of 'not'. It is a bit more complicated than Alex suggested — the tests include "if not" and "if not not", and with the current behaviour you can do things like "if foo and not not" etc. In fact, you can do it (to some extent) with *any* of the keywords e.g. "if and", "if x and or". But not always e.g. "if not or and x" - that, illogically, is a syntax error. I suspect that allowing everything that was possible before will be extremely difficult, because the current behaviour is very badly defined for these edge cases — it works just fine if you do sensible things like not using variables called 'not', 'and' or 'or', but working out the rules for the exceptional cases will be very hard. I propose a backwards incompatibility here, it looks like this: The 'if' tag no longer accepts 'and', 'or' and 'not' as valid variable names. Previously that was allowed in some cases even though they were normally treated as keywords. Now, the keyword status is always enforced, and code like {% if not %} or {% if and %} will throw a TemplateSyntaxError. This would now be the only backwards incompatibility (discounting the possibility that people were relying on TemplateSyntaxError for things that are now legal). Patch = It can be tracked in my 'lp-smartif' branch here: http://bitbucket.org/spookylukey/django-trunk-lukeplant/ Currently various tests about 'not' are failing, and docs are not written. Chris: in the course of my attempted 'Null' behaviour changes, I changed your implementation slightly — I added explicit classes for Less, LessOrEqual, NotEqual and Not, pulling out the 'negate' behaviour from the indiv
Re: smart if tag
On Monday 30 November 2009 20:27:32 SmileyChris wrote: > On Dec 1, 6:08 am, Luke Plant wrote: > > Given that the 'Null' stuff has now been removed, we could > > move back to your way to reduce the code a bit, but I'm not sure > > it is worth it. > > I'd say reduce the code again. I've removed Less, LessOrEqual and NotEqual. I kept 'Not' as I think it's slightly cleaner than putting 'negate' into BaseCalc. It also removes the asymmetry of using 'Or' at one point in the code, which I found slightly puzzling. > And I think you missed adding some files in your repo? Doh! Added now, along with some docs. Regards, Luke -- "Humiliation: The harder you try, the dumber you look." (despair.com) 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-develop...@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.
Re: Session/cookie based messages (#4604)
On Monday 30 November 2009 15:18:03 Tobias McNulty wrote: > Your API suggestions sound fine; at least, as long as what messages > is doing import-wise isn't any worse than the rest of Django, it's > fine with me. :) > > Since this is a fairly significant change I would like to hear > feedback from others before moving forward, in case there's > something we missed. I updated the wiki to reflect the new > proposed API: I just converted some code, and the API seems good to me. I had one issue, which was the fact that I had some code which only had access to the User object, and not the request object, which had to be re- plumbed. The only place this change will be an issue is where it is difficult to do that (e.g. if your code is being called from somewhere else which you can't change). There is no reason this should hold us back - storing messages on the User is really a hack, and if people have a problem with not being able to do it any more (i.e. in Django 1.4), they can use other hacks like storing the request in threadlocal storage. > Once the final API changes have been made, I'll put up another > patch, hopefully before next week. I've been going through the code carefully now, and I've got a bunch of patches, mainly very minor (attached as a mercurial bundle, use "hg in fixes.bundle" to view, or if you give me commit rights to your repository I'll commit directly), and one significant issue: If I read the patch correctly, when a non-authenticated user uses a generic view (create/update/delete), they will get a failure if the messages framework is not installed. This is a big backwards incompatibility. The simplest solution is to catch the exception in the generic views. Another option is to add a 'fail_silently' keyword argument, defaulting to False, like the mail sending API. It would need to be added to all the shortcut functions, and used in the generic views and anywhere that you cannot rely on there being a current authenticated User. Docs should include something about it being intended for re- usable apps. I'm leaning towards the latter solution — adding exception handling makes a one-line call into a 4 line chunk of code, so the fail_silently argument does make it significantly easier for the framework to be used by re-usable apps. Regards, Luke -- "Humiliation: The harder you try, the dumber you look." (despair.com) 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-develop...@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. fixes.bundle Description: Binary data
Re: Single lines between top level classes & functions
On Tuesday 01 December 2009 23:06:45 SmileyChris wrote: > Prompted by Luke's whitespace removal patch for django-contrib- > messages, I thought I'd bring this up. > > The Django contributing guide says "Unless otherwise specified, > follow PEP 8." > > Should new code use two lines between top level classes & > functions, like PEP 8 suggests, or should the contributing guide > be updated? I had no idea it said that! I had got so used to Django's style that anything else just grated... Luke -- "Humiliation: The harder you try, the dumber you look." (despair.com) 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-develop...@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.
SMTPConnection DeprecationWarning
Hi all, With the addition of the configurable email backend functionality, SMTPConnection is raising a DeprecationWarning. Shouldn't this be a PendingDeprecationWarning according to our procedure? Also, I think it ought to be noted (along with a few other things) in the "Features deprecated in 1.2" section of docs/releases/1.2.txt Regards, Luke -- "I am going to let you move around more, just to break up the mahogany." (True Quotes From Induhviduals, Scott Adams) 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-develop...@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.
Re: SMTPConnection DeprecationWarning
On Thursday 03 December 2009 14:36:42 Russell Keith-Magee wrote: > > Also, I think it ought to be noted (along with a few other > > things) in the "Features deprecated in 1.2" section of > > docs/releases/1.2.txt > > Yes it should - but this time, in my defense, the 1.2 release notes > didn't exist at the time the feature was committed. The deprecation > notes (docs/internals/deprecation.txt) does list SMTPConnection as > a deprecated feature for 1.4. No probs, I've already fixed the release notes (there was an omission on my part as well). I'll fix the code for SMTPConnection too. > On a related note, we also need to go through the features that > will be deprecated in 1.3 and upgrade PendingDeprecationWarnings > to full DeprecationWarnings. I may as well do that too, while I'm at it. Luke -- "I am going to let you move around more, just to break up the mahogany." (True Quotes From Induhviduals, Scott Adams) 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-develop...@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.
Re: Session/cookie based messages (#4604)
On Thursday 03 December 2009 14:18:10 Waylan Limberg wrote: > Looking though this patch I couldn't help but notice the new > get_messages function. What if a project has neither the > MessageMiddleware nor the contrib.auth app enabled? Sure a reusable > app could catch the error, but that error will change once all > support for user.messages are removed in 1.4. Shouldn't the call > to user.get_and_delete_messages be wrapped in a check that > request.user exists like the new add_message function does? It is - the inline 'get_user()' function does effectively just that: <<< def get_messages(request): """ Returns the message storage on the request if it exists, otherwise returns user.message_set.all() as the old auth context processor did. """ if hasattr(request, '_messages'): return request._messages def get_user(): if hasattr(request, 'user'): return request.user else: from django.contrib.auth.models import AnonymousUser return AnonymousUser() return lazy(memoize(get_user().get_and_delete_messages, {}, 0), list)() >>> Luke -- "I am going to let you move around more, just to break up the mahogany." (True Quotes From Induhviduals, Scott Adams) 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-develop...@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.
Re: #7052 - Fixing serialization for contrib.contenttypes and contrib.auth
On Thursday 03 December 2009 15:33:44 Russell Keith-Magee wrote: > So, I've taken a different approach with this new patch. The new > approach is much simpler and more explicit than the last. Rather > than trying to embed a query into the serialization language, I've > taken a step back and looked at the problem a different way. > > If there is some mechanism that can be used to look up instances of > a model, then by definition, it must be a surrogate key of some > kind. Completely independent of serialization, it would be nifty > to be able to perform lookups based on this surrogate key. This approach seems good to me. I had a brief look at the patches on the ticket, and it was *much* easier to understand your draft patch than the alternative, which is a good sign, other things being equal. > Two possible objections: > > * It's a string-based serialization format. This means the > developer will need to write parsing code and a microsyntax to > handle composite surrogate keys (e.g., separating permission name > from content type with a pipe, and separating app_label from model > with a colon). How easy would it be to fix this? If you used a list of string values, instead of a single string, wouldn't all the serialization formats handle this with very little work? Without looking at the code, I imagine that JSON and YAML at least would. You are still forcing things to be to be converted to strings and back, but there are usually standard ways to do that. > * You can only define one surrogate key. Some models might have > more than one natural serialization. I agree that this limitation isn't serious. If you added support for multiple surrogate keys, you'd need to add methods to specify which one should be used, and thread that option through all the serialization code... I think the limitation could become more significant if other parts of Django started relying on these surrogate key methods, we should think about that if it comes up. > On a practical note - there is one failing test, which highlights > the one flaw in this scheme that I am aware of. There is still a > circular dependency problem - an object must be deserialized > before it can be referenced using a surrogate key. I presume this problem would exist with the alternative method on that ticket as well, right? Regards, Luke -- "I am going to let you move around more, just to break up the mahogany." (True Quotes From Induhviduals, Scott Adams) 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-develop...@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.
Re: Session/cookie based messages (#4604)
On Saturday 05 December 2009 13:40:40 Russell Keith-Magee wrote: > * I'd rather the AddMessageFailure have a more generic name (like > MessageFailure). I don't see any need to be task specific in the > exception class name. Another thing here we should think about: at the moment, fail_silently only silences the error for the messages framework not being installed. But there are a lot of other ways the messages backend could potentially fail. Should we silence those errors too? I would suggest not, or there should be a separate keyword argument for that. But this should be clear in the docs. > * I don't know why I didn't pick this up earlier, but the emerging > convention for pluggable backends is for the user to specify the > name of the module, and for the module to have a predictable class > name. i.e - each storage backend module should have a > "StorageEngine" class, and the user specifies the name of the > module. There is also an emerging convention to call the pluggable > bits 'backends', rather than picking a name like 'storage'. Net > result: the user should be configuring > MESSAGE_BACKEND="django.contrib.messages.backends.user_messages" > rather than > MESSAGE_STORAGE = > 'django.contrib.messages.storage.user_messages.LegacyFallbackStorag > e' Hmm, it is 'emerging' only in that several established subsystems use it (db, session, cache) and the e-mail stuff just added uses this convention. There are other established conventions in the code base, some of which are more recent IIRC: * that used by the file storage classes, file upload handlers and the authentication backend where the specific class is referred to. * that used by the template loaders, where the specific callable is referred to. I actually much prefer the explicit naming of a class/callable. Here are some arguments in favour of this method, some of which are stronger than others: * you can put multiple backends in one file if you want to, and you are not forced to duplicate imports or get creative with module names if you happen to have several backends that really belong in the same module. * it is much more obvious for someone who does not know the code, since it doesn't rely on a convention of a class with a certain name. If they see the setting for MESSAGE_STORAGE pointing to a class, they will immediately know where the implementation is, whereas if it points to a module and the module contains multiple classes, they will have to guess which class is the main one, or browse docs/source code. * it makes it less verbose and confusing if one backend refers to another (as happens with messages), because all the classes have different names, rather than having the same name. We currently have: from django.contrib.messages.storage.fallback import FallbackStorage ... class LegacyFallbackStorage(FallbackStorage): storage_classes = (UserMessagesStorage,) + FallbackStorage.storage_classes which would need to be changed to: from django.contrib.messages.backends import fallback ... class StorageEngine(fallback.StorageEngine): storage_classes = (UserMessageStorageEngine,) + fallback.StorageEngine.storage_classes or, using 'as' for the import: from django.contrib.messages.backends.fallback import StorageEngine \ as FallbackStorageEngine ... class StorageEngine(FallbackStorageEngine): storage_classes = (UserMessageStorageEngine,) + FallbackStorageEngine.storage_classes (the fallback storage module would have similar issues) The DB subsystem, which uses backends, is big and has multiple classes, so it makes sense for it to be referred to by module names. But if there is exactly one class which implements a feature, I don't see the sense in having a setting that refers to the containing module. There might be a future proofing argument here, but for smaller features it sounds like YAGNI to me. For the cache subsystem, the name of the module (without the 'path') is used as the protocol part of a URI-style setting, so it also makes sense for it to be short and lower case. As for the e-mail and session subsystems, IMO they are using the less preferable convention. Some of the disadvantages mentioned above don't exist for the e-mail backends, because they don't use each other like the messages ones do. Regards, Luke -- "Idiocy: Never underestimate the power of stupid people in large groups." (despair.com) 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-develop...@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.
Re: smart if tag
First, thanks for your review Russell, I appreciate that you've been doing tons of work on lots of things! On Saturday 05 December 2009 15:39:03 Russell Keith-Magee wrote: > Here's the review I promised. First the minor points: > > * Line 814 of templates/defaulttags.py has a wierd UTF-8 character > - an emdash, rather than an ASCII hyphen. I don't know what's > happening on your system, but this causes crashes for me > complaining about PEP-0263 problems. I did add a 'coding' line to fix that, but perhaps after you pulled my latest code. Anyway, it's not worth it so I removed it. > * The if-tag-not-02/05 tests that have been removed should be > retained, but be testing for the TemplateSyntaxError (just like the > if-tag-error tests do). If we're going to implement a new language > contract and introduce some reserved words, we should test that > that those words are actually reserved. There are actually other tests that check this, in the IfParser tests. I didn't see the usefulness of 5 different tests all checking that 'not' is a keyword, but renumbering the following tests would be worse, hence the note about why they were missing. Also, it makes more sense to put the syntax error tests together. > * I'm not wild about the docs for the if tag referring how > comparisons work "just like Python operators". We have historically > made a big deal of templates not being Python, and while we are > blurring the lines a little with this patch, I'd still like to > maintain the pretense. The preceding notes about logical operators > is a good example of how it should be done IMHO - it gives > examples of how and/or work without needing to resort to saying > "just like Python". Fixed now, hopefully, I've given examples for all the operators. > * The explanation of why {% if a > b > c %} doesn't work is a bit > vague. At the very least, it should have an example of the correct > implmentation - {% if a > b and b > c %}. Fixed now. > Now the major issues: There's only one that I found - the parsing > strategy for complex logical expressions. "A or B and C" is parsed > as "(A or B) and C", not the pythonic way "A or (B and C)" (giving > operator precedence to AND over OR). > > Personally, I found this very surprising. When I said in a previous > email that I didn't think it was worth hobbling the if statement to > prevent complex logical operations, I presumed it was going to be > replaced with a full parser. Historically, it hasn't been an issue > because we've only allowed one type of logical operator in an {% if > %}. I think I'd rather see this tradition continued rather than > allow mixing in a way that will be confusing to anyone with prior > programming experience. Good catch. I think it would be nicer to fix than just disallow, although it could be a substantial change to the way the IfParser class works. I don't think it is too much work, however, because the hard bit is tokenizing, which has already been done. I'm not likely to able to look at this before Tuesday. If anyone wants to look at it, I think the right approach is something like the following: http://effbot.org/zone/simple-top-down-parsing.htm (without the globals, obviously, they can be converted to instance variables/methods). I don't think trying to hack this on to the current IfParser would be a good idea. IfParser already has a slightly hacky way of handling the precedence of 'and' and 'or' relative to the other operators, and this is a good opportunity to replace that with something nicer. > Lastly, a controversial topic, but I think we need an answer on > this - whither {% ifequal %}? Should we be deprecating it? Given > the current level of usage of the ifequal/ifnotequal tags, this > seems excessive. Perhaps we should consider this for long-term > deprecation (i.e., flag it as a 2.0 deprecation)? Yes, I see no value in deprecating this until 2.0, given how much it is used. However, while I remember, there is this bug which we may need to revisit: http://code.djangoproject.com/ticket/10975 Essentially, you can't use {% block %} inside an {% if %} tag, but you can in an {% ifequal %} tag. Irrespective of which one is right, it seems unreasonable that {% if x == y %} behaves so differently from {% ifequal x y %} in this regard. (Actually, I didn't test that. Perhaps the behaviour of 'if' has changed with the smart if code). Regards, Luke -- "Idiocy: Never underestimate the power of stupid people in large groups." (despair.com) 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-develop...@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.
Re: smart if tag
On Saturday 05 December 2009 20:09:21 Luke Plant wrote: > I'm not likely to able to look at this before Tuesday. If anyone > wants to look at it, I think the right approach is something like > the following: > http://effbot.org/zone/simple-top-down-parsing.htm > (without the globals, obviously, they can be converted to instance > variables/methods). Cancel that - I unexpectedly had free time this evening, and I implemented this. It's a nice replacement I think (credit to Vaughan Pratt and Fredrik Lundh for the basic approach and Python implementation respectively). The new implementation is pretty much the same length as the old one, and hopefully easier to read, with a much smaller core parser. Precedence is specified directly, rather than implicitly, so it's much easier to check that it's the same as Python's. Latest patch attached to this e-mail. Regards, Luke -- "Idiocy: Never underestimate the power of stupid people in large groups." (despair.com) 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-develop...@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. diff -r 70e75e8cd224 django/template/defaulttags.py --- a/django/template/defaulttags.py Thu Dec 03 15:11:14 2009 + +++ b/django/template/defaulttags.py Sun Dec 06 01:04:04 2009 + @@ -11,6 +11,7 @@ from django.template import Node, NodeList, Template, Context, Variable from django.template import TemplateSyntaxError, VariableDoesNotExist, BLOCK_TAG_START, BLOCK_TAG_END, VARIABLE_TAG_START, VARIABLE_TAG_END, SINGLE_BRACE_START, SINGLE_BRACE_END, COMMENT_TAG_START, COMMENT_TAG_END from django.template import get_library, Library, InvalidTemplateLibrary +from django.template.smartif import IfParser, Literal from django.conf import settings from django.utils.encoding import smart_str, smart_unicode from django.utils.itercompat import groupby @@ -227,10 +228,9 @@ return self.nodelist_false.render(context) class IfNode(Node): -def __init__(self, bool_exprs, nodelist_true, nodelist_false, link_type): -self.bool_exprs = bool_exprs +def __init__(self, var, nodelist_true, nodelist_false=None): self.nodelist_true, self.nodelist_false = nodelist_true, nodelist_false -self.link_type = link_type +self.var = var def __repr__(self): return "" @@ -250,28 +250,11 @@ return nodes def render(self, context): -if self.link_type == IfNode.LinkTypes.or_: -for ifnot, bool_expr in self.bool_exprs: -try: -value = bool_expr.resolve(context, True) -except VariableDoesNotExist: -value = None -if (value and not ifnot) or (ifnot and not value): -return self.nodelist_true.render(context) +if self.var.resolve(context): +return self.nodelist_true.render(context) +if self.nodelist_false: return self.nodelist_false.render(context) -else: -for ifnot, bool_expr in self.bool_exprs: -try: -value = bool_expr.resolve(context, True) -except VariableDoesNotExist: -value = None -if not ((value and not ifnot) or (ifnot and not value)): -return self.nodelist_false.render(context) -return self.nodelist_true.render(context) - -class LinkTypes: -and_ = 0, -or_ = 1 +return '' class RegroupNode(Node): def __init__(self, target, expression, var_name): @@ -761,6 +744,27 @@ return do_ifequal(parser, token, True) ifnotequal = register.tag(ifnotequal) +class TemplateLiteral(Literal): +def __init__(self, value, text): +self.value = value +self.text = text # for better error messages + +def display(self): +return self.text + +def resolve(self, context): +return self.value.resolve(context, ignore_failures=True) + +class TemplateIfParser(IfParser): +error_class = TemplateSyntaxError + +def __init__(self, parser, *args, **kwargs): +self.template_parser = parser +return super(TemplateIfParser, self).__init__(*args, **kwargs) + +def create_var(self, value): +return TemplateLiteral(self.template_parser.compile_filter(value), value) + #...@register.tag(name="if") def do_if(parser, token): """ @@ -805,47 +809,21 @@ There are some athletes and absolutely no coaches. {% endif %} -``if`` tags do not allow ``and`` and ``or`` cla
Re: Session/cookie based messages (#4604)
On Sunday 06 December 2009 00:56:56 Russell Keith-Magee wrote: > > Really? Files definitely seem to be more on the "storage" side > > of things: > > > > http://code.djangoproject.com/browser/django/trunk/django/core/fi > >les/storage.py > > The problem we have here is that we have all sorts of > inconsistencies introduced by history. Yes, files and templates > use function/class level specifiers, but db, session, cache, and > email all use module level specifiers. Module level specifiers > have the majority at this point, and personally, I prefer the > module approach - it shortens the user configuration strings, and > it enforces good usage of modules. I would have to question "enforcing good usage of modules"! It enforces a *single* way of using modules, that is, one class per module. That is not necessarily 'good', it certainly isn't Pythonic in my experience, and it could well be bad. As such, it would be opinionated in a bad way - it forces other developers to have a proliferation of modules which might be very inconvenient. Why not just leave the decision in the hands of the people who are writing and organizing the code? We are all consenting adults here. In addition to the arguments in my other message, you also have the case where some third party writes a backend, and then realises they need another. So, they had: acme.messages.MessageStorage and just want to add acme.messages.SuperMessageStorage Instead, they are forced to move code around so each of these can live in its own module, or else have stub modules that serve no purpose except to appease the Django-gods. I agree that it would be good to be consistent if possible, but I would also like to see the better convention used, and the e-mail system could just as easily be changed as the messages system at this point, which would swing the majority the other way (it's actually currently "4-all" by my count, and that doesn't include the other configurable callables and classes, such as TEST_RUNNER, CSRF_FAILURE_VIEW, context processors, middlewares and view functions, which all use full paths). And #6262 could also go the other way too. Moving forward, we do need *both* methods. The database stuff involves various classes and submodules, requiring the top module to be specified and many callables and classes need to be specified using full dotted paths (unless you want a separate module for every context processor...). So then we need to decide which one to standardise on. Backwards compatibility is possible either way around - if we wanted to change, for example, the session storage setting to being a full path, it would be easy - if the path supplied gets you to a module, just look for 'SessionStorage' inside that module and you are done, otherwise use the thing you've got (or something similar). I think going the other way is also possible, with a similar level of fragility. Regards, Luke -- "Idiocy: Never underestimate the power of stupid people in large groups." (despair.com) 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-develop...@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.
Re: Conventions around plugable backends
Hi Russell, I was away and arrived back too late for this to matter, but for the sake of the archives, I just had a couple of comments: > Module-based configuration: > --- > > * The aesthetic of user-configuration options is cleaner because > configuration items are shorter and don't require duplication of > terms. e.g., django.core.cache.backends.locmem.LocMemCacheBackend > duplicates the 'locmem' bit for no real end-user gain unless you're > planning to have a non-locmem backend in the locmem package. One of the systems i.e. the authentication backend actually doesn't have this problem, due to putting all the provided backends in a single backends.py. We could do the same for other systems by adding imports. However, that would be a bad idea IMO - by having the classes separated, and without imports, you can reduce memory usage for the common case of having certain backends that are never used and therefore never loaded into memory. But the memory usage issue is another general argument in favour of your 'one module per backend' ideal, especially for large backends that you are unlikely to need more than one of. > I hope I've done a reasonable job of summarizing the two positions > in an unbiased way. Yes, thanks :-) I had another argument in favour of class based, which is that it makes the source code slightly more greppable from the point of view of someone new to Django and a Django project: as a non-Django Python developer, if I worked on a project that had a MessageStore class, I would normally find out where it is used by grepping for 'MessageStore'. With the module based system, that could come up with nothing, which is puzzling, but with the class based you will at least find a line in settings.py, which gives you something to google. Cheers, Luke -- "If something is hard, it's not worth doing." (Homer Simpson) 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-develop...@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.
Re: smart if tag
Hi all, I've now addressed everything in Russell's last e-mail, I think, so I think I'm pretty much good to go, apart from: 1) my last change rewrote a lot of IfParser, which was the heart of the patch. That means it probably needs looking at again! On the other hand, the new implementation is based on a much more general and more tested parser methodology, so it should hopefully be pretty solid. 2) what should be done about errors with operators? For the most part, it isn't much of an issue, as 'object' defines many of the magic methods involved. For 'in', however, you are likely to get TypeErrors if the object doesn't support it. Currently missing variables are handled gracefully, but {% if 'x' in 1 %} will get you a TypeError that is not caught. And there is still the possibility that other operators will cause errors. Our current policies don't quite handle this situation, AFAICS. For the case of 'in', catching any Exception and returning False could be defended - to use the above example, 1 is not an object that contains 'x', therefore it should return False. This would protect template authors against various exceptions. Or we could go with the policy for method calls, which is to check the exception for 'silent_variable_failure' attribute. This has the advantage that inappropriate use of 'in' by template authors, which is almost always going to be a mistake, will throw a loud error. The error message in this case isn't particularly obvious though: "TypeError: argument of type 'int' is not iterable" I'm leaning towards the latter. If a view changes so that, in the variables passed to the template, a container with a __contains__() method is replaced by one without, or by one with a buggy __contains__() method that throws exceptions, it's more useful and arguably more correct to get an exception than for the 'if' tag expression to return False. Regards, Luke -- "If something is hard, it's not worth doing." (Homer Simpson) 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-develop...@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.
QuerySet __contains__
Hi all, I discovered that QuerySet supports the 'in' operator: myset = Articles.objects.filter(foo=bar) if someobject in myset: # etc. The Python docs I could find imply (but don't state) that if there is no __contains__() method, but there is __getitem__(), then __getitem__() is called repeatedly, starting with zero, until IndexError, and it then does an 'in' on the resulting list. That would be very inefficient with a QuerySet, but thankfully it appears from stepping through a debugger that that doesn't happen if you define __iter__() (as we have on QuerySet) - it actually calls __iter__() until StopIteration, and does 'in' on that list. This means that the above code is pretty efficient - it does just one SQL query. However, it could be slightly more efficient in some cases, because the entire QuerySet._result_cache does not necessarily need to be filled - we can stop if we find a match, saving us the work of building Model objects that might not be needed. The plumbing to do this is all there already - we already optimize the __nonzero__ method in this way, by getting just the first result. It can of course be continued if the rest of the set is needed. So, I'm thinking of implementing the __contains__ method to do this optimization. Can anyone think of reasons why not? I think the 'in' operator is perfectly well defined for QuerySets. I don't see this as something that should be documented as a normal way to do membership tests, as there will often be much more efficient ways to do it using ORM methods. But, it is something that developers might do, and with the addition of the 'in' operator in the 'if' tag, it's something that template authors will be able to do. An explicit __contains__ method also protects us against changes in Python which could produce quite different performance. Regards, Luke -- "If something is hard, it's not worth doing." (Homer Simpson) 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-develop...@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.
Re: QuerySet __contains__
On Wednesday 09 December 2009 01:52:48 Jeremy Dunck wrote: > On Tue, Dec 8, 2009 at 7:22 PM, Luke Plant > wrote: ... > > > However, it could be slightly more efficient in some cases, > > because the entire QuerySet._result_cache does not necessarily > > need to be filled - we can stop if we find a match, saving us the > > work of building Model objects that might not be needed. > > You could also inspect the item to see if it's an instance of the > .model; if not, fast path False. > > Which leads to a question of edge-case semantics -- "1 in qs" > checking for primary key value. Good or bad? I think it's much better to leave __contains__ just doing an '==' check, rather than anything clever which could surprise people. (If the developer is doing a test which is always going to return False, it's a mistake, not an optimization opportunity). That would turn your question into "should QuerySet.__eq__() allow comparisons to integers?", which to me is an obvious "no", since you can't make it symmetric. Luke -- "If something is hard, it's not worth doing." (Homer Simpson) 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-develop...@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.
Re: smart if tag
On Wednesday 09 December 2009 12:43:04 Russell Keith-Magee wrote: > I've done another review, and it's looking pretty good. I have > three relatively minor comments: > > * There is what appears to be vestigial documentation around line > 346 of ref/templates/builtins.txt; it says evaluation order is > left-to-right, rather than operator precedence as described later > on. Thanks, changed. > * Most of the lambdas you have defined in smartif.py are already > available in the builtin operator module - e.g., operator.or_, > operator.and_, operator.gt, etc. Oh yeah, forgot about those! > * The handling of nodelist_false in IfNode isn't wrong (as in, it > doesn't generate wrong results), but it was contrary to my > expectations. I would have expected a simple "else: > self.nodelist_false.render()". Is there a particular reason for > this? I made the change in source and it didn't break any tests. > > The only two reasons I could come up with to explain this approach > were that it would be marginally faster to return '' rather than > render an empty nodelist, and/or an old-school mandate that every > function should have a return statement at the end. I'm not > necessarily saying this should be changed; but at the very least, > it warrants an explanatory comment as to why the obvious approach > isn't being used. I don't know the reason either, it's part of Chris' original patch, and I've simplified it now. > This is a hard one. Keeping with our tradition of disagreeing with > each other, I'm actually inclined to go the other way :-) > > I have two reasons. Firstly, I'm of the opinion that templates > shouldn't ever throw exceptions during rendering - they should fail > silently. Differentiating between False and Exception may be > difficult or inconvenient for the developer, but it's even more > inconvenient for the end user to see a 500 IMHO. You're right, and I was misreading the current code in thinking that it currently raises exceptions during rendering. While Variable and others raise exceptions, they are caught higher up. (There are just occasional places where builtins propagate exceptions, like {% url %}). > Secondly, while I can see the analog with silent_variable_failure, > it isn't quite the same circumstances. Most of the interesting > functions that will be invoked in templates will be internal to > Django or user-code, so we have almost full control over the > silent_variable_failure attribute. We can set > silent_variable_failure on DoesNotExist because it is internal to > Django, and it will behave nicely when errors occur in templates. > > However, to my reckoning, this isn't true for operators. Many, if > not most of the interesting use cases for operators will be > comparisons with literals, sets, lists, or other Python builtins - > which throw exceptions which we can't control. We may well honor a > silent_variable_failure attribute, but if it's near impossible to > actually set that attribute in practice, there isn't much point in > honoring it in the first place. Very good point. You convinced me easily, contrary to tradition this time :-) Thanks for the review again. Luke -- If you can't answer a man's arguments, all is not lost; you can still call him vile names. (Elbert Hubbard) 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-develop...@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.
Python version roadmap
Hi all, We need a section in our release notes about dropping support for Python 2.3. I was trying to write it, and I wanted to say "as announced in such & such a place", but I can't actually find that place. I know the decision was made somehow... Also, we should be adding any plans to drop 2.4, 2.5 etc into the internals/deprecation.txt documentation, and we ought to do that *before* 1.2 is released, to give as much warning as possible. I don't think we have ever come to a consensus on when to drop 2.4 support, though a year ago James Bennett made a well researched post suggesting that 1.2 should drop Python 2.3 support, 1.3 should drop Python 2.4 support, and 1.4 drop Python 2.5 support, so that we can then continue development against 2.6, with a Python 3.0 port. No-one disagreed with that plan, but then again few people responded. While James' post contained a lot of details on why dropping 2.3 is OK, and there was consensus that coping with Python 2.3 bugs is significant enough to slow development and hinder the road to 3.0, I don't recall any similar analysis with respect to 2.4. We really do need to be giving people warning if we are going to be dropping 2.4, and it needs to be in the version before it happens at the very least. We could possibly say something like "1.2.X is the last series *guaranteed* to support Python 2.4". That way, we can postpone the actual decision, while encouraging people to upgrade and giving us the freedom to drop it if that seems like a good idea. Luke -- "I imagine bugs and girls have a dim suspicion that nature played a cruel trick on them, but they lack the intelligence to really comprehend the magnitude of it." (Calvin and Hobbes) 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-develop...@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.
Re: why last_login in django.contrib.auth.models.User cannot be null?
On Tuesday 15 December 2009 23:10:23 Matt Schinckel wrote: > I came across one today: > contrib.auth.tokens.PasswordResetTokenGenerator > has a method _make_token_with_timestamp, which uses last_login to > create the > token. This means that if someone generates a password reset > request, the token > will be invalidated if that user then logs in. This could occur if > a person creates > password reset requests for a user that is not themself. That behaviour would occur whether or not last_login allowed nulls, and it is deliberate - the token is intended to be "use once", so logging in or setting the password changes the value of the token. However, the bigger issue is that I think changing the definition of this field really requires a migration mechanism (which would update existing databases), which we don't have. We really don't want the situation of bugs which are impossible to reproduce because they depend on installation and upgrade order. Luke -- "I imagine bugs and girls have a dim suspicion that nature played a cruel trick on them, but they lack the intelligence to really comprehend the magnitude of it." (Calvin and Hobbes) 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-develop...@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.
Re: preventing brute forcing passwords
Hi Tom, > Hello Everyone, > > I noticed that Django's contrib.auth doesn't provide a mechanism > for detecting a password brute force attack. This is necessary for > a couple projects I'm working so I have to implement some kind of > solution and would really like to do it in such a way that it > could get contributed back to the community. I'd like to propose > possible two variants to the way that system works and would > appreciate feedback. > > The first option is the more user customizable one, I propose a new > signal (possibly called LoginAttempt) which User.check_password() > would fire before returning so that users could implement their own > logging and lockout policies. This is likely what I will implement > first so that our internal implementation doesn't interfere with > future general implementations. The problem with signals is that they don't return values, and so the mechanism can't interact with the actual login process. It can only notice that something is going on and try to stop it by some external mechanism. > The second option, which is much more thorough, would add a > LoginAttemptLogEntry model which would look something like this: > > class LoginAttemptLogEntry(models.Model): > user = models.ForeignKey(User, null=True) > datetime = models.DateTime(auto_now_add=True) > success = models.BooleanField() > > Then either ModelBackend.authenticate() or User.check_password() > would log each login attempt using the LoginAttemptLogEntry. Any > user's account which had more than N (configurable in settings, > default to 5?) consecutive unsuccessful login attempts would get > locked. A successful password reset would then re-enable the > account. Personally, I really dislike this kind of thing. It makes it trivial to allow an attacker to lock out a user, just by knowing their username. The password reset mechanism has two disadvantages: 1) It won't work if the user's e-mail address is out of date. A user who has not forgotten their password should not be forced to keep their e-mail address up to date if they don't need it to be. 2) It's really inconvenient. This kind of security policy pushes the cost of the attack onto the user, who hasn't done anything wrong. So I would be against this as a default implementation, or as any implementation included in Django. (I understand that this is probably coming from customer requirements). Simon's rate limiting sounds like a better idea, since it doesn't require a reset and limits on IP address, so that the user isn't affected by the attacker's antics. As for integrating a solution, for either one of these it can already be done without a new signal, and a solution like Simon's cannot be done with signals). You simply wrap the view function in question in some code that implements your policy (possibly using a decorator like Simon's does). For protecting the admin, this can be done by using an AdminSite mixin that overrides the 'login' method. So if you want to contribute your work to the community, I would do it as an external project. But as other people have mentioned, check out the existing projects. Regards, Luke -- "I'm at peace with the world. I'm completely serene. I know why I was put here and why everything exists. I am here so everybody can do what I want. Once everybody accepts it, they'll be serene too." (Calvin and Hobbes) 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-develop...@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.
Re: preventing brute forcing passwords
On Friday 18 December 2009 01:44:05 Jeremy Dunck wrote: > On Thu, Dec 17, 2009 at 6:47 PM, Luke Plant > wrote: ... > > > The problem with signals is that they don't return values, and so > > the mechanism can't interact with the actual login process. It > > can only notice that something is going on and try to stop it by > > some external mechanism. > > Actually, they do return values -- an iterable of (receiver, > response) pairs. I had a suspicion that was the case, but couldn't find any examples of it in the docs. I guess the reason for the lack of examples is that in many cases it's kind of difficult to know what to do with the return values from a signal - what do you do with multiple values that contradict each other? But thanks for the correction. Luke -- "I'm at peace with the world. I'm completely serene. I know why I was put here and why everything exists. I am here so everybody can do what I want. Once everybody accepts it, they'll be serene too." (Calvin and Hobbes) 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-develop...@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.
Re: Feedback: Syndication feed views
On Sunday 20 December 2009 13:04:00 Ben Firshman wrote: > Okay, I've updated the ticket with a new patch: > > http://code.djangoproject.com/ticket/12403 > > I wasn't sure on the extent of the depreciation warnings required, > but hopefully that's the sort of thing needed. > > Thanks, > > Ben Some comments: * depreciated != deprecated. You mean the latter. There are various instances of the former in comments and the names of test classes. * Some docs need adding to the 1.2 release notes. * What is going to happen to feeds.Feed? Is it deprecated or not? (there is no PendingDeprecationWarning AFAICS). I for one have a lot of code that uses feeds.Feed directly, bypassing the 'high level' framework that provided the view function. If it is deprecated, what is the migration path? It really would need to be specified in painstaking detail. Other than that, I think the PendingDeprecationWarnings are OK. Regards, Luke -- I never hated a man enough to give him his diamonds back. (Zsa Zsa Gabor) 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-develop...@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.
Re: Call for feedback: django.utils.signed and signed cookies
On Monday 21 December 2009 11:43:19 Simon Willison wrote: > The code has documentation and unit tests. The documentation isn't > 100% complete - I need to improve the explanation of what signing > is and why it is useful and document the new COOKIE_SIGNER_BACKEND > setting which allows users to swap in their own cookie signing > behaviour should they need to. > > Most importantly though, the implementation has not yet been peer > reviewed by real cryptographers. With that in mind, would it be > appropriate to check this in before the 1.2 freeze? We would > certainly get the code reviewed before the final 1.2 release. Given that much of our existing code hasn't been reviewed by cryptographers, I don't think that necessarily needs to hold up the patch, but I agree it should be checked before 1.2 release. A few quick comments, I haven't reviewed at length: Rather than use 'settings.SECRET_KEY' as the default HMAC key, shouldn't we add a prefix so that any usage of SECRET_KEY can't be (potentially) used to attack other usages? We discussed this a while back. The new messages system uses: 'django.contrib.messages' + settings.SECRET_KEY for its HMAC key, which seems like a good convention to adopt. I don't know how strictly necessary it is, but I don't think it can hurt. I also think the "Cryptographic signing" documentation should clearly note that developers should be very careful not to expose functionality that would allow users to retrieve signatures for arbitrary strings. If they do, it will allow any other system which uses the same key for signing to be subverted. And an entry in the 1.2 release notes is needed. Luke -- I never hated a man enough to give him his diamonds back. (Zsa Zsa Gabor) 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-develop...@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.
Re: Feedback: Syndication feed views
On Monday 21 December 2009 17:20:33 Ben Firshman wrote: > > * What is going to happen to feeds.Feed? Is it deprecated or > > not? (there is no PendingDeprecationWarning AFAICS). > > I for one have a lot of code that uses feeds.Feed directly, > > bypassing the 'high level' framework that provided the view > > function. > > I wasn't sure whether to add a warning to that, since it is always > used through the feed() view. Though to be clear, I've added one. No, it's not only used through the feed() view. It can be used directly, and it has been fully documented as a public class. > > If it is deprecated, what is the migration path? It really > > would need to be specified in painstaking detail. > > I've documented it in some detail in the release notes. Is this > painstaking enough? Unless I'm missing something, it's not nearly there (but some of this may be the 'formal stuff' that Jacob doesn't mind being missing for now). First, views.Feed.get_object() takes different arguments to feeds.Feed.get_object(). This is pretty confusing, and really needs to be *much* clearer in the documentation (not just the release notes). The same goes for the get_feed() method. The release notes imply that I can just switch from subclassing feeds.Feed to view.Feed and change get_object(), and it will all work. I can't see how that is possible: - I might have code that calls the constructor of feeds.Feed (or a subclass) directly. But views.Feed has a different signature altogether, and feeds.Feed.__init__() does a bit of work that my own code is going to have to do once feeds.Feed disappears. Obviously that code is doing something that will have to be done differently with views.Feed. But how exactly? - I might have code that calls the get_feed() method of feeds.Feed (or a subclass) directly. That method does extra work compared to views.Feed.get_feed(). When feeds.Feed goes away, what does my code need to be changed to? What I'm looking for is *step by step* instructions on how to update my code, and a list of *all* the differences between views.Feed and feeds.Feed, since the former is supposed to be the replacement for the latter. I shouldn't have to look at *any* of the code or previous documentation to work out what has changed. *All* the hard work should have been done for me. This is about maintenance programmers, who have been given a promise of backwards compatibility, who just want their application to carry on working. I'm insisting on this, because until we see it, it's very difficult to work out how "backwards compatible" this really is, and whether we should deprecate feeds.Feed at all. Jacob was happy for this to go in without the formal stuff of documenting all this, so I guess we can punt the decision about whether to deprecate feeds.Feed or not, but we do need those docs at some point so we can see the implications of deprecating feeds.Feed. So for now, maybe just update the release notes so they don't say that the new LatestEntries class is identical to before, or a caveat like "if you are only using the high level feed framework (the feed() view)" or something. Regards, Luke -- I never hated a man enough to give him his diamonds back. (Zsa Zsa Gabor) 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-develop...@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.
r11964 and PendingDeprecationWarning
Hi all, Since changeset 11964 (i18n improvements), if you import django.forms.fields (directly or indirectly), you get a PendingDeprecationWarning. This is a problem, because it means that Django's own code is emitting a warning. It's especially a problem if you use a line like this: import warnings warnings.simplefilter("error", PendingDeprecationWarning) This is the simplest way to find out if your code is (directly or indirectly) using code that will be deprecated, and it's what I use in my django-admin.py command. But since this changeset, it now causes errors to be raised, which means I can't use this method to find deprecated code. Also, when the current code is changed from PendingDeprecationWarning to DeprecationWarning, then it will become noisy by default, at which point the current behaviour will become much more annoying. So we will need to fix this at *some* point. A simple way to fix it is to use django.utils.functional.lazy() for the DEFAULT_* globals. Patch attached. Using 'lazy' always has some potential complications, but given that all the types are container types (i.e. tuples), I wouldn't anticipate too many problems. If people were taking the repr() or str() of DEFAULT_DATE_INPUT_FORMATS, they will have a bug, but that doesn't seem likely outside of debugging, and otherwise it should work fine. Is there a reason you didn't do this Jannis/Marc? I missed this at the review stage. Russell mentioned adding the warnings, but said it might be hard to do (presumably because of the above problem). The alternative to my patch is to just remove the warnings. Regards, Luke -- I teleported home one night With Ron and Sid and Meg, Ron stole Meggie's heart away And I got Sidney's leg (THHGTTG) 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-develop...@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. Index: django/forms/fields.py === --- django/forms/fields.py (revision 12009) +++ django/forms/fields.py (working copy) @@ -19,6 +19,7 @@ from django.utils.translation import ugettext_lazy as _ from django.utils.encoding import smart_unicode, smart_str from django.utils.formats import get_format +from django.utils.functional import lazy from util import ErrorList, ValidationError from widgets import TextInput, PasswordInput, HiddenInput, MultipleHiddenInput, FileInput, CheckboxInput, Select, NullBooleanSelect, SelectMultiple, DateInput, DateTimeInput, TimeInput, SplitDateTimeWidget, SplitHiddenDateTimeWidget @@ -49,9 +50,9 @@ ) return getattr(formats, name) -DEFAULT_DATE_INPUT_FORMATS = en_format('DATE_INPUT_FORMATS') -DEFAULT_TIME_INPUT_FORMATS = en_format('TIME_INPUT_FORMATS') -DEFAULT_DATETIME_INPUT_FORMATS = en_format('DATETIME_INPUT_FORMATS') +DEFAULT_DATE_INPUT_FORMATS = lazy(lambda: en_format('DATE_INPUT_FORMATS'), tuple)() +DEFAULT_TIME_INPUT_FORMATS = lazy(lambda: en_format('TIME_INPUT_FORMATS'), tuple)() +DEFAULT_DATETIME_INPUT_FORMATS = lazy(lambda: en_format('DATETIME_INPUT_FORMATS'), tuple)() class Field(object): widget = TextInput # Default widget to use when rendering this type of Field.
Re: safe characters used in iri_to_uri (#12445)
Hi Gary, I agree with the your proposals. I've got a few concerns with the current code: In django/utils/encoding.py: function iri_to_uri <<< # The list of safe characters here is constructed from the # printable ASCII characters that are not explicitly excluded # by the list at the end of section 3.1 of RFC 3987. if iri is None: return iri return urllib.quote(smart_str(iri), safe='/#%[]=:;$&()+,!?*') >>> First, I can't find any list at the end of section 3.1 of RFC 3987, unless it's talking about the paragraph starting "Systems accepting IRIs MAY also deal with..." which only lists a few characters which should not be converted, and not the list given in the code. Second, the algorithm given in that section is described very differently, and it's very hard to see that they are doing the same thing. Hence this bug. However, I can't actually come up with a nicer solution, and one that is equally fast is probably even harder. So, +1 to changing this, as well as some fixes to the comments in the code. Luke -- I teleported home one night With Ron and Sid and Meg, Ron stole Meggie's heart away And I got Sidney's leg (THHGTTG) 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-develop...@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.
Re: An idea to eliminate {% csrf token %}
Hi Wim, Your suggestion sounds something like Simon's SafeForm. While some elements of that proposal may end up in Django, it turns out that implementing SafeForm as the default solution requires *bigger* changes to existing code than adding the csrf_token, because you would need to pass additional info from the request to Form for it to be able to do the CSRF checks. It also only works if you are using Django Forms and rendering them via {{ form }} (instead of field-by- field, for instance). If you want to see how we got to where we are, have a look at this thread: http://groups.google.com/group/django- developers/browse_thread/thread/3d2dc750082103dc/f3beb18c27fb7152 (OK, that was a nasty trick - the thread is huge, we discussed this nearly to death, but that's why I'm not going to repeat all the arguments here). Also, you can use CsrfResponseMiddleware as an interim measure to stop your code from breaking, so the change to require csrf_token isn't quite so bad. Thanks, Luke -- "It is a truth universally acknowledged, that a single man in possession of a good fortune, must be in want of a wife." (Jane Austen) 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-develop...@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.
Re: An idea to eliminate {% csrf token %}
On Friday 01 January 2010 16:45:19 Wim Feijen wrote: > I am not talking about SafeForm. I am sorry I wasn't clear before, > but in fact, what I want to propose is to include the lines: > name='csrfmiddlewaretoken' value='1234567890abcdef etc' /> > by default when rendering a form with {{ form }}. > > Would that be possible? I know of several unwanted side-effects, > which I believe we will be able to deal with using the reasoning > in my first post, but please correct me if I overlooked any > loopholes. It's not possible, unless you pass the request object to the Form instance, which requires changing the API for Form to something like SafeForm. That's the problem. Luke -- "I washed a sock. Then I put it in the dryer. When I took it out, it was gone." (Steven Wright) 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-develop...@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.
Re: Design and code review requested for Django string signing / signed cookies
On Monday 04 January 2010 21:45:41 jcampbell1 wrote: > I am not that familiar with your framework, but I think a signed > cookie should use http only cookies by default. There is no valid > reason for a script to read a cookie that it can't verify. http > only cookies significantly decrease the surface area of XSS > attacks. I can think of various circumstances where it might be useful and harmless. For example, if the signed cookie stored the user's login name, and some client side javascript used the login name for some convenience feature, like highlighting the login name wherever it appeared on the page. To generalise, the issue of using HttpOnly cookies is orthogonal to whether they are signed or not, because the value of the cookie can be used in multiple ways, not all of which will depend on the value being verified. Luke -- "Love is like an hourglass, with the heart filling up as the brain empties." 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-develop...@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.
Re: r11964 and PendingDeprecationWarning
pinging Jannis - did you see this? Can you have a look at my suggestion? Cheers, Luke -- "Love is like an hourglass, with the heart filling up as the brain empties." 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-develop...@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.
Re: r11964 and PendingDeprecationWarning
On Tuesday 05 January 2010 00:40:42 Jannis Leidel wrote: > Maby just an odd idea but what happens if someone monkey patched > them as lists (e.g. because the input formats weren't configurable > until now)? Would lazy() handle this automatically given list and > tuples are both iterables or should we pass an additional > resultclass, like > > DEFAULT_DATE_INPUT_FORMATS = lazy(lambda: > en_format('DATE_INPUT_FORMATS'), tuple, list)() > DEFAULT_TIME_INPUT_FORMATS = lazy(lambda: > en_format('TIME_INPUT_FORMATS'), tuple, list)() > DEFAULT_DATETIME_INPUT_FORMATS = lazy(lambda: > en_format('DATETIME_INPUT_FORMATS'), tuple, list)() Personally I wouldn't worry about people who are monkey patching - they ought to expect things to break. But I don't think this addition will harm either. Luke -- "Love is like an hourglass, with the heart filling up as the brain empties." 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-develop...@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.
Re: Design and code review requested for Django string signing / signed cookies
On Tuesday 05 January 2010 16:53:17 Elias Torres wrote: > Simon, > > I'm not a security expert by any means, but I really the fact that > you're making use of HMACs in your design. I will ask a good friend > (Ben Adida) who's really an expert on the subject to see if your > paranoia on adding a salt and hashing the key helps you in any way. > My intuition says if the salt will be stored and made available to > anyone with access to the DB, I'm not sure it will make much of a > difference. HMAC takes care of the prefix/suffix attacks already. The point of the 'salt' is to make it easy to use unique keys. Otherwise, one use of HMAC with SECRET_KEY in a web app could be trivially used to compromise another usage. For example, suppose that, on a social network, the user can specify the username of someone that they nominate as a 'best friend' . This value might be stored in a signed cookie. If we use SECRET_KEY without salt as the HMAC key, the user then has access to the value HMAC(SECRET_KEY, some_username). But suppose another signed cookie is used to store the username when a user logs in (as opposed to using a session). The value will be HMAC(SECRET_KEY, users_username). Since an attacker can trivially get hold of HMAC(SECRET_KEY, somone_elses_username), they can log in as anyone they like. 'Salt' in the key is to protect against that. The signed cookie implementation, for example, uses the cookie *name* as part of the salt, so that all cookies have their own key. The salt is not a secret, so doesn't provide any additional protection against brute force attacks, but it isn't meant to. Luke -- "Making it up? Why should I want to make anything up? Life's bad enough as it is without wanting to invent any more of it." (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-develop...@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.
Re: Possible contrib.humanize addition
On Tuesday 05 January 2010 21:24:13 harrym wrote: > I'm working a templatetag that determines whether to use 'a' or > 'an' in front of English words. My particular use case for this is > in a tumblelog app I'm developing - many different types of entry > may be added (link, html, quote, etc), and I'm linking to the 'Add > a[n] entry' pages by iterating over the different types. > Would this be considered a useful addition to contrib.humanize? Hmm, can it handle the following? an honest man a history book an historical book (debatable) My gut instinct is that it's not possible to work this out programmatically. When it comes to other languages, I imagine it's going to be even harder (if it's possible to get harder than 'impossible'), because you have things like gender and case to worry about, which certainly cannot be worked out by an algorithm. To give some examples, in French, the choice is between 'un' and 'une', depending on whether the word is masculine or feminine. In Greek, the choice is between ̔εις, ̔ενα, ̔ενος, ̔ενι, μια, μιαν, μιας, μια, ̔εν, ̔εν, ̔ενος, ̔ενι, depending on whether the word is masculine, feminine or neuter, and in nominative, accusative, genitive or dative case. Although in many cases you would probably omit the article altogether - the above words often mean "one" rather than "a". (That's NT Koine Greek, it might be different/simpler/more complicated in modern Greek). I imagine there are plenty of languages where this gets even worse, violating almost every assumption you don't even know you are making (like whether the article comes before or after or in the middle, or exists at all, etc. etc.) To summarise: if I were you, I would give up now. Luke -- "Mediocrity: It takes a lot less time, and most people don't realise until it's too late." (despair.com) 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-develop...@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.
Re: Design and code review requested for Django string signing / signed cookies
On Wednesday 06 January 2010 04:24:15 Elias Torres wrote: > Thanks Luke for your explanation. I think I have learned something > here in terms of my own application security independent of > Django's multi-app environment. Basically, you're reminding me > that as an application, I must be careful to not sign any random > string for a user, because it can be re-used for another purpose. > Therefore, we include the 'purpose', salt, or namespace in the > process. One thing I would like to ask though, is whether the salt > needs to be part of the key or the value? If you look at > TimestampSigner, the timestamp goes as part of the value. I think > the same can be done with the name of the cookie. In other words > you can always use a method like _cookie_signature as in Tornado's > implementation [1] and always pass two parts: cookie name and > cookie value. Technically, as long as your SECRET_KEY is > protected, there should not be a need to creating multiple signing > keys, especially if the data is not secret. > > def _cookie_signature(self, *parts): > hash = hmac.new(SECRET_KEY, digestmod=hashlib.sha1) > for part in parts: hash.update(part) > return hash.hexdigest() This is equivalent to: hmac.new(SECRET_KEY, digestmod=hashlib.sha1).update("".join(parts)).hexdigest() With this, one problem is that accidental collisions between different parts of code are easier. Consider two cookies: Cookie 1, name = "use", value = arbitrary string set by user Cookie 2, name = "user_id", value = server assigned once they've logged in correctly. By supplying "r_id123" as the value for cookie 1, I can forge a user_id=123 cookie. If some validation is imposed on cookie 1, it might still be possible to manipulate the system such that "r_id123" is a valid choice, and the exploit would still work. Actually, the implementation in Tornado does not include the cookie name in the signature at all, making it even more vulnerable to this kind of attack. So that would be my defence of why it's better to put the "purpose" namespace into the key, rather than the value, in the context of HMAC. I'm not an expert though. This is one of the tricky things with security - it's never just a case of "use this API" - what you feed into the API is always critical. > Any thoughts on Django's auth using HMAC besides md5 and sha1 > hashing alone? It sounds like a good idea, I'm not aware of any particular problems with our current implementation that would make this a priority change. Luke -- "Mediocrity: It takes a lot less time, and most people don't realise until it's too late." (despair.com) 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-develop...@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.
Re: Design and code review requested for Django string signing / signed cookies
On Wednesday 06 January 2010 17:12:29 Elias Torres wrote: > > So that would be my defence of why it's better to put the > > "purpose" namespace into the key, rather than the value, in the > > context of HMAC. I'm not an expert though. > > Can a separator solve that issue? In that instance, yes. I'm wary of other applications of HMAC producing loopholes in which the user provides the separator as part as the value being signed, and is able to generate the same string. In Tornado, they are suggesting have a separate key for signing cookies, in which case just signing "name=value" should be enough (provided the developer doesn't do something silly like make "=" part of the name of the cookie). Luke -- "Mediocrity: It takes a lot less time, and most people don't realise until it's too late." (despair.com) 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-develop...@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.
Re: user_passes_test decorator changes in 1.2
On Thursday 07 January 2010 00:12:08 gaz wrote: > However I thought I'd drop a line here since this isn't in the > backwards incompatible changes listed for 1.2 (I guess I'm possibly > already playing with voodoo, view_func isn't really documented so > it's my own fault I guess). Yes, that definitely falls into the category of relying on an implementation detail, rather than something that should be mentioned as a backwards incompatibility. At the level of inspecting code objects (which is essentially what your code was doing), almost any change is backwards incompatible. 'view_func' is not only not documented, it is a member of a class which is private and marked as such - _CheckLogin. The relevant changesets are: http://code.djangoproject.com/changeset/11587 http://code.djangoproject.com/changeset/11586 Between them, the '_CheckLogin' object was replaced with a more general method. That may enable you to find a solution to your problem. Personally, I'd use this as an opportunity to find a more robust way of getting that information to the template tag :-) Luke -- "Mediocrity: It takes a lot less time, and most people don't realise until it's too late." (despair.com) 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-develop...@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.
Re: Call for comment: #5390 Many To Many signals
On Friday 08 January 2010 12:04:27 Russell Keith-Magee wrote: > Any feedback on this design? Just a few things: 1) In assessing performance impact, people need to be clear that there is just one signal handler for the whole project, like for the post_save signal etc. Adding a handler will affect every m2m operation. One possibly way to reduce the impact slightly would be to have 3 signals - m2m_add, m2m_delete, m2m_clear. A handler attached to one would then produce no impact on the others being called. I'm not sure it is worth it though, especially since a handler will often need to hand all three situations, in which case the performance benefit is cancelled out, and developers will need to attach the handler three times. 2) What happens with 'reverse' when the m2m is to 'self'? I think your code always generates 'reverse=False' in this case, since the target of the relation is always the same model. This hurts my head to think about in the abstract, but I think it needs to distinguish between forward and reverse for non-symmetrical models. For example, given: class Person(Model): fans = ManyToManyField('self', related_name='favorites', symmetrical=False) friends = ManyToManyField('self') p1 = Person.create(...); p2 = Person.create(...) The symmetrical 'friends' relation obviously should always have 'reverse=False'. But I think this call: p1.fans.add(p2) needs to send a different signal than p1.favorites.add(p2) and with your patch it wouldn't. But my brain doesn't seem to be functioning properly today, I may have made a mistake. Luke -- "My capacity for happiness you could fit into a matchbox without taking out the matches first." (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-develop...@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.
DB optimization docs
Hi all, I was prompted by this post: http://it.toolbox.com/blogs/database-soup/stuff-id-love-to-see-from- django-36278 to add some notes about some DB access optimizations (essentially the things I mentioned in my comment on that page), but then thought that even if I add them, people are unlikely to find them. It's also not possible to put admonitions everywhere in the docs to stop people doing potentially expensive things. Rather, we need a list of tips for someone looking to optimize DB queries. It could link to all the relevant documentation (select_related() etc), as well as having misc other tips. Good idea/bad idea? At the moment, the information is mainly all there, but spread out in many places - I think we need an "optimize DB access" topic. Luke -- "My capacity for happiness you could fit into a matchbox without taking out the matches first." (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-develop...@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.
Re: DB optimization docs
On Friday 08 January 2010 14:12:56 Mat Clayton wrote: > As someone going through this pain right now, this would be very > helpful. > > Mat I don't know when I'll have more time to work on this, but I've committed the beginnings of draft of docs/topics/db/optimization.txt to my hg repos. http://bitbucket.org/spookylukey/django-trunk- lukeplant/src/tip/docs/topics/db/optimization.txt It needs to be hyperlinked to all the right bits, and filled out. But it might be of some help to you in its current state. If anyone wants to take this on as a mini-project, it would be gratefully received :-) I'm not planning on doing any more on this for at least a few days. Luke -- "My capacity for happiness you could fit into a matchbox without taking out the matches first." (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-develop...@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.
Re: DB optimization docs
I've added these docs now, or at least a good first stab at them. Suggestions for improvements are welcome, patches are more welcome, as always :-) I backported to 1.1.X, and tried to remove any anachronisms. Regards, Luke -- "Outside of a dog, a book is a man's best friend... inside of a dog, it's too dark to read." 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-develop...@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.
Re: AnonymousUser has_perm/has_module_perms function check authentication backends
Hi Harro, > Hmm I guess I'll just have to keep on hacking django then.. > because that 1% case is something I keep running into for every > project in one way or another. > And if it was designed for most apps, why was the row level > permission bits added? It's useless without simply always being > able to call request.user.has_perm('permission', obj) Despite a slight overstatement in that last paragraph, your argument seems pretty good to me. The whole point of these methods is to allow custom backends to implement their own logic, so obviously it is pointless to arbitrarily limit it. The only downside is that custom backends need to be able to cope with anonymous users being passed to the has_perm methods, but that is already well catered for with the is_anonymous() method. It is also better to make this change before 1.2 lands, otherwise we have a slight backwards incompatibility if we wanted to do it in the future (backends could break if they unexpectedly got an AnonymousUser instead of a User). Anyone got a good reason reason why this *shouldn't* go in? I'm +1 on committing. Some small tweaks to the patch will help: - 'set()' is nicer than 'set([])' - in topics/auth.html, it would be nice to document that the backend should be able to cope with anonymous users being passed to has_perm(). Luke -- "Pessimism: Every dark cloud has a silver lining, but lightning kills hundreds of people each year trying to find it." (despair.com) 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-develop...@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.