since the unicode waves have calmed down, wouldn't it be a nice time to include the autoescaping patch?
I've brought most of the patches in ticket #2359 up to date. There are 3 patches, the first two deal with implementing autoescape as discussed and based on original ideas from Simon Willison, the third rewrites the admin interface to take use of it.
It doesn't make sense to put work in the third patch since the admin rewrite isn't done, but the other patches could be applied independently. Actually, I think it would be a good idea to first put in the autoescaping, see if there are any issues overlooked, and only then make the admin interface use it. Auto-escaping is off by default, so there shouldn't be any compatibility issues at all.
As you can see from the history, I use this patch for quite some time in production. Last comment from Malcolm (thanks for this great patch!), the patch author, asked Adrian for a review.
> since the unicode waves have calmed down, wouldn't it be a nice time to > include the autoescaping patch?
Malcolm, Simon and I talked about this at OSCON, and I'm now convinced. I'd like to have Malcolm look over your work, Michael, and make any changes he wants; from there lets get it done.
> On 8/1/07, Michael Radziej <m...@noris.de> wrote: > > since the unicode waves have calmed down, wouldn't it be a nice time to > > include the autoescaping patch?
> Malcolm, Simon and I talked about this at OSCON, and I'm now > convinced. I'd like to have Malcolm look over your work, Michael, and > make any changes he wants; from there lets get it done.
Thanks for the quick feedback! During the various merges it got touched quite a bit, so I'd consider a review essential. But it's still Malcolm's work. I only merged and the bugs are properly mine ;-)
Michael
-- noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg - Tel +49-911-9352-0 - Fax +49-911-9352-100 http://www.noris.de - The IT-Outsourcing Company
Vorstand: Ingo Kraupa (Vorsitzender), Joachim Astel, Hansjochen Klenk - Vorsitzender des Aufsichtsrats: Stefan Schnabel - AG Nürnberg HRB 17689
> Auto-escaping is off by default, so there shouldn't be any compatibility > issues at all.
I felt my hackles rising until I read this part. ^_^ I'm somewhere between -0 and +0 at this point so long as auto-escaping is off by default (and -1 otherwise, as per my objections in previous discussions on the topic). I still don't like it, but I have the feeling this is *never* going to go away. :P
On Wed, 2007-08-01 at 16:51 +0200, Michael Radziej wrote: > On Wed, Aug 01, Jacob Kaplan-Moss wrote:
> > On 8/1/07, Michael Radziej <m...@noris.de> wrote: > > > since the unicode waves have calmed down, wouldn't it be a nice time to > > > include the autoescaping patch?
> > Malcolm, Simon and I talked about this at OSCON, and I'm now > > convinced. I'd like to have Malcolm look over your work, Michael, and > > make any changes he wants; from there lets get it done.
> Thanks for the quick feedback! During the various merges it got touched > quite a bit, so I'd consider a review essential. But it's still Malcolm's > work. I only merged and the bugs are properly mine ;-)
I updated the patch during OSCON and it's pretty much ready to go. I've held off launching it this week because I'm on the road and in a hotel that hasn't discovered the Internet yet. So connectivity is a bit sporadic. Pending the outcome of this thread, I'll drop it in (defaulting to off for now) when I get back to Sydney next week and can be around to nurse people through any immediate problems.
The latest set of patches in the ticket were pretty much applicable. I think I tweaked one thing and have read through it in light of recent changes to trunk and updated the docs a bit. The one thing Jacob and I agreed to change at OSCON was to move to "autoescape on" and "autoescape off", rather than "autoescape/noautoescape" as the tags. This way, if somebody solves the impossible and comes up with a way to escape Javascript in a robust fashion, we can have "{% autoescape js %}" or similar; it's just a bit more extensible and Jacob made a convincing argument.
> I felt my hackles rising until I read this part. ^_^ I'm somewhere > between -0 and +0 at this point so long as auto-escaping is off by > default (and -1 otherwise, as per my objections in previous > discussions on the topic). I still don't like it, but I have the > feeling this is *never* going to go away. :P
I don't like it, but I don't think we have a choice: if it's off by default, the people who need to be using it will never turn it on. But if it's on by default, the people who think they don't need it will know enough to turn it off again.
-- "Bureaucrat Conrad, you are technically correct -- the best kind of correct."
On 8/1/07, James Bennett <ubernost...@gmail.com> wrote:
> On 8/1/07, Tom Tobin <korp...@korpios.com> wrote: > > I felt my hackles rising until I read this part. ^_^ I'm somewhere > > between -0 and +0 at this point so long as auto-escaping is off by > > default (and -1 otherwise, as per my objections in previous > > discussions on the topic). I still don't like it, but I have the > > feeling this is *never* going to go away. :P
> I don't like it, but I don't think we have a choice: if it's off by > default, the people who need to be using it will never turn it on. But > if it's on by default, the people who think they don't need it will > know enough to turn it off again.
I've only had a chance to take a brief look at the latest changes on that ticket (and I'm seeing a bunch of patches that are showing as blank in Trac for some reason), so I'm speaking a bit from ignorance here. I can probably live at -0 with something that lets me set one global setting, e.g., ``AUTOESCAPE = False``; anything that forces me to do ``{% autoescape off %}`` in every single base template is going to make me flip out with the hardest -1 I can muster. :P
> I've only had a chance to take a brief look at the latest changes on > that ticket (and I'm seeing a bunch of patches that are showing as > blank in Trac for some reason), so I'm speaking a bit from ignorance > here. I can probably live at -0 with something that lets me set one > global setting, e.g., ``AUTOESCAPE = False``; anything that forces me > to do ``{% autoescape off %}`` in every single base template is going > to make me flip out with the hardest -1 I can muster. :P
Unfortunately, AUTOESCAPE=False just is a bad idea; that's asking to introduce our very own "magic_quotes" feature, and we all know how well that worked for PHP.
Is autoescaping being on *really* that big a deal to you? How many places do your variables actually contain HTML? And if the answer is "many", then is it really all that big a deal to make a one-time modification to your base templates?
Fact is there isn't a solution that's gonna make everyone happy. Experienced developers like you and me are gonna hate this since it feels like making Django not trust us any more.
However, there's a *huge* number of not-so-experienced developers out there, and we *have* to keep their best needs in mind. Again with the PHP: we've seen how well trusting developers not to write security holes works. Anything we can do to prevent newbies from XSS attacks, we should.
We've seen how bad XSS can be; I think we have an almost moral obligation here.
Okay, post-Starbucks chat with my co-workers, here's my last-ditch mitigation proposal:
Set autoescaping on by default for anything ending in ``.html`` (and, perhaps, ``.htm``), and off otherwise.
Being (at least ideally) language-neutral has precedent in Django; we've already moved away from assuming templates end in ``.html``, and at least part of the reasoning there was acknowledging that the template language is used for more than just HTML. By having autoescape off by default for non-HTML templates, one can write one-off plain-text email templates, server configuration files, LaTeX, or what-have-you — stuff that doesn't necessarily lend well to hierarchical template extension — without always having to drop an {% autoescape off %} line on top.
Is this a bit magical? Yes, of course — but certainly no more so than having autoescaping on by default, and in fact removes a bit of magic from non-HTML use cases.
> Okay, post-Starbucks chat with my co-workers, here's my last-ditch > mitigation proposal:
Mmmmm.... Starbucks....
Yes, we had quite a good little argument about all this. It's a tricky problem; it feels like the only way to get this right is to piss off anyone not producing HTML, and that kinda blows.
So, given that, I'm pretty happy with Tom's mini-counter-proposal. It might take some nasty hacking to "remember" where a template was loaded from (not sure; gotta check), but not having < show up in ".txt" templates would be a Good Thing, methinks.
Malcolm -- think this'll be possible to do in a minimally ugly way?
On 8/1/07, Jacob Kaplan-Moss <jacob.kaplanm...@gmail.com> wrote:
> Is autoescaping being on *really* that big a deal to you? How many > places do your variables actually contain HTML? And if the answer is > "many", then is it really all that big a deal to make a one-time > modification to your base templates?
Hi, Jacob.
May I ask a clarifying question here? Is autoescaping going to be on by default, or no? And what exactly is it that's being added here -- a template tag for turning on/off autoescaping?
I know we've gone round and round about this, so I don't mean to cover old ground, but I did search the archives and wiki a bit and I'm still not clear. If it's not on by default, as James said, how does this make life safer for anyone? Don't get me wrong, I've argued against autoescaping before, but if it's in the framework why not turn it on by default? And I guess I don't really see the difference in turning on/off autoescaping globally in a base template vs. a global setting.
> May I ask a clarifying question here? Is autoescaping going to be on > by default, or no? And what exactly is it that's being added here -- > a template tag for turning on/off autoescaping?
The proposal is:
* Autoescaping is on by default. * A tag will be provided, which will inherit down into child templates, to turn it off if you desire (so you could turn off escaping in your base template, and anything inheriting from it would also have escaping off). * Maybe -- in Tom's counter-proposal -- this will only apply in templates whose filenames end with '.html'.
Personally, I don't like the last bit of that, because there are use cases where you'd be wanting to use HTML in a template that didn't come from a file whose extension was '.html'.
-- "Bureaucrat Conrad, you are technically correct -- the best kind of correct."
> May I ask a clarifying question here? Is autoescaping going to be on > by default, or no? And what exactly is it that's being added here -- > a template tag for turning on/off autoescaping?
Yes, the plan is to have autoescaping on unless you explicitally turn it off in your template (or by not using ".html" template file names if Tom's proposal holds).
What's being added is both the autoescaping behavior -- Django currently does no automatic escaping whatsoever -- and the mechanism to turn it off.
I think the current debate has moved away from "should we have autoescaping" since I think we're all pretty much agreed that *some* form of automatic escaping is neccisary if we're gonna protect our users from shooting themselves in the foot.
> I know we've gone round and round about this, so I don't mean to cover > old ground, but I did search the archives and wiki a bit and I'm still > not clear. If it's not on by default, as James said, how does this > make life safer for anyone? Don't get me wrong, I've argued against > autoescaping before, but if it's in the framework why not turn it on > by default?
Yup, you said it. If we're gonna do this, we should do it right. I've been pretty much convinced that we've got to have autoescape, and it's got to be on by default. It's basically a tradeoff between annoyance (of having to turn it off when you need it off) and security, and security has to win that argument.
> And I guess I don't really see the difference in turning > on/off autoescaping globally in a base template vs. a global setting.
The problem with a global AUTOESCAPE setting is that it makes writing distributable Django applications impossible.
OK, that came out a bit cryptic, so let me explain:
Back in my days as a PHP developer, I tried to write a handful of distributable libraries (including a really, really shitty ORM. You think Django's ORM is weak? You shoulda seen this one...). It isn't long until I ran into trouble with magic_quotes. I'll save the heartache, but the short version is that it's actually impossible to write code that handles magic_quotes defensively. You literally *can't* write code that works for any input without dictating that magic_quotes must be on or off.
From a user's point of view, it sucks even harder. So you install that cool PHP forum everyone's been crowing about. In the process, you turn off magic_quotes like the docs tell you too. A few months later, you install new blog software, and start getting weird SQL errors. Oops, that blog needs magic_quotes on, but if you do that, the forum breaks...
The lesson here is that code interpretation *can't* be dependent on environment. If we have a global autoescape setting, we can kiss easily-distributable Django apps goodbye.
If, however, we turn it off in a base template, we're golden; apps can simply distribute a base.html that does whatever's right for the library.
On 8/1/07, Jacob Kaplan-Moss <jacob.kaplanm...@gmail.com> wrote:
> From a user's point of view, it sucks even harder. So you install that > cool PHP forum everyone's been crowing about. In the process, you turn > off magic_quotes like the docs tell you too. A few months later, you > install new blog software, and start getting weird SQL errors. Oops, > that blog needs magic_quotes on, but if you do that, the forum > breaks...
> The lesson here is that code interpretation *can't* be dependent on > environment. If we have a global autoescape setting, we can kiss > easily-distributable Django apps goodbye.
> If, however, we turn it off in a base template, we're golden; apps can > simply distribute a base.html that does whatever's right for the > library.
I guess I was envisioning the work flow where someone sets autescape off in their site-wide base.html, but if I as an app developer rely on the behavior being one way and I know to just explicitly set autoescape on/off at the app-level templates (again assuming this really matters), I can see the difference.
So I'm cool with all this. I agree it's inevitable and just want the implementation to be as sane as possible. This seems headed down the right path to me.
Thanks for the detailed response and clarification!
On Wednesday 01 August 2007 19:56:05 Tom Tobin wrote:
> Okay, post-Starbucks chat with my co-workers, here's my last-ditch > mitigation proposal:
> Set autoescaping on by default for anything ending in ``.html`` (and, > perhaps, ``.htm``), and off otherwise.
-1 on this, it's much too magic for me. If you actually look at implementing it, it feels even worse -- you have to modify the Template.render() method to use information it doesn't even reliably have (the name of the template -- won't exist for templates from strings), and then *modify* the context object it is passed in on that basis...it's nasty.
Also consider cases where someone goes from using a template inline in a Python file (as I have in a few places in my source code for very small templates) to having them stored in files. Copying and pasting into a new file certainly qualifies as a simple refactor in my mind -- I probably wouldn't bother even testing the contents of the output. It would be pretty evil if the template could start behaving differently after doing this.
As for autoescaping on by default, I'm +1 -- I'm a reasonably experienced Django developer, and sometimes I forget |escape. And sometimes this mistake has existed in Django admin. The fact that you can get it right 99% of the time is no argument -- the 1% is enough to get you totally pwned. The answer to this is the same as the answer to SQL injection -- 'Never fix a bug twice' [1].
On Wed, 2007-08-01 at 14:00 -0500, Jacob Kaplan-Moss wrote:
[...]
> So, given that, I'm pretty happy with Tom's mini-counter-proposal. It > might take some nasty hacking to "remember" where a template was > loaded from (not sure; gotta check), but not having < show up in > ".txt" templates would be a Good Thing, methinks.
> Malcolm -- think this'll be possible to do in a minimally ugly way?
For file based loaders, it's technically possible. For anything not file-based, pick one. We'd better extend the file extensions to include .xhtml and .htm as well, I guess, since they're common product from some tools and alleged operating systems.
Artistically ugly, but I've given up caring. This is just insane. On or off really, really doesn't matter. Off by default and requiring anybody who cares about being vaguely professional has to type in the tag isn't even that big a deal.
Anybody who thinks this decision is actually a really big life-changing decision needs to re-evaluate their priorities. And perhaps pick some bigger-item bugs to fix. This is too depressing; just pick one and move on.
On 8/1/07, Luke Plant <L.Plant...@cantab.net> wrote:
> On Wednesday 01 August 2007 19:56:05 Tom Tobin wrote: > > Okay, post-Starbucks chat with my co-workers, here's my last-ditch > > mitigation proposal:
> > Set autoescaping on by default for anything ending in ``.html`` (and, > > perhaps, ``.htm``), and off otherwise.
> -1 on this, it's much too magic for me. If you actually look at > implementing it, it feels even worse -- you have to modify the > Template.render() method to use information it doesn't even reliably > have (the name of the template -- won't exist for templates from > strings), and then *modify* the context object it is passed in on that > basis...it's nasty.
I'm indeed assuming implementation issues can be surmounted; right now I'm just floating the idea, and I'm more than willing to take a crack at implementation if it gets a reasonable pass.
> Also consider cases where someone goes from using a template inline in a > Python file (as I have in a few places in my source code for very small > templates) to having them stored in files. Copying and pasting into a > new file certainly qualifies as a simple refactor in my mind -- I > probably wouldn't bother even testing the contents of the output. It > would be pretty evil if the template could start behaving differently > after doing this.
Let's assume autoescaping is on unless the template engine knows otherwise; your inline templates will work as expected both before and after extraction (again, assuming you're extracting them to a file ending in ``.html``). Inline templates that shouldn't be escaped can either be decorated somehow, or have the ``{% autoescape off %}`` tag prepended. (I'd prefer the former, but my objection for this uncommon subset of an already uncommon case is weaker than my general file-template autoescaping objection.)
On 8/1/07, Malcolm Tredinnick <malc...@pointy-stick.com> wrote:
> Anybody who thinks this decision is actually a really big life-changing > decision needs to re-evaluate their priorities. And perhaps pick some > bigger-item bugs to fix. This is too depressing; just pick one and move > on.
Translating "passionate about getting this right for Django" to "considering this a life-changing decision" is one *hell* of a jump. Please don't tar me with that brush; I'm sorry that you're weary of this debate, but that's just not cool. This is a priority for me *within Django*; we all have different parts of Django that we focus on perhaps more than others, and this happens to be one of mine.
> Let's assume autoescaping is on unless the template engine knows > otherwise; your inline templates will work as expected both before and > after extraction (again, assuming you're extracting them to a file > ending in ``.html``).
Let's just do:
1. Autoescape on by default. 2. Autoescape is turned off by the {% autoescape off %} 3. Autoescape happens irregardless of what the template's source file or source string happened to be named.
-- "Bureaucrat Conrad, you are technically correct -- the best kind of correct."
On 8/1/07, James Bennett <ubernost...@gmail.com> wrote:
> On 8/1/07, Tom Tobin <korp...@korpios.com> wrote: > > Let's assume autoescaping is on unless the template engine knows > > otherwise; your inline templates will work as expected both before and > > after extraction (again, assuming you're extracting them to a file > > ending in ``.html``).
> Let's just do:
> 1. Autoescape on by default. > 2. Autoescape is turned off by the {% autoescape off %} > 3. Autoescape happens irregardless of what the template's source file > or source string happened to be named.
Thank you for taking this opportunity to remind me of the autoescaping proposal. I must have forgotten it somewhere along the way here. (Next time, you *could* just say you're -1.) :P
> Thank you for taking this opportunity to remind me of the autoescaping > proposal. I must have forgotten it somewhere along the way here. > (Next time, you *could* just say you're -1.) :P
I'm just saying: let's do this.
Malcolm's right: the amount of argument this is generating is out of proportion to how important the change is, so let's just do it and move on.
-- "Bureaucrat Conrad, you are technically correct -- the best kind of correct."
> 1. Autoescape on by default. > 2. Autoescape is turned off by the {% autoescape off %} > 3. Autoescape happens irregardless of what the template's source file > or source string happened to be named.
On Wed, Aug 01, James Bennett wrote: > Let's just do:
> 1. Autoescape on by default. > 2. Autoescape is turned off by the {% autoescape off %} > 3. Autoescape happens irregardless of what the template's source file > or source string happened to be named.
I tried to keep myself out of this discussion, as I also don't care so much. But have you thought about the admin interface? Would it work when autoescape is on by default?
I'd still suggest a 'default off' policy at least when autoescape is introduced, just to give users a transition period. A 'default on' policy means that autoescape will break a lot of custom template tags and filters. The policy could then change to 'default on' shortly before a new release.
Michael
-- noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg - Tel +49-911-9352-0 - Fax +49-911-9352-100 http://www.noris.de - The IT-Outsourcing Company
Vorstand: Ingo Kraupa (Vorsitzender), Joachim Astel, Hansjochen Klenk - Vorsitzender des Aufsichtsrats: Stefan Schnabel - AG Nürnberg HRB 17689
On Aug 2, 12:55 am, Michael Radziej <m...@noris.de> wrote:
> I'd still suggest a 'default off' policy at least when autoescape is > introduced, just to give users a transition period. A 'default on' policy > means that autoescape will break a lot of custom template tags and filters. > The policy could then change to 'default on' shortly before a new release.
I kind of think it should just be done 100% in 0.97 and announced in the release notes. Everyone should know that pre 1.0 there will be some backward incompatible changes and should watch out for them, no? So far Django has had great docs on transitioning between releases.
