Skip to content

Commit

Permalink
Be explicit about escaping ".
Browse files Browse the repository at this point in the history
html.escape has its second argument quote=True by default, while
cgi.escape had default=False
  • Loading branch information
hannosch committed May 15, 2017
1 parent 00146ca commit 91d7411
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 57 deletions.
6 changes: 3 additions & 3 deletions src/App/Management.py
Expand Up @@ -89,17 +89,17 @@ def tabs_path_default(self, REQUEST):
script = REQUEST['BASEPATH1']
linkpat = '<a href="%s/manage_workspace">%s</a>'
out = []
url = linkpat % (escape(script, 1), '&nbsp;/')
url = linkpat % (escape(script, True), '&nbsp;/')
if not steps:
return url
last = steps.pop()
for step in steps:
script = '%s/%s' % (script, step)
out.append(linkpat % (escape(script, 1), escape(unquote(step))))
out.append(linkpat % (escape(script, True), escape(unquote(step))))
script = '%s/%s' % (script, last)
out.append(
'<a class="strong-link" href="%s/manage_workspace">%s</a>' %
(escape(script, 1), escape(unquote(last))))
(escape(script, True), escape(unquote(last), False)))
return '%s%s' % (url, '/'.join(out))

def tabs_path_info(self, script, path):
Expand Down
4 changes: 2 additions & 2 deletions src/OFS/Image.py
Expand Up @@ -910,11 +910,11 @@ def tag(self, height=None, width=None, alt=None,

if alt is None:
alt = getattr(self, 'alt', '')
result = '%s alt="%s"' % (result, escape(alt, 1))
result = '%s alt="%s"' % (result, escape(alt, True))

if title is None:
title = getattr(self, 'title', '')
result = '%s title="%s"' % (result, escape(title, 1))
result = '%s title="%s"' % (result, escape(title, True))

if height:
result = '%s height="%s"' % (result, height)
Expand Down
11 changes: 6 additions & 5 deletions src/OFS/ObjectManager.py
Expand Up @@ -94,11 +94,12 @@ def checkValidId(self, id, allow_dup=0):
# set to false before the object is added.
if not id or not isinstance(id, str):
if isinstance(id, text_type):
id = escape(id)
id = escape(id, True)
raise BadRequest('Empty or invalid id specified', id)
if bad_id(id) is not None:
raise BadRequest(
'The id "%s" contains characters illegal in URLs.' % escape(id))
('The id "%s" contains characters '
'illegal in URLs.' % escape(id, True)))
if id in ('.', '..'):
raise BadRequest(
'The id "%s" is invalid because it is not traversable.' % id)
Expand Down Expand Up @@ -541,7 +542,7 @@ def manage_delObjects(self, ids=[], REQUEST=None):
'Object "%s" is locked.' % v.getId())

if v is self:
raise BadRequest('%s does not exist' % escape(ids[-1]))
raise BadRequest('%s does not exist' % escape(ids[-1], True))
self._delObject(id)
del ids[-1]
if REQUEST is not None:
Expand Down Expand Up @@ -615,14 +616,14 @@ def manage_importObject(self, file, REQUEST=None, set_owner=1):
"""Import an object from a file"""
dirname, file = os.path.split(file)
if dirname:
raise BadRequest('Invalid file name %s' % escape(file))
raise BadRequest('Invalid file name %s' % escape(file, True))

for impath in self._getImportPaths():
filepath = os.path.join(impath, 'import', file)
if os.path.exists(filepath):
break
else:
raise BadRequest('File does not exist: %s' % escape(file))
raise BadRequest('File does not exist: %s' % escape(file, True))

imported = self._importObjectFromFile(
filepath, verify=bool(REQUEST), set_owner=set_owner)
Expand Down
19 changes: 13 additions & 6 deletions src/OFS/PropertyManager.py
Expand Up @@ -124,8 +124,10 @@ class PropertyManager(Base):

security.declareProtected(access_contents_information, 'valid_property_id')
def valid_property_id(self, id):
if not id or id[:1] == '_' or (id[:3] == 'aq_') \
or (' ' in id) or hasattr(aq_base(self), id) or escape(id) != id:
if (not id or id[:1] == '_' or (id[:3] == 'aq_') or
(' ' in id) or
hasattr(aq_base(self), id) or
escape(id, True) != id):
return 0
return 1

Expand Down Expand Up @@ -202,7 +204,8 @@ def _updateProperty(self, id, value):
# the value to the type of the existing property.
self._wrapperCheck(value)
if not self.hasProperty(id):
raise BadRequest('The property %s does not exist' % escape(id))
raise BadRequest(
'The property %s does not exist' % escape(id, True))
if isinstance(value, str):
proptype = self.getPropertyType(id) or 'string'
if proptype in type_converters:
Expand All @@ -211,7 +214,8 @@ def _updateProperty(self, id, value):

def _delProperty(self, id):
if not self.hasProperty(id):
raise ValueError('The property %s does not exist' % escape(id))
raise ValueError(
'The property %s does not exist' % escape(id, True))
self._delPropValue(id)
self._properties = tuple(i for i in self._properties if i['id'] != id)

Expand Down Expand Up @@ -326,8 +330,10 @@ def manage_changeProperties(self, REQUEST=None, **kw):
for name, value in props.items():
if self.hasProperty(name):
if 'w' not in propdict[name].get('mode', 'wd'):
raise BadRequest('%s cannot be changed' % escape(name))
raise BadRequest(
'%s cannot be changed' % escape(name, True))
self._updateProperty(name, value)

if REQUEST:
message = "Saved changes."
return self.manage_propertiesForm(self, REQUEST,
Expand Down Expand Up @@ -367,7 +373,8 @@ def manage_delProperties(self, ids=None, REQUEST=None):
for id in ids:
if not hasattr(aq_base(self), id):
raise BadRequest(
'The property <em>%s</em> does not exist' % escape(id))
('The property <em>%s</em> '
'does not exist' % escape(id, True)))
if ('d' not in propdict[id].get('mode', 'wd')) or (id in nd):
raise BadRequest('Cannot delete %s' % id)
self._delProperty(id)
Expand Down
21 changes: 12 additions & 9 deletions src/OFS/PropertySheets.py
Expand Up @@ -163,7 +163,7 @@ def p_self(self):

def valid_property_id(self, id):
if not id or id[:1] == '_' or (id[:3] == 'aq_') \
or (' ' in id) or escape(id) != id:
or (' ' in id) or escape(id, True) != id:
return 0
return 1

Expand Down Expand Up @@ -205,7 +205,7 @@ def _setProperty(self, id, value, type='string', meta=None):
# systems.
self._wrapperCheck(value)
if not self.valid_property_id(id):
raise BadRequest('Invalid property id, %s.' % escape(id))
raise BadRequest('Invalid property id, %s.' % escape(id, True))

if not self.property_extensible_schema__():
raise BadRequest(
Expand All @@ -216,7 +216,7 @@ def _setProperty(self, id, value, type='string', meta=None):
if not (id == 'title' and id not in self.__dict__):
raise BadRequest(
'Invalid property id, <em>%s</em>. It is in use.' %
escape(id))
escape(id, True))
if meta is None:
meta = {}
prop = {'id': id, 'type': type, 'meta': meta}
Expand All @@ -243,10 +243,11 @@ def _updateProperty(self, id, value, meta=None):
# it will used to _replace_ the properties meta data.
self._wrapperCheck(value)
if not self.hasProperty(id):
raise BadRequest('The property %s does not exist.' % escape(id))
raise BadRequest('The property %s does not exist.' %
escape(id, True))
propinfo = self.propertyInfo(id)
if 'w' not in propinfo.get('mode', 'wd'):
raise BadRequest('%s cannot be changed.' % escape(id))
raise BadRequest('%s cannot be changed.' % escape(id, True))
if isinstance(value, str):
proptype = propinfo.get('type', 'string')
if proptype in type_converters:
Expand All @@ -268,14 +269,15 @@ def _delProperty(self, id):
# Delete the property with the given id. If a property with the
# given id does not exist, a ValueError is raised.
if not self.hasProperty(id):
raise BadRequest('The property %s does not exist.' % escape(id))
raise BadRequest('The property %s does not exist.' %
escape(id, True))
vself = self.v_self()
if hasattr(vself, '_reserved_names'):
nd = vself._reserved_names
else:
nd = ()
if ('d' not in self.propertyInfo(id).get('mode', 'wd')) or (id in nd):
raise BadRequest('%s cannot be deleted.' % escape(id))
raise BadRequest('%s cannot be deleted.' % escape(id, True))
delattr(vself, id)
pself = self.p_self()
pself._properties = tuple(
Expand Down Expand Up @@ -303,7 +305,7 @@ def propertyInfo(self, id):
for p in self._propertyMap():
if p['id'] == id:
return p
raise ValueError('The property %s does not exist.' % escape(id))
raise ValueError('The property %s does not exist.' % escape(id, True))

def _propertyMap(self):
# Return a tuple of mappings, giving meta-data for properties.
Expand Down Expand Up @@ -366,7 +368,8 @@ def manage_changeProperties(self, REQUEST=None, **kw):
for name, value in props.items():
if self.hasProperty(name):
if 'w' not in propdict[name].get('mode', 'wd'):
raise BadRequest('%s cannot be changed' % escape(name))
raise BadRequest('%s cannot be changed' %
escape(name, True))
self._updateProperty(name, value)
message = 'Your changes have been saved.'
return self.manage(self, REQUEST, manage_tabs_message=message)
Expand Down
2 changes: 1 addition & 1 deletion src/OFS/Uninstalled.py
Expand Up @@ -48,7 +48,7 @@ class BrokenClass(ZODB_Broken, Explicit, Item, Overridable):
def __getattr__(self, name):
if name[:3] == '_p_':
return BrokenClass.inheritedAttribute('__getattr__')(self, name)
raise AttributeError(escape(name))
raise AttributeError(escape(name, True))

manage = DTMLFile('dtml/brokenEdit', globals())
manage_main = DTMLFile('dtml/brokenEdit', globals())
Expand Down
2 changes: 1 addition & 1 deletion src/OFS/role.py
Expand Up @@ -126,7 +126,7 @@ def manage_changePermissions(self, REQUEST):

if fails:
raise BadRequest('Some permissions had errors: ' +
escape(', '.join(fails)))
escape(', '.join(fails), True))
if REQUEST is not None:
return self.manage_access(REQUEST)

Expand Down
2 changes: 1 addition & 1 deletion src/Products/PageTemplates/tests/secure.pt
@@ -1,5 +1,5 @@
<div xmlns="http://www.w3.org/1999/xhtml"
xmlns:tal="http://xml.zope.org/namespaces/tal">
<span tal:define="soup view/tagsoup | options/soup"
tal:replace="structure python: modules['cgi'].escape(soup)" />
tal:replace="structure python: modules['cgi'].escape(soup, True)" />
</div>
15 changes: 10 additions & 5 deletions src/Products/PageTemplates/tests/test_persistenttemplate.py
@@ -1,10 +1,15 @@
import cgi
import re
import unittest

from Products.PageTemplates.ZopePageTemplate import manage_addPageTemplate
from Testing.ZopeTestCase import ZopeTestCase

try:
from html import escape
except ImportError: # PY2
from cgi import escape


macro_outer = """
<metal:defm define-macro="master">
<metal:defs define-slot="main_slot">
Expand Down Expand Up @@ -118,15 +123,15 @@ def test_simple(self):
result = template().strip()
self.assertEqual(result, u'Hello, World')
editable_text = get_editable_content(template)
self.assertEqual(editable_text, cgi.escape(simple_i18n))
self.assertEqual(editable_text, escape(simple_i18n, False))

def test_escape_on_edit(self):
# check that escapable chars can round-trip intact.
source = u"&gt; &amp; &lt;"
template = self._makeOne('foo', source)
self.assertEqual(template(), source) # nothing to render
editable_text = get_editable_content(template)
self.assertEqual(editable_text, cgi.escape(source))
self.assertEqual(editable_text, escape(source, False))

def test_macro_with_i18n(self):
self._makeOne('macro_outer', macro_outer)
Expand Down Expand Up @@ -208,9 +213,9 @@ def test_edit_with_errors(self):
editable_text = get_editable_content(template)
# and the errors should be in an xml comment at the start of
# the editable text
error_prefix = cgi.escape(
error_prefix = escape(
'<!-- Page Template Diagnostics\n {0}\n-->\n'.format(
'\n '.join(template._v_errors)
'\n '.join(template._v_errors), False
)
)
self.assertTrue(editable_text.startswith(error_prefix))
Expand Down
8 changes: 4 additions & 4 deletions src/ZPublisher/Converters.py
Expand Up @@ -76,7 +76,7 @@ def field2int(v):
return int(v)
except ValueError:
raise ValueError(
"An integer was expected in the value %r" % escape(v)
"An integer was expected in the value %r" % escape(v, True)
)
raise ValueError('Empty entry when <strong>integer</strong> expected')

Expand All @@ -91,7 +91,7 @@ def field2float(v):
except ValueError:
raise ValueError(
"A floating-point number was expected in the value %r" %
escape(v)
escape(v, True)
)
raise ValueError(
'Empty entry when <strong>floating-point number</strong> expected')
Expand All @@ -109,7 +109,7 @@ def field2long(v):
return int(v)
except ValueError:
raise ValueError(
"A long integer was expected in the value %r" % escape(v)
"A long integer was expected in the value %r" % escape(v, True)
)
raise ValueError('Empty entry when <strong>integer</strong> expected')

Expand All @@ -133,7 +133,7 @@ def field2date(v):
try:
v = DateTime(v)
except SyntaxError:
raise SyntaxError("Invalid DateTime " + escape(repr(v)))
raise SyntaxError("Invalid DateTime " + escape(repr(v), True))
return v


Expand Down
17 changes: 9 additions & 8 deletions src/ZPublisher/HTTPRequest.py
Expand Up @@ -650,7 +650,7 @@ def processInputs(
if '<' in attr:
raise ValueError(
"%s is not a valid record attribute name" %
escape(attr))
escape(attr, True))

# defer conversion
if flags & CONVERTED:
Expand Down Expand Up @@ -1469,36 +1469,37 @@ def __str__(self):
result = "<h3>form</h3><table>"
row = '<tr valign="top" align="left"><th>%s</th><td>%s</td></tr>'
for k, v in _filterPasswordFields(self.form.items()):
result = result + row % (escape(k), escape(repr(v)))
result = result + row % (escape(k, False), escape(repr(v), False))
result = result + "</table><h3>cookies</h3><table>"
for k, v in _filterPasswordFields(self.cookies.items()):
result = result + row % (escape(k), escape(repr(v)))
result = result + row % (escape(k, False), escape(repr(v, False)))
result = result + "</table><h3>lazy items</h3><table>"
for k, v in _filterPasswordFields(self._lazies.items()):
result = result + row % (escape(k), escape(repr(v)))
result = result + row % (escape(k, False), escape(repr(v), False))
result = result + "</table><h3>other</h3><table>"
for k, v in _filterPasswordFields(self.other.items()):
if k in ('PARENTS', 'RESPONSE'):
continue
result = result + row % (escape(k), escape(repr(v)))
result = result + row % (escape(k, False), escape(repr(v), False))

for n in "0123456789":
key = "URL%s" % n
try:
result = result + row % (key, escape(self[key]))
result = result + row % (key, escape(self[key], False))
except KeyError:
pass
for n in "0123456789":
key = "BASE%s" % n
try:
result = result + row % (key, escape(self[key]))
result = result + row % (key, escape(self[key], False))
except KeyError:
pass

result = result + "</table><h3>environ</h3><table>"
for k, v in self.environ.items():
if k not in hide_key:
result = result + row % (escape(k), escape(repr(v)))
result = result + row % (
escape(k, False), escape(repr(v), False))
return result + "</table>"

def __repr__(self):
Expand Down
4 changes: 2 additions & 2 deletions src/ZPublisher/HTTPResponse.py
Expand Up @@ -393,7 +393,7 @@ def insertBase(self,
self.body = (
body[:index] +
b'\n<base href="' +
escape(self.base, 1).encode('utf-8') +
escape(self.base, True).encode('utf-8') +
b'" />\n' +
body[index:]
)
Expand Down Expand Up @@ -985,7 +985,7 @@ def notFoundError(self, entry='Unknown'):
exc.detail = (
'Sorry, the requested resource does not exist.'
'<p>Check the URL and try again.</p>'
'<p><b>Resource:</b> %s</p>' % escape(entry))
'<p><b>Resource:</b> %s</p>' % escape(entry, True))
raise exc

# If a resource is forbidden, why reveal that it exists?
Expand Down

0 comments on commit 91d7411

Please sign in to comment.