Edgewall Software
Modify

Opened 11 months ago

Closed 5 months ago

Last modified 5 months ago

#13122 closed defect (fixed)

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

Reported by: anonymous Owned by: Jun Omae
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:

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.

Attachments (0)

Change History (7)

comment:1 by Christian Boos, 11 months 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, 11 months 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, 11 months 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, 11 months 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, 11 months ago

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

comment:6 by Ryan J Ollos, 5 months 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.

comment:7 by Ryan J Ollos, 5 months ago

Owner: set to Jun Omae

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.