Edgewall Software

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#13122 closed defect (fixed)

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

Reported by: anonymous Owned by:
Priority: normal Milestone: 1.3.4
Component: general Version:
Severity: normal Keywords: genshi
Cc: Branch:
Release Notes:

Changed Genshi requirement to 0.7 or later.

API Changes:
Internal Changes:

Description

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.

Change History (6)

comment:1 by Christian Boos, 5 years ago

Keywords: genshi added
Milestone: 1.4

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.

in reply to:  1 comment:2 by Ryan J Ollos, 5 years ago

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.

comment:3 by Jun Omae, 5 years ago

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

    diff --git a/trac/util/html.py b/trac/util/html.py
    index f018e16c4..c2f9dbfc0 100644
    a b if genshi:  
    12261226                text = '&#%s;' % name
    12271227            self._enqueue(TEXT, text)
    12281228
     1229        def handle_data(self, text):
     1230            self._enqueue(TEXT, text)
     1231
    12291232        def handle_entityref(self, name):
    12301233            text = None
    12311234            try:

comment:4 by Christian Boos, 5 years ago

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.

comment:5 by Jun Omae, 5 years ago

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

comment:6 by Ryan J Ollos, 5 years ago

Milestone: 1.41.3.4
Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

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.

Note: See TracTickets for help on using tickets.