Repository: mozilla/bleach

Issue:
move html5lib override code into separate "html5lib shim" module
We're increasingly subclassing html5lib classes and overriding behavior and that code is getting tangled with Bleach's behavior code. I'm concerned this is getting worse over time and it'll become increasingly more difficult to upgrade it to support new versions of html5lib.

I think it behooves us to centralize and move the html5lib shim code into a separate module with a "defined API" and then change the Bleach code to sit on top of that.

Discussion:

Patch:
diff --git a/CHANGES b/CHANGES
index 423c1ec..427363e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,7 +1,7 @@
 Bleach changes
 ==============
 
-Version 2.1.5 (in development)
+Version 3.0.0 (in development)
 ------------------------------
 
 **Security fixes**
@@ -10,12 +10,32 @@ None
 
 **Backwards incompatible changes**
 
-None
+* A bunch of functions were moved from one module to another:
+
+  ``bleach.sanitizer`` -> ``bleach.html5lib_shim``:
+
+  * ``convert_entity``
+  * ``convert_entities``
+  * ``match_entity``
+  * ``next_possible_entity``
+  * ``BleachHTMLSerializer``
+  * ``BleachHTMLTokenizer``
+  * ``BleachHTMLParser``
+
+  These weren't documented and aren't part of the public API, but people
+  read code and might be using them so we're considering it an incompatible
+  API change.
+
+  If you're using them, you'll need to update your code.
 
 **Features**
 
-* No longer depends on html5lib. html5lib==1.0.1 was vendored into Bleach.
-  (#386)
+* Bleach no longer depends on html5lib. html5lib==1.0.1 is now vendored into
+  Bleach. You can remove it from your requirements file if none of your other
+  requirements require html5lib.
+
+  This means Bleach will now work fine with other libraries that depend on
+  html5lib regardless of what version of html5lib they require. (#386)
 
 **Bug fixes**
 
diff --git a/bleach/__init__.py b/bleach/__init__.py
index f953fc5..b4a927a 100644
--- a/bleach/__init__.py
+++ b/bleach/__init__.py
@@ -20,7 +20,7 @@ from bleach.sanitizer import (
 # yyyymmdd
 __releasedate__ = ''
 # x.y.z or x.y.z.dev0 -- semver
-__version__ = '2.1.5.dev0'
+__version__ = '3.0.0.dev0'
 VERSION = parse_version(__version__)
 
 
diff --git a/bleach/html5lib_shim.py b/bleach/html5lib_shim.py
new file mode 100644
index 0000000..e84011d
--- /dev/null
+++ b/bleach/html5lib_shim.py
@@ -0,0 +1,266 @@
+# flake8: noqa
+"""
+Shim module between Bleach and html5lib. This makes it easier to upgrade the
+html5lib library without having to change a lot of code.
+"""
+
+from __future__ import unicode_literals
+
+import re
+import string
+
+import six
+
+from bleach._vendor.html5lib import (
+    HTMLParser,
+    getTreeWalker,
+)
+from bleach._vendor.html5lib.constants import (
+    entities,
+    namespaces,
+    prefixes,
+    tokenTypes,
+)
+from bleach._vendor.html5lib.constants import _ReparseException as ReparseException
+from bleach._vendor.html5lib.filters.base import Filter
+from bleach._vendor.html5lib.filters.sanitizer import allowed_protocols
+from bleach._vendor.html5lib.filters.sanitizer import Filter as SanitizerFilter
+from bleach._vendor.html5lib.serializer import HTMLSerializer
+from bleach._vendor.html5lib._tokenizer import HTMLTokenizer
+from bleach._vendor.html5lib._trie import Trie
+
+
+#: Map of entity name to expanded entity
+ENTITIES = entities
+
+#: Trie of html entity string -> character representation
+ENTITIES_TRIE = Trie(ENTITIES)
+
+
+class BleachHTMLTokenizer(HTMLTokenizer):
+    """Tokenizer that doesn't consume character entities"""
+    def consumeEntity(self, allowedChar=None, fromAttribute=False):
+        # We don't want to consume and convert entities, so this overrides the
+        # html5lib tokenizer's consumeEntity so that it's now a no-op.
+        #
+        # However, when that gets called, it's consumed an &, so we put that in
+        # the stream.
+        if fromAttribute:
+            self.currentToken['data'][-1][1] += '&'
+
+        else:
+            self.tokenQueue.append({"type": tokenTypes['Characters'], "data": '&'})
+
+
+class BleachHTMLParser(HTMLParser):
+    """Parser that uses BleachHTMLTokenizer"""
+    def _parse(self, stream, innerHTML=False, container='div', scripting=False, **kwargs):
+        # Override HTMLParser so we can swap out the tokenizer for our own.
+        self.innerHTMLMode = innerHTML
+        self.container = container
+        self.scripting = scripting
+        self.tokenizer = BleachHTMLTokenizer(stream, parser=self, **kwargs)
+        self.reset()
+
+        try:
+            self.mainLoop()
+        except ReparseException:
+            self.reset()
+            self.mainLoop()
+
+
+def convert_entity(value):
+    """Convert an entity (minus the & and ; part) into what it represents
+
+    This handles numeric, hex, and text entities.
+
+    :arg value: the string (minus the ``&`` and ``;`` part) to convert
+
+    :returns: unicode character or None if it's an ambiguous ampersand that
+        doesn't match a character entity
+
+    """
+    if value[0] == '#':
+        if value[1] in ('x', 'X'):
+            return six.unichr(int(value[2:], 16))
+        return six.unichr(int(value[1:], 10))
+
+    return ENTITIES.get(value, None)
+
+
+def convert_entities(text):
+    """Converts all found entities in the text
+
+    :arg text: the text to convert entities in
+
+    :returns: unicode text with converted entities
+
+    """
+    if '&' not in text:
+        return text
+
+    new_text = []
+    for part in next_possible_entity(text):
+        if not part:
+            continue
+
+        if part.startswith('&'):
+            entity = match_entity(part)
+            if entity is not None:
+                converted = convert_entity(entity)
+
+                # If it's not an ambiguous ampersand, then replace with the
+                # unicode character. Otherwise, we leave the entity in.
+                if converted is not None:
+                    new_text.append(converted)
+                    remainder = part[len(entity) + 2:]
+                    if part:
+                        new_text.append(remainder)
+                    continue
+
+        new_text.append(part)
+
+    return u''.join(new_text)
+
+
+def match_entity(stream):
+    """Returns first entity in stream or None if no entity exists
+
+    Note: For Bleach purposes, entities must start with a "&" and end with
+    a ";". This ignoresambiguous character entities that have no ";" at the
+    end.
+
+    :arg stream: the character stream
+
+    :returns: ``None`` or the entity string without "&" or ";"
+
+    """
+    # Nix the & at the beginning
+    if stream[0] != '&':
+        raise ValueError('Stream should begin with "&"')
+
+    stream = stream[1:]
+
+    stream = list(stream)
+    possible_entity = ''
+    end_characters = '<&=;' + string.whitespace
+
+    # Handle number entities
+    if stream and stream[0] == '#':
+        possible_entity = '#'
+        stream.pop(0)
+
+        if stream and stream[0] in ('x', 'X'):
+            allowed = '0123456789abcdefABCDEF'
+            possible_entity += stream.pop(0)
+        else:
+            allowed = '0123456789'
+
+        # FIXME(willkg): Do we want to make sure these are valid number
+        # entities? This doesn't do that currently.
+        while stream and stream[0] not in end_characters:
+            c = stream.pop(0)
+            if c not in allowed:
+                break
+            possible_entity += c
+
+        if possible_entity and stream and stream[0] == ';':
+            return possible_entity
+        return None
+
+    # Handle character entities
+    while stream and stream[0] not in end_characters:
+        c = stream.pop(0)
+        if not ENTITIES_TRIE.has_keys_with_prefix(possible_entity):
+            break
+        possible_entity += c
+
+    if possible_entity and stream and stream[0] == ';':
+        return possible_entity
+
+    return None
+
+
+AMP_SPLIT_RE = re.compile('(&)')
+
+
+def next_possible_entity(text):
+    """Takes a text and generates a list of possible entities
+
+    :arg text: the text to look at
+
+    :returns: generator where each part (except the first) starts with an
+        "&"
+
+    """
+    for i, part in enumerate(AMP_SPLIT_RE.split(text)):
+        if i == 0:
+            yield part
+        elif i % 2 == 0:
+            yield '&' + part
+
+
+class BleachHTMLSerializer(HTMLSerializer):
+    """HTMLSerializer that undoes & -> &amp; in attributes"""
+    def escape_base_amp(self, stoken):
+        """Escapes just bare & in HTML attribute values"""
+        # First, undo escaping of &. We need to do this because html5lib's
+        # HTMLSerializer expected the tokenizer to consume all the character
+        # entities and convert them to their respective characters, but the
+        # BleachHTMLTokenizer doesn't do that. For example, this fixes
+        # &amp;entity; back to &entity; .
+        stoken = stoken.replace('&amp;', '&')
+
+        # However, we do want all bare & that are not marking character
+        # entities to be changed to &amp;, so let's do that carefully here.
+        for part in next_possible_entity(stoken):
+            if not part:
+                continue
+
+            if part.startswith('&'):
+                entity = match_entity(part)
+                # Only leave entities in that are not ambiguous. If they're
+                # ambiguous, then we escape the ampersand.
+                if entity is not None and convert_entity(entity) is not None:
+                    yield '&' + entity + ';'
+
+                    # Length of the entity plus 2--one for & at the beginning
+                    # and and one for ; at the end
+                    part = part[len(entity) + 2:]
+                    if part:
+                        yield part
+                    continue
+
+            yield part.replace('&', '&amp;')
+
+    def serialize(self, treewalker, encoding=None):
+        """Wrap HTMLSerializer.serialize and conver & to &amp; in attribute values
+
+        Note that this converts & to &amp; in attribute values where the & isn't
+        already part of an unambiguous character entity.
+
+        """
+        in_tag = False
+        after_equals = False
+
+        for stoken in super(BleachHTMLSerializer, self).serialize(treewalker, encoding):
+            if in_tag:
+                if stoken == '>':
+                    in_tag = False
+
+                elif after_equals:
+                    if stoken != '"':
+                        for part in self.escape_base_amp(stoken):
+                            yield part
+
+                        after_equals = False
+                        continue
+
+                elif stoken == '=':
+                    after_equals = True
+
+                yield stoken
+            else:
+                if stoken.startswith('<'):
+                    in_tag = True
+                yield stoken
diff --git a/bleach/linkifier.py b/bleach/linkifier.py
index ef8afc6..3c8c3ee 100644
--- a/bleach/linkifier.py
+++ b/bleach/linkifier.py
@@ -2,12 +2,8 @@ from __future__ import unicode_literals
 import re
 import six
 
-from bleach._vendor import html5lib
-from bleach._vendor.html5lib.filters.base import Filter
-from bleach._vendor.html5lib.filters.sanitizer import allowed_protocols
-from bleach._vendor.html5lib.serializer import HTMLSerializer
-
 from bleach import callbacks as linkify_callbacks
+from bleach import html5lib_shim
 from bleach.utils import alphabetize_attributes, force_unicode
 
 
@@ -33,7 +29,7 @@ TLDS = """ac ad ae aero af ag ai al am an ao aq ar arpa as asia at au aw ax az
 TLDS.reverse()
 
 
-def build_url_re(tlds=TLDS, protocols=allowed_protocols):
+def build_url_re(tlds=TLDS, protocols=html5lib_shim.allowed_protocols):
     """Builds the url regex used by linkifier
 
    If you want a different set of tlds or allowed protocols, pass those in
@@ -114,9 +110,9 @@ class Linker(object):
         self.url_re = url_re
         self.email_re = email_re
 
-        self.parser = html5lib.HTMLParser(namespaceHTMLElements=False)
-        self.walker = html5lib.getTreeWalker('etree')
-        self.serializer = HTMLSerializer(
+        self.parser = html5lib_shim.HTMLParser(namespaceHTMLElements=False)
+        self.walker = html5lib_shim.getTreeWalker('etree')
+        self.serializer = html5lib_shim.HTMLSerializer(
             quote_attr_values='always',
             omit_optional_tags=False,
 
@@ -157,7 +153,7 @@ class Linker(object):
         return self.serializer.render(filtered)
 
 
-class LinkifyFilter(Filter):
+class LinkifyFilter(html5lib_shim.Filter):
     """html5lib filter that linkifies text
 
     This will do the following:
diff --git a/bleach/sanitizer.py b/bleach/sanitizer.py
index 7cbb9e8..05e2675 100644
--- a/bleach/sanitizer.py
+++ b/bleach/sanitizer.py
@@ -1,35 +1,16 @@
 from __future__ import unicode_literals
+
 from itertools import chain
 import re
-import string
 
 import six
 from six.moves.urllib.parse import urlparse
 from xml.sax.saxutils import unescape
 
-from bleach._vendor import html5lib
-from bleach._vendor.html5lib.constants import (
-    entities,
-    namespaces,
-    prefixes,
-    tokenTypes,
-)
-from bleach._vendor.html5lib.constants import _ReparseException as ReparseException
-from bleach._vendor.html5lib.filters.base import Filter
-from bleach._vendor.html5lib.filters import sanitizer
-from bleach._vendor.html5lib.serializer import HTMLSerializer
-from bleach._vendor.html5lib._tokenizer import HTMLTokenizer
-from bleach._vendor.html5lib._trie import Trie
-
+from bleach import html5lib_shim
 from bleach.utils import alphabetize_attributes, force_unicode
 
 
-#: Map of entity name to expanded entity
-ENTITIES = entities
-
-#: Trie of html entity string -> character representation
-ENTITIES_TRIE = Trie(ENTITIES)
-
 #: List of allowed tags
 ALLOWED_TAGS = [
     'a',
@@ -54,17 +35,12 @@ ALLOWED_ATTRIBUTES = {
     'acronym': ['title'],
 }
 
-
 #: List of allowed styles
 ALLOWED_STYLES = []
 
-
 #: List of allowed protocols
 ALLOWED_PROTOCOLS = ['http', 'https', 'mailto']
 
-
-AMP_SPLIT_RE = re.compile('(&)')
-
 #: Invisible characters--0 to and including 31 except 9 (tab), 10 (lf), and 13 (cr)
 INVISIBLE_CHARACTERS = ''.join([chr(c) for c in chain(range(0, 9), range(11, 13), range(14, 32))])
 
@@ -79,90 +55,6 @@ INVISIBLE_CHARACTERS_RE = re.compile(
 INVISIBLE_REPLACEMENT_CHAR = '?'
 
 
-def convert_entity(value):
-    """Convert an entity (minus the & and ; part) into what it represents
-
-    This handles numeric, hex, and text entities.
-
-    :arg value: the string (minus the ``&`` and ``;`` part) to convert
-
-    :returns: unicode character or None if it's an ambiguous ampersand that
-        doesn't match a character entity
-
-    """
-    if value[0] == '#':
-        if value[1] in ('x', 'X'):
-            return six.unichr(int(value[2:], 16))
-        return six.unichr(int(value[1:], 10))
-
-    return ENTITIES.get(value, None)
-
-
-def convert_entities(text):
-    """Converts all found entities in the text
-
-    :arg text: the text to convert entities in
-
-    :returns: unicode text with converted entities
-
-    """
-    if '&' not in text:
-        return text
-
-    new_text = []
-    for part in next_possible_entity(text):
-        if not part:
-            continue
-
-        if part.startswith('&'):
-            entity = match_entity(part)
-            if entity is not None:
-                converted = convert_entity(entity)
-
-                # If it's not an ambiguous ampersand, then replace with the
-                # unicode character. Otherwise, we leave the entity in.
-                if converted is not None:
-                    new_text.append(converted)
-                    remainder = part[len(entity) + 2:]
-                    if part:
-                        new_text.append(remainder)
-                    continue
-
-        new_text.append(part)
-
-    return u''.join(new_text)
-
-
-class BleachHTMLTokenizer(HTMLTokenizer):
-    def consumeEntity(self, allowedChar=None, fromAttribute=False):
-        # We don't want to consume and convert entities, so this overrides the
-        # html5lib tokenizer's consumeEntity so that it's now a no-op.
-        #
-        # However, when that gets called, it's consumed an &, so we put that in
-        # the stream.
-        if fromAttribute:
-            self.currentToken['data'][-1][1] += '&'
-
-        else:
-            self.tokenQueue.append({"type": tokenTypes['Characters'], "data": '&'})
-
-
-class BleachHTMLParser(html5lib.HTMLParser):
-    def _parse(self, stream, innerHTML=False, container="div", scripting=False, **kwargs):
-        # Override HTMLParser so we can swap out the tokenizer for our own.
-        self.innerHTMLMode = innerHTML
-        self.container = container
-        self.scripting = scripting
-        self.tokenizer = BleachHTMLTokenizer(stream, parser=self, **kwargs)
-        self.reset()
-
-        try:
-            self.mainLoop()
-        except ReparseException:
-            self.reset()
-            self.mainLoop()
-
-
 class Cleaner(object):
     """Cleaner for cleaning HTML fragments of malicious content
 
@@ -223,9 +115,9 @@ class Cleaner(object):
         self.strip_comments = strip_comments
         self.filters = filters or []
 
-        self.parser = BleachHTMLParser(namespaceHTMLElements=False)
-        self.walker = html5lib.getTreeWalker('etree')
-        self.serializer = BleachHTMLSerializer(
+        self.parser = html5lib_shim.BleachHTMLParser(namespaceHTMLElements=False)
+        self.walker = html5lib_shim.getTreeWalker('etree')
+        self.serializer = html5lib_shim.BleachHTMLSerializer(
             quote_attr_values='always',
             omit_optional_tags=False,
             escape_lt_in_attrs=True,
@@ -325,80 +217,7 @@ def attribute_filter_factory(attributes):
     raise ValueError('attributes needs to be a callable, a list or a dict')
 
 
-def match_entity(stream):
-    """Returns first entity in stream or None if no entity exists
-
-    Note: For Bleach purposes, entities must start with a "&" and end with
-    a ";".
-
-    :arg stream: the character stream
-
-    :returns: ``None`` or the entity string without "&" or ";"
-
-    """
-    # Nix the & at the beginning
-    if stream[0] != '&':
-        raise ValueError('Stream should begin with "&"')
-
-    stream = stream[1:]
-
-    stream = list(stream)
-    possible_entity = ''
-    end_characters = '<&=;' + string.whitespace
-
-    # Handle number entities
-    if stream and stream[0] == '#':
-        possible_entity = '#'
-        stream.pop(0)
-
-        if stream and stream[0] in ('x', 'X'):
-            allowed = '0123456789abcdefABCDEF'
-            possible_entity += stream.pop(0)
-        else:
-            allowed = '0123456789'
-
-        # FIXME(willkg): Do we want to make sure these are valid number
-        # entities? This doesn't do that currently.
-        while stream and stream[0] not in end_characters:
-            c = stream.pop(0)
-            if c not in allowed:
-                break
-            possible_entity += c
-
-        if possible_entity and stream and stream[0] == ';':
-            return possible_entity
-        return None
-
-    # Handle character entities
-    while stream and stream[0] not in end_characters:
-        c = stream.pop(0)
-        if not ENTITIES_TRIE.has_keys_with_prefix(possible_entity):
-            break
-        possible_entity += c
-
-    if possible_entity and stream and stream[0] == ';':
-        return possible_entity
-
-    return None
-
-
-def next_possible_entity(text):
-    """Takes a text and generates a list of possible entities
-
-    :arg text: the text to look at
-
-    :returns: generator where each part (except the first) starts with an
-        "&"
-
-    """
-    for i, part in enumerate(AMP_SPLIT_RE.split(text)):
-        if i == 0:
-            yield part
-        elif i % 2 == 0:
-            yield '&' + part
-
-
-class BleachSanitizerFilter(sanitizer.Filter):
+class BleachSanitizerFilter(html5lib_shim.SanitizerFilter):
     """html5lib Filter that sanitizes text
 
     This filter can be used anywhere html5lib filters can be used.
@@ -430,14 +249,13 @@ class BleachSanitizerFilter(sanitizer.Filter):
 
         """
         self.attr_filter = attribute_filter_factory(attributes)
-
         self.strip_disallowed_elements = strip_disallowed_elements
         self.strip_html_comments = strip_html_comments
 
         return super(BleachSanitizerFilter, self).__init__(source, **kwargs)
 
     def __iter__(self):
-        for token in Filter.__iter__(self):
+        for token in html5lib_shim.Filter.__iter__(self):
             ret = self.sanitize_token(token)
 
             if not ret:
@@ -523,12 +341,12 @@ class BleachSanitizerFilter(sanitizer.Filter):
 
         # For each possible entity that starts with a "&", we try to extract an
         # actual entity and re-tokenize accordingly
-        for part in next_possible_entity(data):
+        for part in html5lib_shim.next_possible_entity(data):
             if not part:
                 continue
 
             if part.startswith('&'):
-                entity = match_entity(part)
+                entity = html5lib_shim.match_entity(part)
                 if entity is not None:
                     new_tokens.append({'type': 'Entity', 'name': entity})
                     # Length of the entity plus 2--one for & at the beginning
@@ -556,7 +374,7 @@ class BleachSanitizerFilter(sanitizer.Filter):
         # different than the original value.
 
         # Convert all character entities in the value
-        new_value = convert_entities(value)
+        new_value = html5lib_shim.convert_entities(value)
 
         # Nix backtick, space characters, and control characters
         new_value = re.sub(
@@ -645,7 +463,9 @@ class BleachSanitizerFilter(sanitizer.Filter):
 
                 # Drop href and xlink:href attr for svg elements with non-local IRIs
                 if (None, token['name']) in self.svg_allow_local_href:
-                    if namespaced_name in [(None, 'href'), (namespaces['xlink'], 'href')]:
+                    if namespaced_name in [
+                            (None, 'href'), (html5lib_shim.namespaces['xlink'], 'href')
+                    ]:
                         if re.search(r'^\s*[^#\s]', val):
                             continue
 
@@ -676,10 +496,10 @@ class BleachSanitizerFilter(sanitizer.Filter):
 
                 # Figure out namespaced name if the namespace is appropriate
                 # and exists; if the ns isn't in prefixes, then drop it.
-                if ns is None or ns not in prefixes:
+                if ns is None or ns not in html5lib_shim.prefixes:
                     namespaced_name = name
                 else:
-                    namespaced_name = '%s:%s' % (prefixes[ns], name)
+                    namespaced_name = '%s:%s' % (html5lib_shim.prefixes[ns], name)
 
                 attrs.append(' %s="%s"' % (
                     namespaced_name,
@@ -704,7 +524,7 @@ class BleachSanitizerFilter(sanitizer.Filter):
     def sanitize_css(self, style):
         """Sanitizes css in style tags"""
         # Convert entities in the style so that it can be parsed as CSS
-        style = convert_entities(style)
+        style = html5lib_shim.convert_entities(style)
 
         # Drop any url values before we do anything else
         style = re.compile('url\s*\(\s*[^\s)]+?\s*\)\s*').sub(' ', style)
@@ -737,59 +557,3 @@ class BleachSanitizerFilter(sanitizer.Filter):
                 clean.append(prop + ': ' + value + ';')
 
         return ' '.join(clean)
-
-
-class BleachHTMLSerializer(HTMLSerializer):
-    """Wraps the HTMLSerializer and undoes & -> &amp; in attributes"""
-    def escape_base_amp(self, stoken):
-        """Escapes bare & in HTML attribute values"""
-        # First, undo what the HTMLSerializer did
-        stoken = stoken.replace('&amp;', '&')
-
-        # Then, escape any bare &
-        for part in next_possible_entity(stoken):
-            if not part:
-                continue
-
-            if part.startswith('&'):
-                entity = match_entity(part)
-                # Only leave entities in that are not ambiguous. If they're
-                # ambiguous, then we escape the ampersand.
-                if entity is not None and convert_entity(entity) is not None:
-                    yield '&' + entity + ';'
-
-                    # Length of the entity plus 2--one for & at the beginning
-                    # and and one for ; at the end
-                    part = part[len(entity) + 2:]
-                    if part:
-                        yield part
-                    continue
-
-            yield part.replace('&', '&amp;')
-
-    def serialize(self, treewalker, encoding=None):
-        """Wrap HTMLSerializer.serialize and escape bare & in attributes"""
-        in_tag = False
-        after_equals = False
-
-        for stoken in super(BleachHTMLSerializer, self).serialize(treewalker, encoding):
-            if in_tag:
-                if stoken == '>':
-                    in_tag = False
-
-                elif after_equals:
-                    if stoken != '"':
-                        for part in self.escape_base_amp(stoken):
-                            yield part
-
-                        after_equals = False
-                        continue
-
-                elif stoken == '=':
-                    after_equals = True
-
-                yield stoken
-            else:
-                if stoken.startswith('<'):
-                    in_tag = True
-                yield stoken


Test patch:
diff --git a/tests/test_clean.py b/tests/test_clean.py
index 96f0916..10d9c89 100644
--- a/tests/test_clean.py
+++ b/tests/test_clean.py
@@ -1,10 +1,10 @@
 import os
 
-from bleach._vendor.html5lib.filters.base import Filter
 import pytest
 
 from bleach import clean
-from bleach.sanitizer import convert_entities, Cleaner
+from bleach.html5lib_shim import Filter
+from bleach.sanitizer import Cleaner
 
 
 def test_clean_idempotent():
@@ -744,24 +744,6 @@ def test_invisible_characters(data, expected):
     assert clean(data) == expected
 
 
-@pytest.mark.parametrize('data, expected', [
-    # Strings without character entities pass through as is
-    ('', ''),
-    ('abc', 'abc'),
-
-    # Handles character entities--both named and numeric
-    ('&nbsp;', u'\xa0'),
-    ('&#32;', ' '),
-    ('&#x20;', ' '),
-
-    # Handles ambiguous ampersand
-    ('&xx;', '&xx;'),
-])
-def test_convert_entities(data, expected):
-    print(repr(convert_entities(data)))
-    assert convert_entities(data) == expected
-
-
 def test_nonexistent_namespace():
     """Verify if the namespace doesn't exist, it doesn't fail with a KeyError
 
diff --git a/tests/test_html5lib_shim.py b/tests/test_html5lib_shim.py
new file mode 100644
index 0000000..3bd859a
--- /dev/null
+++ b/tests/test_html5lib_shim.py
@@ -0,0 +1,77 @@
+import pytest
+
+from bleach import html5lib_shim
+
+
+@pytest.mark.parametrize('data, expected', [
+    # Strings without character entities pass through as is
+    ('', ''),
+    ('abc', 'abc'),
+
+    # Handles character entities--both named and numeric
+    ('&nbsp;', u'\xa0'),
+    ('&#32;', ' '),
+    ('&#x20;', ' '),
+
+    # Handles ambiguous ampersand
+    ('&xx;', '&xx;'),
+
+    # Handles multiple entities in the same string
+    ('this &amp; that &amp; that', 'this & that & that'),
+])
+def test_convert_entities(data, expected):
+    assert html5lib_shim.convert_entities(data) == expected
+
+
+@pytest.mark.parametrize('data, expected', [
+    ('', ''),
+    ('text', 'text'),
+
+    # & in Characters is escaped
+    ('&', '&amp;'),
+
+    # FIXME(willkg): This happens because the BleachHTMLTokenizer is ignoring
+    # character entities. What it should be doing is creating Entity tokens
+    # for character entities.
+    #
+    # That was too hard at the time I was fixing it, so I fixed it in
+    # BleachSanitizerFilter. When that gest fixed correctly in the tokenizer,
+    # then this test cases will get fixed.
+    ('a &amp; b', 'a &amp;amp; b'),    # should be 'a &amp; b'
+
+    # & in HTML attribute values are escaped
+    (
+        '<a href="http://example.com?key=value&key2=value">tag</a>',
+        '<a href="http://example.com?key=value&amp;key2=value">tag</a>'
+    ),
+    # & marking character entities in HTML attribute values aren't escaped
+    (
+        '<a href="http://example.com?key=value&amp;key2=value">tag</a>',
+        '<a href="http://example.com?key=value&amp;key2=value">tag</a>'
+    ),
+    # & marking ambiguous character entities in attribute values are escaped
+    # (&curren; is a character entity)
+    (
+        '<a href="http://example.com?key=value&current=value">tag</a>',
+        '<a href="http://example.com?key=value&amp;current=value">tag</a>'
+    ),
+
+])
+def test_serializer(data, expected):
+    # Build a parser, walker, and serializer just like we do in clean()
+    parser = html5lib_shim.BleachHTMLParser(namespaceHTMLElements=False)
+    walker = html5lib_shim.getTreeWalker('etree')
+    serializer = html5lib_shim.BleachHTMLSerializer(
+        quote_attr_values='always',
+        omit_optional_tags=False,
+        escape_lt_in_attrs=True,
+        resolve_entities=False,
+        sanitize=False,
+        alphabetical_attributes=False,
+    )
+
+    # Parse, walk, and then serialize the output
+    dom = parser.parseFragment(data)
+    serialized = serializer.render(walker(dom))
+
+    assert serialized == expected