Re: CSRF proposal and patch ready for review

2009-08-29 Thread Luke Plant

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

2009-08-31 Thread Luke Plant
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

2009-08-31 Thread Luke Plant

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

2009-08-31 Thread Luke Plant
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

2009-09-04 Thread Luke Plant

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

2009-09-10 Thread Luke Plant

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

2009-09-14 Thread Luke Plant

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

2009-09-15 Thread Luke Plant

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

2009-09-16 Thread Luke Plant

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

2009-09-17 Thread Luke Plant
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

2009-09-19 Thread Luke Plant

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

2009-09-19 Thread Luke Plant

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

2009-09-21 Thread Luke Plant

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

2009-09-21 Thread Luke Plant

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

2009-09-21 Thread Luke Plant

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

2009-09-21 Thread Luke Plant

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

2009-09-22 Thread Luke Plant

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

2009-09-22 Thread Luke Plant

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

2009-09-22 Thread Luke Plant

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

2009-09-22 Thread Luke Plant

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

2009-09-23 Thread Luke Plant

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

2009-09-23 Thread Luke Plant

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

2009-09-23 Thread Luke Plant

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

2009-09-25 Thread Luke Plant

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

2009-09-25 Thread Luke Plant

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?

2009-09-26 Thread Luke Plant

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?

2009-09-26 Thread Luke Plant

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

2009-09-26 Thread Luke Plant

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

2009-10-09 Thread Luke Plant

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

2009-10-10 Thread Luke Plant

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

2009-10-14 Thread Luke Plant

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

2009-10-14 Thread Luke Plant

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

2009-10-14 Thread Luke Plant

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

2009-10-15 Thread Luke Plant

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

2009-10-15 Thread Luke Plant

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

2009-10-15 Thread Luke Plant

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)

2009-10-16 Thread Luke Plant

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

2009-10-16 Thread Luke Plant

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

2009-10-16 Thread Luke Plant

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

2009-10-16 Thread Luke Plant

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

2009-10-16 Thread Luke Plant

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)

2009-10-22 Thread Luke Plant

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()?

2009-10-22 Thread Luke Plant

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()?

2009-10-22 Thread Luke Plant

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()?

2009-10-23 Thread Luke Plant

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

2009-10-24 Thread Luke Plant

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

2009-10-26 Thread Luke Plant

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

2009-10-27 Thread Luke Plant

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

2009-10-27 Thread Luke Plant

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

2009-10-27 Thread Luke Plant

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

2009-10-27 Thread Luke Plant

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)

2009-10-30 Thread Luke Plant

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"

2009-10-31 Thread Luke Plant

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

2009-10-31 Thread Luke Plant

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

2009-11-13 Thread Luke Plant
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

2009-11-14 Thread Luke Plant
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?

2009-11-21 Thread Luke Plant
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

2009-11-28 Thread Luke Plant
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

2009-11-28 Thread Luke Plant
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

2009-11-30 Thread Luke Plant
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

2009-11-30 Thread Luke Plant
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)

2009-12-01 Thread Luke Plant
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

2009-12-01 Thread Luke Plant
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

2009-12-03 Thread Luke Plant
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

2009-12-03 Thread Luke Plant
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)

2009-12-03 Thread Luke Plant
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

2009-12-03 Thread Luke Plant
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)

2009-12-05 Thread Luke Plant
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

2009-12-05 Thread Luke Plant
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

2009-12-05 Thread Luke Plant
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)

2009-12-05 Thread Luke Plant
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

2009-12-08 Thread Luke Plant
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

2009-12-08 Thread Luke Plant
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__

2009-12-08 Thread Luke Plant
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__

2009-12-08 Thread Luke Plant
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

2009-12-09 Thread Luke Plant
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

2009-12-15 Thread Luke Plant
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?

2009-12-15 Thread Luke Plant
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

2009-12-17 Thread Luke Plant
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

2009-12-18 Thread Luke Plant
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

2009-12-21 Thread Luke Plant
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

2009-12-21 Thread Luke Plant
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

2009-12-21 Thread Luke Plant
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

2009-12-28 Thread Luke Plant
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)

2009-12-28 Thread Luke Plant
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 %}

2009-12-30 Thread Luke Plant
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 %}

2010-01-01 Thread Luke Plant
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

2010-01-04 Thread Luke Plant
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

2010-01-04 Thread Luke Plant
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

2010-01-04 Thread Luke Plant
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

2010-01-05 Thread Luke Plant
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

2010-01-06 Thread Luke Plant
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

2010-01-06 Thread Luke Plant
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

2010-01-06 Thread Luke Plant
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

2010-01-06 Thread Luke Plant
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

2010-01-08 Thread Luke Plant
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

2010-01-08 Thread Luke Plant
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

2010-01-08 Thread Luke Plant
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

2010-01-15 Thread Luke Plant
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

2010-01-18 Thread Luke Plant
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.




  1   2   3   4   5   6   7   8   >