Fixed a bug in Message.toFormMarkup() related to encoding UTF-8 encoded form values.

dokai · dokai · commit 0dd2e8109e7d · 2010-10-14T15:50:21.000+03:00
The .toFormMarkup() method that generates a &lt;form&gt; HTML structure had a bug
when the form field values contained UTF-8 encoded strings with characters
outside the 7-bit ASCII space.

If the lxml implementation of the ElementTree API was in use these values
would result in a ValueError being raised (ValueError: All strings must be XML
compatible: Unicode or ASCII, no NULL bytes or control characters). If the
stdlib implementation of ElementTree was used these characters were silently
replaced by their XML character reference equivalents (&amp;#XXX;).

This patch generates the form using Unicode values for everything and then
serializes the form to a UTF-8 encoded string ensuring that the final form is
what is expected and constant regardless of the ElementTree API
implementation.
diff --git a/openid/message.py b/openid/message.py
@@ -298,7 +298,7 @@ def toArgs(self):
         return kvargs
 
     def toFormMarkup(self, action_url, form_tag_attrs=None,
-                     submit_text="Continue"):
+                     submit_text=u"Continue"):
         """Generate HTML form markup that contains the values in this
         message, to be HTTP POSTed as x-www-form-urlencoded UTF-8.
 
@@ -324,28 +324,28 @@ def toFormMarkup(self, action_url, form_tag_attrs=None,
 
         assert action_url is not None
 
-        form = ElementTree.Element('form')
+        form = ElementTree.Element(u'form')
 
         if form_tag_attrs:
             for name, attr in form_tag_attrs.iteritems():
                 form.attrib[name] = attr
 
-        form.attrib['action'] = action_url
-        form.attrib['method'] = 'post'
-        form.attrib['accept-charset'] = 'UTF-8'
-        form.attrib['enctype'] = 'application/x-www-form-urlencoded'
+        form.attrib[u'action'] = oidutil.toUnicode(action_url)
+        form.attrib[u'method'] = u'post'
+        form.attrib[u'accept-charset'] = u'UTF-8'
+        form.attrib[u'enctype'] = u'application/x-www-form-urlencoded'
 
         for name, value in self.toPostArgs().iteritems():
-            attrs = {'type': 'hidden',
-                     'name': name,
-                     'value': value}
-            form.append(ElementTree.Element('input', attrs))
+            attrs = {u'type': u'hidden',
+                     u'name': oidutil.toUnicode(name),
+                     u'value': oidutil.toUnicode(value)}
+            form.append(ElementTree.Element(u'input', attrs))
 
-        submit = ElementTree.Element(
-                'input', {'type':'submit', 'value':submit_text})
+        submit = ElementTree.Element(u'input',
+            {u'type':'submit', u'value':oidutil.toUnicode(submit_text)})
         form.append(submit)
 
-        return ElementTree.tostring(form)
+        return ElementTree.tostring(form, encoding='utf-8')
 
     def toURL(self, base_url):
         """Generate a GET URL with the parameters in this message
diff --git a/openid/oidutil.py b/openid/oidutil.py
@@ -5,7 +5,7 @@
 interesting.
 """
 
-__all__ = ['log', 'appendArgs', 'toBase64', 'fromBase64', 'autoSubmitHTML']
+__all__ = ['log', 'appendArgs', 'toBase64', 'fromBase64', 'autoSubmitHTML', 'toUnicode']
 
 import binascii
 import sys
@@ -21,6 +21,18 @@
     'elementtree.ElementTree',
     ]
 
+def toUnicode(value):
+    """Returns the given argument as a unicode object.
+
+    @param value: A UTF-8 encoded string or a unicode (coercable) object
+    @type message: str or unicode
+
+    @returns: Unicode object representing the input value.
+    """
+    if isinstance(value, str):
+        return value.decode('utf-8')
+    return unicode(value)
+
 def autoSubmitHTML(form, title='OpenID transaction in progress'):
     return """
 <html>
diff --git a/openid/test/oidutil.py b/openid/test/oidutil.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 import unittest
 import codecs
 import string
@@ -53,7 +54,17 @@ def runTest(self):
     def shortDescription(self):
         return self.desc
 
+class TestUnicodeConversion(unittest.TestCase):
 
+    def test_toUnicode(self):
+        # Unicode objects pass through
+        self.failUnless(isinstance(oidutil.toUnicode(u'fööbär'), unicode))
+        self.assertEquals(oidutil.toUnicode(u'fööbär'), u'fööbär')
+        # UTF-8 encoded string are decoded
+        self.failUnless(isinstance(oidutil.toUnicode('fööbär'), unicode))
+        self.assertEquals(oidutil.toUnicode('fööbär'), u'fööbär')
+        # Other encodings raise exceptions
+        self.assertRaises(UnicodeDecodeError, lambda: oidutil.toUnicode(u'fööbär'.encode('latin-1')))
 
 class TestSymbol(unittest.TestCase):
     def testCopyHash(self):
@@ -154,6 +165,7 @@ def buildAppendTests():
 def pyUnitTests():
     some = buildAppendTests()
     some.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(TestSymbol))
+    some.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(TestUnicodeConversion))
     return some
 
 def test_appendArgs():
diff --git a/openid/test/test_message.py b/openid/test/test_message.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 from openid import message
 from openid import oidutil
 from openid.extensions import sreg
@@ -445,7 +446,6 @@ def test_toURL(self):
     def test_isOpenID1(self):
         self.failUnless(self.msg.isOpenID1())
 
-
 class OpenID2MessageTest(unittest.TestCase):
     def setUp(self):
         self.msg = message.Message.fromPostArgs({'openid.mode':'error',
@@ -846,6 +846,29 @@ def test_toFormMarkup(self):
         self._checkForm(html, m, self.action_url,
                         self.form_tag_attrs, self.submit_text)
 
+    def test_toFormMarkup_bug_with_utf8_values(self):
+        postargs = {
+            'openid.ns': message.OPENID2_NS,
+            'openid.mode': 'checkid_setup',
+            'openid.identity': 'http://bogus.example.invalid:port/',
+            'openid.assoc_handle': 'FLUB',
+            'openid.return_to': 'Neverland',
+            'ünicöde_key' : 'ünicöde_välüe',
+            }
+        m = message.Message.fromPostArgs(postargs)
+        # Calling m.toFormMarkup with lxml used for ElementTree will throw
+        # a ValueError.
+        html = m.toFormMarkup(self.action_url, self.form_tag_attrs,
+                              self.submit_text)
+        # Using the (c)ElementTree from stdlib will result in the UTF-8
+        # encoded strings to be converted to XML character references,
+        # "ünicöde_key" becomes "&#195;&#188;nic&#195;&#182;de_key" and
+        # "ünicöde_välüe" becomes "&#195;&#188;nic&#195;&#182;de_v&#195;&#164;l&#195;&#188;e"
+        self.failIf('&#195;&#188;nic&#195;&#182;de_key' in html,
+                    'UTF-8 bytes should not convert to XML character references')
+        self.failIf('&#195;&#188;nic&#195;&#182;de_v&#195;&#164;l&#195;&#188;e' in html,
+                    'UTF-8 bytes should not convert to XML character references')
+
     def test_overrideMethod(self):
         """Be sure that caller cannot change form method to GET."""
         m = message.Message.fromPostArgs(self.postargs)
