trac.util.tests.html.TracHTMLSanitizerLegacyGenshiTestCase: TypeError: decode() argument 1 must be string, not None

[anonymous]

Running the unit tests I get one failure:

======================================================================
ERROR: test_special_characters_data (trac.util.tests.html.TracHTMLSanitizerLegacyGenshiTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\trac\trunk\trac\util\tests\html.py", line 287, in test_special_characters_data
    test('<p>&amp;</p>',            '<p>&</p>')
  File "c:\trac\trunk\trac\util\tests\html.py", line 336, in _assert_sanitize
    self.assertEqual(expected, self.sanitize(content))
  File "c:\trac\trunk\trac\util\tests\html.py", line 354, in sanitize
    return unicode(HTML(html, encoding='utf-8') | sanitizer)
  File "c:\trac\trunk\trac\util\html.py", line 1251, in HTML
    return Stream(list(parser))
  File "build\bdist.win-amd64\egg\genshi\core.py", line 272, in _ensure
    event = stream.next()
  File "build\bdist.win-amd64\egg\genshi\input.py", line 432, in _coalesce
    for kind, data, pos in chain(stream, [(None, None, None)]):
  File "build\bdist.win-amd64\egg\genshi\input.py", line 327, in _generate
    self.feed(data)
  File "c:\python27\Lib\HTMLParser.py", line 117, in feed
    self.goahead(0)
  File "c:\python27\Lib\HTMLParser.py", line 222, in goahead
    self.handle_data("&")
  File "build\bdist.win-amd64\egg\genshi\input.py", line 379, in handle_data
    text = text.decode(self.encoding, 'replace')
TypeError: decode() argument 1 must be string, not None

----------------------------------------------------------------------

Searching for TracHTMLSanitizerLegacyGenshiTestCase finds #12847, but I don't understand if it's related.

Genshi version is 0.6.1.

[Christian Boos]

Indeed, reproduced with 0.6.1 and Python 2.7.14 on Windows.

All the other tests seem to pass correctly with 0.6.1, and I can't see at first glance if this test failure indicates a real problem, or just the test itself triggering this failure due to the way it's written.

Now the question is, is there a compelling argument to keep backward compatibility with the 6 years old Genshi ​0.6.1? The latest released is ​0.7.1 and with that one, this problem doesn't appear.

So unless someone comes up with a simple fix at the test level, I'd go for raising the required version to 0.7.1. That is also the safest choice in case the problem is more serious than just a superficial problem in the test.

[Ryan J Ollos]

Replying to Christian Boos:

Now the question is, is there a compelling argument to keep backward compatibility with the 6 years old Genshi ​0.6.1?

We have the Compatible Distros Table to help with decisions like this one, and looks like it would be okay to raise the requirement to at least Genshi 0.7. The table could use some updates to account for the latest of some distros, such as Ubuntu 18.04 and Debian 9.6.

[Jun Omae]

The instance checking in handle_data() has been removed in Genshi 0.7.1, ​https://github.com/edgewall/genshi/commit/99619128d2ca86baf1daab4e038bad987b0e0b2d#diff-186ec79af775a8dd488fcc814a932971.

Ad hoc patch (verified with Genshi 0.6, 0.6.1, 0.7 and 0.7.1):

trac/util/html.py

@@ -1226 +1226 @@
                 text = '&#%s;' % name
             self._enqueue(TEXT, text)
 
+        def handle_data(self, text):
+            self._enqueue(TEXT, text)
+
         def handle_entityref(self, name):
             text = None
             try:

[Christian Boos]

Ah, I didn't know about r16613, I guess I've some catch-up to do ;-)

Yes, so that change looks good to me. As we're placing Genshi on a backward compatibility track for 1.4.x, minimizing the changes needed and still supporting 0.6.x at little cost is fine by me, but I think that if other more serious problems are found with 0.6.x later on, we shouldn't hesitate to update the requirements.

[Jun Omae]

No problem. Raising the version to >=0.7.1 on trunk looks good to me.

[Ryan J Ollos]

Bumped Genshi requirement to 0.7 or later in r16941.

I'm fine bumping to a later version of Genshi, but went ahead with the change so we can get the releases out.