Reword comments about replacing handlers within a callback

Author: gpshead

Lib/test/test_pyexpat.py

@@ -836,9 +836,10 @@ def handler(text):
 
 
 class DefaultHandlerExceptionTest(unittest.TestCase):
-    # Expat delivers a token that needs encoding conversion to the default
-    # handler in pieces of at most 1024 bytes, re-reading the handler in
-    # between.  Removing the handler from within the callback must not crash.
+    # In a document that is not UTF-8, Expat passes a long piece of text such
+    # as this comment to the default handler in chunks of at most 1024 bytes,
+    # re-reading the handler between chunks.  Clearing the handler from within
+    # the callback, by raising or by setting it to None, must not crash.
     xml = ("<?xml version='1.0' encoding='iso8859'?><s><!--%s--></s>"
            % ('a' * 2000)).encode('iso8859')
 
@@ -872,8 +873,10 @@ def handler(text):
         self.assertTrue(calls[-1].startswith('<!--'), calls)
 
     def test_CharacterDataHandler_with_buffer_text(self):
-        # An exception in the CharacterDataHandler while the default
-        # handler trampoline flushes the text buffer.
+        # With buffer_text, buffered character data is passed to the
+        # CharacterDataHandler just before the default handler is called;
+        # an exception there clears the handlers while Expat is still
+        # passing the comment to the default handler.
         def handler(text):
             raise self.SpecificException
 

Modules/pyexpat.c

@@ -293,12 +293,13 @@ error_external_entity_ref_handler(XML_Parser parser,
 
 /* Dummy character data handler used when an error (exception) has
    been detected, and the actual parsing can be terminated early.
-   This is needed since character data handler can't be safely removed
-   from within the character data handler, but can be replaced.  It is
-   used from the character data handler trampoline, and must be
-   used right after `flag_error()` is called.  It is also installed as
-   the default handler (same signature) by clear_handlers(), since Expat
-   can't cope with that one being removed from within a callback either. */
+   This is needed since the character data and default handlers can't be
+   safely removed from within a callback, only replaced: Expat may call
+   them several times for one piece of text (a long comment in a document
+   that is not UTF-8, for example).  The default handler has the same
+   signature, so this function serves for both.  clear_handlers() installs
+   it after `flag_error()`, and the handler setter installs it when a
+   handler is set to None from within a callback. */
 static void
 noop_character_data_handler(void *userData, const XML_Char *data, int len)
 {
@@ -1841,8 +1842,9 @@ xmlparse_handler_setter(PyObject *op, PyObject *v, void *closure)
            otherwise would, but that's really an odd case.  A more
            elaborate system of handlers and state could remove the
            C handler more effectively.  The same applies to the
-           default handler, which Expat calls repeatedly for a single
-           token that needs encoding conversion. */
+           default handler, which Expat can call several times for one
+           piece of text, such as a long comment in a document that is
+           not UTF-8. */
         if (self->in_callback
             && (handlernum == CharacterData
                 || handlernum == Default
@@ -2796,9 +2798,10 @@ clear_handlers(xmlparseobject *self, int initial)
             xmlhandler c_handler = NULL;
             if (i == Default || i == DefaultHandlerExpand) {
                 /* Like the character data handler, the default handler
-                   can't be safely removed from within a callback: Expat's
-                   reportDefault() re-reads it between the pieces of a
-                   converted token.  Replace it with a no-op instead. */
+                   can't be removed from within a callback: for a document
+                   that is not UTF-8, Expat passes a long piece of text to
+                   it in several calls and re-reads the handler before each
+                   one (reportDefault() in xmlparse.c). */
                 c_handler = (xmlhandler)noop_character_data_handler;
             }
             CALL_XML_HANDLER_SETTER(&handler_info[i], self->itself, c_handler);