Re: [Zope-Checkins] CVS: Zope/lib/python/TAL - TALInterpreter.py:1.69.6.10
What prompted this? What does it fix? I don't see that quotes need escaping here. Also, there should be a unit test for it. (And if it's a fix it should be in HEAD too.) Florent In article <200212180033.gBI0Xh515285@cvs.baymountain.com> you write:
Modified Files: Tag: Zope-2_6-branch TALInterpreter.py Log Message: Escape quotes correctly
=== Zope/lib/python/TAL/TALInterpreter.py 1.69.6.9 => 1.69.6.10 === --- Zope/lib/python/TAL/TALInterpreter.py:1.69.6.9 Wed Oct 9 10:37:38 2002 +++ Zope/lib/python/TAL/TALInterpreter.py Tue Dec 17 19:33:43 2002 @@ -486,7 +486,7 @@ if text is self.Default: self.interpret(stuff[1]) return - s = escape(text) + s = escape(text, 1) self._stream_write(s) i = s.rfind('\n') if i < 0:
-- Florent Guillaume, Nuxeo (Paris, France) +33 1 40 33 79 87 http://nuxeo.com mailto:fg@nuxeo.com
Collector number 735: http://collector.zope.org/Zope/735 This was prompted by some bugs people found with Plone. For a two character fix that to me looked like a simple oversight I didn't bother with a unit test. -- Andy McKay ----- Original Message ----- From: "Florent Guillaume" <fg@nuxeo.com> To: <andy@agmweb.ca> Cc: <zope-dev@zope.org> Sent: Sunday, December 22, 2002 11:01 AM Subject: [Zope-dev] Re: [Zope-Checkins] CVS: Zope/lib/python/TAL - TALInterpreter.py:1.69.6.10
What prompted this? What does it fix? I don't see that quotes need escaping here. Also, there should be a unit test for it. (And if it's a fix it should be in HEAD too.)
Florent
In article <200212180033.gBI0Xh515285@cvs.baymountain.com> you write:
Modified Files: Tag: Zope-2_6-branch TALInterpreter.py Log Message: Escape quotes correctly
=== Zope/lib/python/TAL/TALInterpreter.py 1.69.6.9 => 1.69.6.10 === --- Zope/lib/python/TAL/TALInterpreter.py:1.69.6.9 Wed Oct 9 10:37:38 2002 +++ Zope/lib/python/TAL/TALInterpreter.py Tue Dec 17 19:33:43 2002 @@ -486,7 +486,7 @@ if text is self.Default: self.interpret(stuff[1]) return - s = escape(text) + s = escape(text, 1) self._stream_write(s) i = s.rfind('\n') if i < 0:
-- Florent Guillaume, Nuxeo (Paris, France) +33 1 40 33 79 87 http://nuxeo.com mailto:fg@nuxeo.com
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
But why? Why escape double quotes when it's not a reserved character? I really don't understand the need, and the example in the collector doesn't convince me. What is the problem? Escaping quotes is needed in double quotes-delimited attributes but not elsewhere. Florent On Mon, 2002-12-23 at 01:49, Andy McKay wrote:
Collector number 735: http://collector.zope.org/Zope/735
This was prompted by some bugs people found with Plone. For a two character fix that to me looked like a simple oversight I didn't bother with a unit test. -- Andy McKay
----- Original Message ----- From: "Florent Guillaume" <fg@nuxeo.com> To: <andy@agmweb.ca> Cc: <zope-dev@zope.org> Sent: Sunday, December 22, 2002 11:01 AM Subject: [Zope-dev] Re: [Zope-Checkins] CVS: Zope/lib/python/TAL - TALInterpreter.py:1.69.6.10
What prompted this? What does it fix? I don't see that quotes need escaping here. Also, there should be a unit test for it. (And if it's a fix it should be in HEAD too.)
Florent
In article <200212180033.gBI0Xh515285@cvs.baymountain.com> you write:
Modified Files: Tag: Zope-2_6-branch TALInterpreter.py Log Message: Escape quotes correctly
=== Zope/lib/python/TAL/TALInterpreter.py 1.69.6.9 => 1.69.6.10 === --- Zope/lib/python/TAL/TALInterpreter.py:1.69.6.9 Wed Oct 9 10:37:38 2002 +++ Zope/lib/python/TAL/TALInterpreter.py Tue Dec 17 19:33:43 2002 @@ -486,7 +486,7 @@ if text is self.Default: self.interpret(stuff[1]) return - s = escape(text) + s = escape(text, 1) self._stream_write(s) i = s.rfind('\n') if i < 0:
-- Florent Guillaume, Nuxeo (Paris, France) +33 1 40 33 79 87 http://nuxeo.com mailto:fg@nuxeo.com
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
-- Florent Guillaume, Nuxeo (Paris, France) +33 1 40 33 79 87 http://nuxeo.com mailto:fg@nuxeo.com
But why? Why escape double quotes when it's not a reserved character? I really don't understand the need, and the example in the collector doesn't convince me. What is the problem?
The problem occurs where you want include the value of a variable inside an attribute. For example: <input type="string" tal:attributes="value someValue"> By default Page Templates replaces the escaped value in the form, unless "structure" is specified. If someValue contained a " (eg: test with a " quote) then you would get the following invalid html (this is what you currently get): <input type="string" value="test with a " quote"> This of course should be (and what the patch changes it to): <input type="string" value="test with a " quote"> If you do not escape double quotes in the Page Template html quoting function, you will never be able to easily use Page Templates unless you are sure that someValue does not have double quotes escaped. This would mean having to import an escaping function into your page templates or adding an extra layer of python involved for almost every use of a variable. Otherwise you would be living on the edge as you would never be quite sure if yout HTML is properly escaped. For example this occurs in almost all of the CMF skin. Note that in good old DTML if you use html_quote, you're escaping " by default (lib\python\DocumentTemplate\html_quote.py). If we are escaping <> and &, I see no reason not to escape " as well. And I believe the Zope Book Appendix C needs updating to reflect this.
(And if it's a fix it should be in HEAD too.)
Why? Test checked in by the way. -- Andy McKay
Ok thanks for the explanation, I didn't realize this piece of code was used for attribute replacement too.
Test checked in by the way.
Thanks a lot. However it doesn't test the case you describe below, does it?
(And if it's a fix it should be in HEAD too.)
Why?
Well, because otherwise 2.7 won't have the fix :-) Florent On Mon, 2002-12-23 at 18:29, Andy McKay wrote:
But why? Why escape double quotes when it's not a reserved character? I really don't understand the need, and the example in the collector doesn't convince me. What is the problem?
The problem occurs where you want include the value of a variable inside an attribute.
For example:
<input type="string" tal:attributes="value someValue">
By default Page Templates replaces the escaped value in the form, unless "structure" is specified. If someValue contained a " (eg: test with a " quote) then you would get the following invalid html (this is what you currently get):
<input type="string" value="test with a " quote">
This of course should be (and what the patch changes it to):
<input type="string" value="test with a " quote">
If you do not escape double quotes in the Page Template html quoting function, you will never be able to easily use Page Templates unless you are sure that someValue does not have double quotes escaped. This would mean having to import an escaping function into your page templates or adding an extra layer of python involved for almost every use of a variable.
Otherwise you would be living on the edge as you would never be quite sure if yout HTML is properly escaped. For example this occurs in almost all of the CMF skin. Note that in good old DTML if you use html_quote, you're escaping " by default (lib\python\DocumentTemplate\html_quote.py).
If we are escaping <> and &, I see no reason not to escape " as well.
And I believe the Zope Book Appendix C needs updating to reflect this.
Test checked in by the way. -- Andy McKay -- Florent Guillaume, Nuxeo (Paris, France) +33 1 40 33 79 87 http://nuxeo.com mailto:fg@nuxeo.com
Thanks a lot. However it doesn't test the case you describe below, does it?
Its just tests that a " is always escaped.
(And if it's a fix it should be in HEAD too.)
Why?
Well, because otherwise 2.7 won't have the fix :-)
Really, so all the changes and bug fixes done for 2.6.1 have to be put in HEAD too? Thats a pain. -- Andy McKay
The problem occurs where you want include the value of a variable inside an attribute.
For example:
<input type="string" tal:attributes="value someValue">
By default Page Templates replaces the escaped value in the form, unless "structure" is specified. If someValue contained a " (eg: test with a " quote) then you would get the following invalid html (this is what you currently get):
<input type="string" value="test with a " quote">
This of course should be (and what the patch changes it to):
<input type="string" value="test with a " quote">
If you do not escape double quotes in the Page Template html quoting function, you will never be able to easily use Page Templates unless you are sure that someValue does not have double quotes escaped. This would mean having to import an escaping function into your page templates or adding an extra layer of python involved for almost every use of a variable.
I think page templates should escape quotes in tal:attributes, but not under other circumstances. Also, I believe that an alternative in the example you give would be to use single quotes. This is consistent with how quoteattr works:
from xml.sax.saxutils import quoteattr print quoteattr('foo" bar') 'foo" bar' print quoteattr("""foo' bar""") "foo' bar" print quoteattr("""foo'" bar""") "foo'" bar"
quoteattr only resorts to using " when there is no alternative -- that is, when the string being quoted contains both a single quote and a double quote. So, perhaps an alternative fix would be to revert the current fix, and instead make tal:attributes use quoteattr. -- Steve Alexander
Andy McKay wrote:
Collector number 735: http://collector.zope.org/Zope/735
This was prompted by some bugs people found with Plone. For a two character fix that to me looked like a simple oversight I didn't bother with a unit test.
I'm not going to comment on whether or not page templates should work this way. There good reasons to write a unit test in just this kind of situation: It is only a two character fix. So, someone else might come along later, and think that those two characters shouldn't be there -- just as you think those two characters should be there. They could remove those characters, and no tests would fail. Also, in a refactoring, this behaviour couild be easily lost if there is no unit test to demonstrate correct behaviour. -- Steve Alexander
Andy McKay wrote:
Collector number 735: http://collector.zope.org/Zope/735
This was prompted by some bugs people found with Plone. For a two character fix that to me looked like a simple oversight I didn't bother with a unit test.
*bzzzt* bad. Everything should be unittested, please correct ;-) Chris
participants (4)
-
Andy McKay -
Chris Withers -
Florent Guillaume -
Steve Alexander