588458 - Crash in bleach with <a>

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Jeff Balogh (:jbalogh)]

I think this is fixed upstream, but we've been avoiding upstream since their treewalker shenanigans tend to crash.

POST:<QueryDict: {u'description': [u'\r\n<xml id="X"><a><b><script>document.vulnerable=true;</script>;</b></a></xml>\r\n'], u'listed': [u'True'], u'csrfmiddlewaretoken': [u'b4f7dbb32ce37ca074ed6145ba8da5c1'], u'icon': [u''], u'slug': [u'HISTEVEY'], u'name': [u'tags/']}>,

 File "/data/amo_python/www/preview/zamboni/vendor/src/bleach/bleach/__init__.py", line 121, in linkify_nodes
   linkify_nodes(node)

 File "/data/amo_python/www/preview/zamboni/vendor/src/bleach/bleach/__init__.py", line 118, in linkify_nodes
   href = self.filter_url(node.attributes['href'])

KeyError: 'href'

Comment 1

[James Socol [:jsocol, :james]]

http://github.com/jsocol/bleach/commit/e6225c26a9bfabcd6c849e8437a2a5390051023a

Comment 2

[James Socol [:jsocol, :james]]

I would totally accept a patch to switch that from `try...except` to `if 'href' in...`

Comment 3

[Jeff Balogh (:jbalogh)]

We need to upgrade bleach, but we don't want more errors like bug 568317, so I asked jsocol to pull in http://github.com/jbalogh/bleach/commit/df924d79b0.

It uses logging to discover more error cases and to make clouserw beam.

Comment 4

[Jeff Balogh (:jbalogh)]

http://github.com/jsocol/bleach/commit/d4bdcba5565d0b6071ad75fb5008bb835a0ff936

Comment 5

[Stephen Donner [:stephend] Not actively reading bugmail]

Jeff - is there a simplified testcase I can use?  Thanks.

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #5)
> Jeff - is there a simplified testcase I can use?  Thanks.

Verified FIXED using the string in comment 0.