classification
Title: Copy cgi.escape() to html
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, brett.cannon, eric.araujo, fdrake, georg.brandl, pablomouzo
Priority: high Keywords: easy, patch

Created on 2008-05-12 03:41 by brett.cannon, last changed 2010-10-15 15:58 by georg.brandl. This issue is now closed.

Files
File name Uploaded Description Edit
issue2830.diff pablomouzo, 2010-08-29 22:31 review
Messages (15)
msg66704 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-05-12 03:41
cgi.escape() really belong in the new 'html' package.
msg109268 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-04 22:02
I'm guessing that this has simply slipped under the radar.
msg109273 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2010-07-04 22:49
Yep since it is not a critical change. If someone came up with a patch to move the code and docs over, make cgi.escape use the moved code, and add a PendingDeprecationWarning then it would get done.
msg112186 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-07-31 19:35
In light of #9061, it should also start quoting single quotes when the arg is true.

Since this function is called a LOT, it might also make sense to trivially implement it in C.

If there are no objections, I can do that for 3.2.
msg114724 - (view) Author: Pablo Mouzo (pablomouzo) Date: 2010-08-23 01:56
I'm attaching a patch against py3k trunk that moves the function to the html module and fixes the documentation as Brett asked for. It also changes all the occurrences of cgi.escape I found for html.escape .
msg114733 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-08-23 10:08
Looks good already.  Two points:

* if we do the move, we should finally make sure all problematic characters are escaped.  For now, I think the single quote is the most important one in attribute mode.

* the new docs for cgi.escape() are missing a newline between signature and body.
msg114763 - (view) Author: Pablo Mouzo (pablomouzo) Date: 2010-08-24 00:25
Thanks Georg for the review, I'm attaching a new patch with those problems fixed.
The new patch escapes ' when the quote parameter is true, and / always.
msg114779 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-08-24 12:08
The actual implementation seems to be missing in the new patch; also the docs are not updated.

Is it necessary to escape the slash?
msg114862 - (view) Author: Pablo Mouzo (pablomouzo) Date: 2010-08-25 01:00
Sorry about that, I have no idea how I managed to generate that diff.

I'm attaching the correct patch.

About the slash, there's a link in #9061 that recommends to escape the slash too because it's used to close tags in HTML. Is there any chance that escaping it could cause problems?
msg114864 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-25 01:03
The link says “forward slash is included as it helps end an HTML entity”, which I don’t understand.
msg114871 - (view) Author: Fred L. Drake, Jr. (fdrake) (Python committer) Date: 2010-08-25 01:45
Encoding the forward slash should not cause problems, but the quote

  “forward slash is included as it helps end an HTML entity”

is confused; there's no need or additional value in escaping the forward slash.
msg115153 - (view) Author: Pablo Mouzo (pablomouzo) Date: 2010-08-28 13:43
I'm attaching a new patch without escaping the slash.
msg115156 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-08-28 15:15
The docs are still not updated for the quote.

I wonder if we shouldn't make the second argument True by default, while we're at it (or ignore it altogether and always escape everything) -- it would make the escape() much safer to use.

Also quoting "'" already introduces incompatibility if someone compares the result literally, so I would not worry about additional incompatibilities so much.
msg115197 - (view) Author: Pablo Mouzo (pablomouzo) Date: 2010-08-29 22:31
I'm attaching a new patch with the documentation updated.

I agree with Georg that it'd be better to escape everything by default. Are there any good reasons not to?
msg118786 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-10-15 15:58
Refined and applied the patch in r85531.  Thanks all!
History
Date User Action Args
2010-10-15 15:58:00georg.brandlsetstatus: open -> closed
resolution: fixed
messages: + msg118786
2010-08-29 22:41:36pablomouzosetfiles: - issue2830.diff
2010-08-29 22:31:42pablomouzosetfiles: + issue2830.diff

messages: + msg115197
2010-08-28 15:15:58georg.brandlsetmessages: + msg115156
2010-08-28 13:43:10pablomouzosetfiles: + issue2830.diff

messages: + msg115153
2010-08-28 13:42:12pablomouzosetfiles: - issue2830.diff
2010-08-25 01:45:45fdrakesetnosy: + fdrake
messages: + msg114871
2010-08-25 01:03:18eric.araujosetnosy: + eric.araujo
messages: + msg114864
2010-08-25 01:00:54pablomouzosetfiles: + issue2830.diff

messages: + msg114862
2010-08-25 00:46:12pablomouzosetfiles: - issue2830.diff
2010-08-24 12:08:44georg.brandlsetmessages: + msg114779
2010-08-24 01:31:39benjamin.petersonlinkissue9061 superseder
2010-08-24 00:26:03pablomouzosetfiles: - issue2830.diff
2010-08-24 00:25:45pablomouzosetfiles: + issue2830.diff

messages: + msg114763
2010-08-23 10:08:18georg.brandlsetmessages: + msg114733
2010-08-23 01:56:53pablomouzosetfiles: + issue2830.diff

nosy: + pablomouzo
messages: + msg114724

keywords: + patch
2010-07-31 19:35:08georg.brandlsetpriority: low -> high
nosy: + georg.brandl
messages: + msg112186

2010-07-04 22:49:46brett.cannonsetversions: + Python 3.2, - Python 2.6, Python 3.0
2010-07-04 22:49:32brett.cannonsetpriority: normal -> low
keywords: + easy
messages: + msg109273
2010-07-04 22:02:19BreamoreBoysetnosy: + BreamoreBoy
messages: + msg109268
2008-05-12 03:41:32brett.cannoncreate