A critique of cgi.escape

[Lawrence D'Oliveiro]

The "escape" function in the "cgi" module escapes characters with special
meanings in HTML. The ones that need escaping are '<', '&' and '"'.
However, cgi.escape only escapes the quote character if you pass a second
argument of True (the default is False):

>>cgi.escape("t he \"quick\" & <brownfox")

'the "quick" &amp; &lt;brown&gt ; fox'

>>cgi.escape("t he \"quick\" & <brownfox", True)

'the &quot;quick&quo t; &amp; &lt;brown&gt ; fox'

This seems to me to be dumb. The default option should be the safe one: that
is, escape _all_ the potentially troublesome characters. The only time you
can get away with NOT escaping the quote character is outside of markup,
e.g.

<TEXTAREA>
unescaped "quotes" allowed here
</TEXTAREA>

Nevertheless, even in that situation, escaped quotes are acceptable.

So I think the default for the second argument to cgi.escape should be
changed to True. Or alternatively, the second argument should be removed
altogether, and quotes should always be escaped.

Can changing the default break existing scripts? I don't see how. It might
even fix a few lurking bugs out there.

[Fredrik Lundh]

Lawrence D'Oliveiro wrote:

So I think the default for the second argument to cgi.escape should be
changed to True. Or alternatively, the second argument should be removed
altogether, and quotes should always be escaped.

you're confused: cgi.escape(s) is designed to be used for ordinary text,
cgi.escape(s, True) is designed for attributes. if you use the code the
way it's intended to be used, it works perfectly fine.

Can changing the default break existing scripts? I don't see how. It might
even fix a few lurking bugs out there.

I'm not sure this "every time I don't immediately understand something,
I'll write a change proposal instead of reading the library reference"
approach is healthy, really.

</F>

[Lawrence D'Oliveiro]

In message <ma************ *************** ***********@pyt hon.org>, Fredrik
Lundh wrote:

Lawrence D'Oliveiro wrote:

>So I think the default for the second argument to cgi.escape should be
changed to True. Or alternatively, the second argument should be removed
altogether, and quotes should always be escaped.

you're confused: cgi.escape(s) is designed to be used for ordinary text,
cgi.escape(s, True) is designed for attributes.

What works for attributes also works for ordinary text.

[Jon Ribbens]

In article <ma************ *************** ***********@pyt hon.org>, Fredrik Lundh wrote:

Lawrence D'Oliveiro wrote:

>So I think the default for the second argument to cgi.escape should be
changed to True. Or alternatively, the second argument should be removed
altogether, and quotes should always be escaped.

you're confused: cgi.escape(s) is designed to be used for ordinary text,
cgi.escape(s, True) is designed for attributes. if you use the code the
way it's intended to be used, it works perfectly fine.

He's not confused, he's correct; the author of cgi.escape is the
confused one. The optional extra parameter is completely unnecessary
and achieves nothing except to make it easier for people to end up
with bugs in their code.

Making cgi.escape always escape the '"' character would not break
anything, and would probably fix a few bugs in existing code. Yes,
those bugs are not cgi.escape's fault, but that's no reason not to
be helpful. It's a minor improvement with no downside.

One thing that is flat-out wrong, by the way, is that cgi.escape()
does not encode the apostrophe (') character. This is essentially
identical to the quote character in HTML, so any code which escaping
one should always be escaping the other.

[Lawrence D'Oliveiro]

In message <sl************ ***********@sno wy.squish.net>, Jon Ribbens wrote:

In article <ma************ *************** ***********@pyt hon.org>, Fredrik
Lundh wrote:

>Lawrence D'Oliveiro wrote:

>>>
So I think the default for the second argument to cgi.escape should be
changed to True. Or alternatively, the second argument should be removed
altogether, and quotes should always be escaped.

you're confused: cgi.escape(s) is designed to be used for ordinary text,
cgi.escape(s , True) is designed for attributes. if you use the code the
way it's intended to be used, it works perfectly fine.

He's not confused, he's correct; the author of cgi.escape is the
confused one.

Thanks for backing me up. :)

One thing that is flat-out wrong, by the way, is that cgi.escape()

does not encode the apostrophe (') character. This is essentially
identical to the quote character in HTML, so any code which escaping
one should always be escaping the other.

I must confess I did a double-take on this. But I rechecked the HTML spec
(HTML 4.0, section 3.2.2, "Attributes "), and you're right--single quotes
ARE allowed as an alternative to double quotes. It's just I've never used
them as quotes. :)

[Fredrik Lundh]

Lawrence D'Oliveiro wrote:

What works for attributes also works for ordinary text.

attributes and ordinary text are two different things in HTML and XML.
you're arguing that it's a good idea for *everyone* to bloat down
ordinary text just because you're too lazy to use a piece of code in the
intended way.

</F>

[Fredrik Lundh]

Jon Ribbens wrote:

Making cgi.escape always escape the '"' character would not break
anything, and would probably fix a few bugs in existing code. Yes,
those bugs are not cgi.escape's fault, but that's no reason not to
be helpful. It's a minor improvement with no downside.

the "improvemen t with no downside" would bloat down the output for
everyone who's using the function in the intended way, and will also
break unit tests.

One thing that is flat-out wrong, by the way, is that cgi.escape()
does not encode the apostrophe (') character.

it's intentional, of course: you're supposed to use " if you're using
cgi.escape(s, True) to escape attributes. again, punishing people who
actually read the docs and understand them is not a very good way to
maintain software.

btw, you're both missing that cgi.escape isn't good enough for general
use anyway, since it doesn't deal with encodings at all. if you want a
general purpose function that can be used for everything that can be put
in an HTML file, you need more than just a modified cgi.escape. feel
free to propose a general-purpose replacement (which should have a new
name), but make sure you think through *all* the issues before you do that.

</F>

[Lawrence D'Oliveiro]

In message <ma************ *************** ***********@pyt hon.org>, Fredrik
Lundh wrote:

Jon Ribbens wrote:

>Making cgi.escape always escape the '"' character would not break
anything, and would probably fix a few bugs in existing code. Yes,
those bugs are not cgi.escape's fault, but that's no reason not to
be helpful. It's a minor improvement with no downside.

the "improvemen t with no downside" would bloat down the output for
everyone who's using the function in the intended way, and will also
break unit tests.

I don't understand this "bloat down" nonsense. Any tests that would break
are obviously testing the wrong thing.

One thing that is flat-out wrong, by the way, is that cgi.escape()
does not encode the apostrophe (') character.

it's intentional, of course: you're supposed to use " if you're using
cgi.escape(s, True) to escape attributes.

Attributes can be quoted with either single or double quotes. That's what
the HTML spec says. cgi.escape doesn't correctly allow for that. Ergo,
cgi.escape is broken. QED.

btw, you're both missing that cgi.escape isn't good enough for general
use anyway, since it doesn't deal with encodings at all.

Why does it need to?

[Fredrik Lundh]

Lawrence D'Oliveiro wrote:

Attributes can be quoted with either single or double quotes. That's what
the HTML spec says. cgi.escape doesn't correctly allow for that. Ergo,
cgi.escape is broken. QED.

do you ever think before you post?

</F>

[Georg Brandl]

Lawrence D'Oliveiro wrote:

In message <ma************ *************** ***********@pyt hon.org>, Fredrik
Lundh wrote:

>Jon Ribbens wrote:

>>Making cgi.escape always escape the '"' character would not break
anything, and would probably fix a few bugs in existing code. Yes,
those bugs are not cgi.escape's fault, but that's no reason not to
be helpful. It's a minor improvement with no downside.

the "improvemen t with no downside" would bloat down the output for
everyone who's using the function in the intended way, and will also
break unit tests.

I don't understand this "bloat down" nonsense. Any tests that would break
are obviously testing the wrong thing.

&quot; is 4 characters more than ".

> One thing that is flat-out wrong, by the way, is that cgi.escape()
does not encode the apostrophe (') character.

it's intentional, of course: you're supposed to use " if you're using
cgi.escape(s , True) to escape attributes.

Attributes can be quoted with either single or double quotes. That's what
the HTML spec says. cgi.escape doesn't correctly allow for that. Ergo,
cgi.escape is broken. QED.

A function is broken if its implementation doesn't match the documentation.

As a courtesy, I've pasted it below.

escape(s[, quote])
Convert the characters "&", "<" and ">" in string s to HTML-safe sequences.
Use this if you need to display text that might contain such characters in HTML.
If the optional flag quote is true, the quotation mark character (""") is also
translated; this helps for inclusion in an HTML attribute value, as in <A
HREF="...">. If the value to be quoted might include single- or double-quote
characters, or both, consider using the quoteattr() function in the
xml.sax.saxutil s module instead.
Now, do you still think cgi.escape is broken?
Georg