On Thursday 16 April 2015 13:59:28 Christian Schoenebeck wrote:
> On Thursday 16 April 2015 10:32:32 you wrote:
> > > There you go; you find the updated patch attached. It now requires
> > > HTML_PARSE_RECOVER option to be set for recovering from stand-alone
> > > less-than characters.
> > 
> > That sounds fine *except* it doesn't raise an error.
> > The parser knows it's a broken construct that must be pointed out.
> 
> Ok, I see what I can do about that. ;)

Even two patches this time. The 1st patch (libxml-less-than-char_v3.patch) 
just addresses that last minor issue you came up with regarding missing error 
messages. However you probably might skip that patch and rather look at the 
second patch instead.

> >  It sounds a bit weird to handle that error case as one of the main
> >  content
> > 
> > cases, I would still be tempted to go into htmlParseStartTag, get the
> > error reported, but push corrective data instead in recover mode.
> 
> My initial thought solution was to enter htmlParseElement() like before,
> and in case htmlParseElement() encounters an error, it would handle the
> chunk as text instead (if recover option is on). That would probably come
> to the closest what most browsers seem to do. But the problem: that would

The 2nd patch (libxml-invalid-tag-as-text.patch) uses that more general way to 
resolve this overall issue. That is, instead of looking at the content and 
trying to guess ahead whether a less than character will yield in a valid tag, 
this 2nd patch rather uses the regular element parse code, and if it fails to 
parse the tag start it returns a special return value which will cause the 
next input to be consumed as text instead. Most notably this solution has the 
advantage, that many more misfit cases will be consumed as text instead (if 
recovery option is on). For example this 2nd patch also allows to consume 
this:

        a << b

The 1st patch would still have failed in this case.

Please review this 2nd patch carefully though. Because that patch is rewinding 
the parser input, and since I am not very familiar with the libxm2 internals, 
I am not sure whether my rewinding code is a) safe and b) if it does actually 
work with all kinds of input stream types supported by the libxml2 API.

Best regards,
Christian Schoenebeck
diff -r -u libxml2-2.9.1+dfsg1.orig/HTMLparser.c libxml2-2.9.1+dfsg1.SIMPLE_PATCH/HTMLparser.c
--- libxml2-2.9.1+dfsg1.orig/HTMLparser.c	2015-04-14 13:05:01.000000000 +0200
+++ libxml2-2.9.1+dfsg1.SIMPLE_PATCH/HTMLparser.c	2015-04-25 14:29:13.472858931 +0200
@@ -2948,8 +2948,10 @@
 
 
 /**
- * htmlParseCharData:
+ * htmlParseCharDataInternal:
  * @ctxt:  an HTML parser context
+ * @prep:  optional character to be prepended to text, 0 if no character
+ *         shall be prepended
  *
  * parse a CharData section.
  * if we are within a CDATA section ']]>' marks an end of section.
@@ -2958,12 +2960,15 @@
  */
 
 static void
-htmlParseCharData(htmlParserCtxtPtr ctxt) {
-    xmlChar buf[HTML_PARSER_BIG_BUFFER_SIZE + 5];
+htmlParseCharDataInternal(htmlParserCtxtPtr ctxt, char prep) {
+    xmlChar buf[HTML_PARSER_BIG_BUFFER_SIZE + 6];
     int nbchar = 0;
     int cur, l;
     int chunk = 0;
 
+    if (prep)
+	buf[nbchar++] = prep;
+
     SHRINK;
     cur = CUR_CHAR(l);
     while (((cur != '<') || (ctxt->token == '<')) &&
@@ -3043,6 +3048,21 @@
 }
 
 /**
+ * htmlParseCharData:
+ * @ctxt:  an HTML parser context
+ *
+ * parse a CharData section.
+ * if we are within a CDATA section ']]>' marks an end of section.
+ *
+ * [14] CharData ::= [^<&]* - ([^<&]* ']]>' [^<&]*)
+ */
+
+static void
+htmlParseCharData(htmlParserCtxtPtr ctxt) {
+    htmlParseCharDataInternal(ctxt, 0);
+}
+
+/**
  * htmlParseExternalID:
  * @ctxt:  an HTML parser context
  * @publicID:  a xmlChar** receiving PubidLiteral
@@ -4157,14 +4177,27 @@
 	    }
 
 	    /*
-	     * Third case :  a sub-element.
+	     * Third case : (unescaped) stand-alone less-than character.
+	     *              Only if HTML_PARSE_RECOVER option is set.
+	     */
+	    else if (ctxt->recovery && (CUR == '<') &&
+		     (IS_BLANK_CH(NXT(1)) || (NXT(1) == '='))) {
+		htmlParseErr(ctxt, XML_ERR_NAME_REQUIRED,
+			 "htmlParseContent: invalid element name\n",
+			 NULL, NULL);
+		NEXT;
+		htmlParseCharDataInternal(ctxt, '<');
+	    }
+
+	    /*
+	     * Fourth case :  a sub-element.
 	     */
 	    else if (CUR == '<') {
 		htmlParseElement(ctxt);
 	    }
 
 	    /*
-	     * Fourth case : a reference. If if has not been resolved,
+	     * Fifth case : a reference. If if has not been resolved,
 	     *    parsing returns it's Name, create the node
 	     */
 	    else if (CUR == '&') {
@@ -4172,7 +4205,7 @@
 	    }
 
 	    /*
-	     * Fifth case : end of the resource
+	     * Sixth case : end of the resource
 	     */
 	    else if (CUR == 0) {
 		htmlAutoCloseOnEnd(ctxt);
@@ -4567,7 +4600,20 @@
 	    }
 
 	    /*
-	     * Third case :  a sub-element.
+	     * Third case : (unescaped) stand-alone less-than character.
+	     *              Only if HTML_PARSE_RECOVER option is set.
+	     */
+	    else if (ctxt->recovery && (CUR == '<') &&
+		     (IS_BLANK_CH(NXT(1)) || (NXT(1) == '='))) {
+		htmlParseErr(ctxt, XML_ERR_NAME_REQUIRED,
+			 "htmlParseContent: invalid element name\n",
+			 NULL, NULL);
+		NEXT;
+		htmlParseCharDataInternal(ctxt, '<');
+	    }
+
+	    /*
+	     * Fourth case :  a sub-element.
 	     */
 	    else if (CUR == '<') {
 		htmlParseElementInternal(ctxt);
@@ -4578,7 +4624,7 @@
 	    }
 
 	    /*
-	     * Fourth case : a reference. If if has not been resolved,
+	     * Fifth case : a reference. If if has not been resolved,
 	     *    parsing returns it's Name, create the node
 	     */
 	    else if (CUR == '&') {
@@ -4586,7 +4632,7 @@
 	    }
 
 	    /*
-	     * Fifth case : end of the resource
+	     * Sixth case : end of the resource
 	     */
 	    else if (CUR == 0) {
 		htmlAutoCloseOnEnd(ctxt);
diff -r -u libxml2-2.9.1+dfsg1.orig/HTMLparser.c libxml2-2.9.1+dfsg1/HTMLparser.c
--- libxml2-2.9.1+dfsg1.orig/HTMLparser.c	2015-04-14 13:05:01.000000000 +0200
+++ libxml2-2.9.1+dfsg1/HTMLparser.c	2015-04-26 03:18:31.088399169 +0200
@@ -59,6 +59,7 @@
 xmlChar * htmlDecodeEntities(htmlParserCtxtPtr ctxt, int len,
 			     xmlChar end, xmlChar  end2, xmlChar end3);
 static void htmlParseComment(htmlParserCtxtPtr ctxt);
+static int htmlParseElementRecursive(htmlParserCtxtPtr ctxt);
 
 /************************************************************************
  *									*
@@ -2946,10 +2947,11 @@
     }
 }
 
-
 /**
- * htmlParseCharData:
+ * htmlParseCharDataInternal:
  * @ctxt:  an HTML parser context
+ * @prep:  optional character to be prepended to text, 0 if no character
+ *         shall be prepended
  *
  * parse a CharData section.
  * if we are within a CDATA section ']]>' marks an end of section.
@@ -2958,12 +2960,15 @@
  */
 
 static void
-htmlParseCharData(htmlParserCtxtPtr ctxt) {
-    xmlChar buf[HTML_PARSER_BIG_BUFFER_SIZE + 5];
+htmlParseCharDataInternal(htmlParserCtxtPtr ctxt, char prep) {
+    xmlChar buf[HTML_PARSER_BIG_BUFFER_SIZE + 6];
     int nbchar = 0;
     int cur, l;
     int chunk = 0;
 
+    if (prep)
+	buf[nbchar++] = prep;
+
     SHRINK;
     cur = CUR_CHAR(l);
     while (((cur != '<') || (ctxt->token == '<')) &&
@@ -3043,6 +3048,21 @@
 }
 
 /**
+ * htmlParseCharData:
+ * @ctxt:  an HTML parser context
+ *
+ * parse a CharData section.
+ * if we are within a CDATA section ']]>' marks an end of section.
+ *
+ * [14] CharData ::= [^<&]* - ([^<&]* ']]>' [^<&]*)
+ */
+
+static void
+htmlParseCharData(htmlParserCtxtPtr ctxt) {
+    htmlParseCharDataInternal(ctxt, 0);
+}
+
+/**
  * htmlParseExternalID:
  * @ctxt:  an HTML parser context
  * @publicID:  a xmlChar** receiving PubidLiteral
@@ -4051,7 +4071,7 @@
  * htmlParseContent:
  * @ctxt:  an HTML parser context
  *
- * Parse a content: comment, sub-element, reference or text.
+ * Parse a content recursively: comment, sub-element, reference or text.
  * Kept for compatibility with old code
  */
 
@@ -4060,6 +4080,7 @@
     xmlChar *currentNode;
     int depth;
     const xmlChar *name;
+    int res;
 
     currentNode = xmlStrdup(ctxt->name);
     depth = ctxt->nameNr;
@@ -4157,10 +4178,19 @@
 	    }
 
 	    /*
-	     * Third case :  a sub-element.
+	     * Third case :  a sub-element (recursively).
 	     */
 	    else if (CUR == '<') {
-		htmlParseElement(ctxt);
+		res = htmlParseElementRecursive(ctxt);
+
+		/*
+		 * If it was an invalid element tag and parser position was
+		 * rewinded to tag start, then consume next input as text.
+		 */
+		if (res == -2) {
+		    NEXT;
+		    htmlParseCharDataInternal(ctxt, '<');
+		}
 	    }
 
 	    /*
@@ -4201,19 +4231,24 @@
 }
 
 /**
- * htmlParseElement:
+ * htmlParseElementRecursive:
  * @ctxt:  an HTML parser context
  *
  * parse an HTML element, this is highly recursive
  * this is kept for compatibility with previous code versions
+ * regarding htmlParseElement()
  *
  * [39] element ::= EmptyElemTag | STag content ETag
  *
  * [41] Attribute ::= Name Eq AttValue
+ * 
+ * Returns 0: success,
+ *        -1: error - consumed tag input (that is dumped invalid tag),
+ *        -2: error - rewinded input to tag start - handle next char as text
  */
 
-void
-htmlParseElement(htmlParserCtxtPtr ctxt) {
+static int
+htmlParseElementRecursive(htmlParserCtxtPtr ctxt) {
     const xmlChar *name;
     xmlChar *currentNode = NULL;
     const htmlElemDesc * info;
@@ -4221,15 +4256,16 @@
     int failed;
     int depth;
     const xmlChar *oldptr;
+    xmlParserInputPtr startPos = NULL;
 
     if ((ctxt == NULL) || (ctxt->input == NULL)) {
 	htmlParseErr(ctxt, XML_ERR_INTERNAL_ERROR,
-		     "htmlParseElement: context error\n", NULL, NULL);
-	return;
+		     "htmlParseElementRecursive: context error\n", NULL, NULL);
+	return -1;
     }
 
     if (ctxt->instate == XML_PARSER_EOF)
-        return;
+        return -1;
 
     /* Capture start position */
     if (ctxt->record_info) {
@@ -4237,13 +4273,18 @@
                           (CUR_PTR - ctxt->input->base);
 	node_info.begin_line = ctxt->input->line;
     }
+    startPos = xmlNewInputStream(ctxt);
+    if (startPos != NULL)
+       *startPos = *ctxt->input;
+    else
+       return -1;
 
     failed = htmlParseStartTag(ctxt);
     name = ctxt->name;
     if ((failed == -1) || (name == NULL)) {
 	if (CUR == '>')
 	    NEXT;
-        return;
+        goto errorDoRewind;
     }
 
     /*
@@ -4263,7 +4304,7 @@
 	if ((ctxt->sax != NULL) && (ctxt->sax->endElement != NULL))
 	    ctxt->sax->endElement(ctxt->userData, name);
 	htmlnamePop(ctxt);
-	return;
+	goto errorDoConsume;
     }
 
     if (CUR == '>') {
@@ -4290,7 +4331,7 @@
 	   node_info.node = ctxt->node;
 	   xmlParserAddNodeInfo(ctxt, &node_info);
 	}
-	return;
+	goto errorDoConsume;
     }
 
     /*
@@ -4300,7 +4341,7 @@
 	if ((ctxt->sax != NULL) && (ctxt->sax->endElement != NULL))
 	    ctxt->sax->endElement(ctxt->userData, name);
 	htmlnamePop(ctxt);
-	return;
+	goto errorDoConsume;
     }
 
     /*
@@ -4331,6 +4372,52 @@
 
     if (currentNode != NULL)
 	xmlFree(currentNode);
+
+    /* Success */
+    failed = 0;
+    goto done;
+
+    /* Error occurred, leave input consumed. */
+errorDoConsume:
+    failed = -1;
+    goto done;
+
+    /* 
+     * Error occurred, rewind parser to tag start position.
+     * (but only if HTML_PARSE_RECOVER is set, otherwise leave input consumed)
+     */
+errorDoRewind:
+    if (ctxt->recovery) {
+	failed = -2;
+	if (startPos)
+	    *ctxt->input = *startPos;
+    } else {
+	failed = -1;
+    }
+    goto done;
+
+done:
+    if (startPos != NULL)
+	xmlFree(startPos);
+
+    return failed;
+}
+
+/**
+ * htmlParseElement:
+ * @ctxt:  an HTML parser context
+ *
+ * parse an HTML element, this is highly recursive
+ * this is kept for compatibility with previous code versions
+ *
+ * [39] element ::= EmptyElemTag | STag content ETag
+ *
+ * [41] Attribute ::= Name Eq AttValue
+ */
+
+void
+htmlParseElement(htmlParserCtxtPtr ctxt) {
+    htmlParseElementRecursive(ctxt);
 }
 
 static void
@@ -4352,7 +4439,7 @@
 }
 
 /**
- * htmlParseElementInternal:
+ * htmlParseElementNonRecursive:
  * @ctxt:  an HTML parser context
  *
  * parse an HTML element, new version, non recursive
@@ -4360,23 +4447,28 @@
  * [39] element ::= EmptyElemTag | STag content ETag
  *
  * [41] Attribute ::= Name Eq AttValue
+ * 
+ * Returns 0: success,
+ *        -1: error - consumed tag input (that is dumped invalid tag),
+ *        -2: error - rewinded input to tag start - handle next char as text
  */
 
-static void
-htmlParseElementInternal(htmlParserCtxtPtr ctxt) {
+static int
+htmlParseElementNonRecursive(htmlParserCtxtPtr ctxt) {
     const xmlChar *name;
     const htmlElemDesc * info;
     htmlParserNodeInfo node_info = { 0, };
     int failed;
+    xmlParserInputPtr startPos = NULL;
 
     if ((ctxt == NULL) || (ctxt->input == NULL)) {
 	htmlParseErr(ctxt, XML_ERR_INTERNAL_ERROR,
-		     "htmlParseElementInternal: context error\n", NULL, NULL);
-	return;
+		     "htmlParseElementNonRecursive: context error\n", NULL, NULL);
+	return -1;
     }
 
     if (ctxt->instate == XML_PARSER_EOF)
-        return;
+        return -1;
 
     /* Capture start position */
     if (ctxt->record_info) {
@@ -4384,13 +4476,18 @@
                           (CUR_PTR - ctxt->input->base);
 	node_info.begin_line = ctxt->input->line;
     }
+    startPos = xmlNewInputStream(ctxt);
+    if (startPos != NULL)
+	*startPos = *ctxt->input;
+    else
+	return -1;
 
     failed = htmlParseStartTag(ctxt);
     name = ctxt->name;
     if ((failed == -1) || (name == NULL)) {
 	if (CUR == '>')
 	    NEXT;
-        return;
+	goto errorDoRewind;
     }
 
     /*
@@ -4410,7 +4507,7 @@
 	if ((ctxt->sax != NULL) && (ctxt->sax->endElement != NULL))
 	    ctxt->sax->endElement(ctxt->userData, name);
 	htmlnamePop(ctxt);
-	return;
+	goto errorDoConsume;
     }
 
     if (CUR == '>') {
@@ -4430,7 +4527,7 @@
         if (ctxt->record_info)
             htmlNodeInfoPush(ctxt, &node_info);
         htmlParserFinishElementParsing(ctxt);
-	return;
+	goto errorDoConsume;
     }
 
     /*
@@ -4440,26 +4537,56 @@
 	if ((ctxt->sax != NULL) && (ctxt->sax->endElement != NULL))
 	    ctxt->sax->endElement(ctxt->userData, name);
 	htmlnamePop(ctxt);
-	return;
+	goto errorDoConsume;
     }
 
     if (ctxt->record_info)
         htmlNodeInfoPush(ctxt, &node_info);
+
+    /* Success */
+    failed = 0;
+    goto done;
+
+    /* Error occurred, leave input consumed. */
+errorDoConsume:
+    failed = -1;
+    goto done;
+
+    /* 
+     * Error occurred, rewind parser to tag start position.
+     * (but only if HTML_PARSE_RECOVER is set, otherwise leave input consumed)
+     */
+errorDoRewind:
+    if (ctxt->recovery) {
+	failed = -2;
+	if (startPos)
+	    *ctxt->input = *startPos;
+    } else {
+	failed = -1;
+    }
+    goto done;
+
+done:
+    if (startPos != NULL)
+	xmlFree(startPos);
+
+    return failed;
 }
 
 /**
- * htmlParseContentInternal:
+ * htmlParseContentNonRecursive:
  * @ctxt:  an HTML parser context
  *
  * Parse a content: comment, sub-element, reference or text.
- * New version for non recursive htmlParseElementInternal
+ * New version for non recursive htmlParseElementNonRecursive
  */
 
 static void
-htmlParseContentInternal(htmlParserCtxtPtr ctxt) {
+htmlParseContentNonRecursive(htmlParserCtxtPtr ctxt) {
     xmlChar *currentNode;
     int depth;
     const xmlChar *name;
+    int res;
 
     currentNode = xmlStrdup(ctxt->name);
     depth = ctxt->nameNr;
@@ -4570,11 +4697,20 @@
 	     * Third case :  a sub-element.
 	     */
 	    else if (CUR == '<') {
-		htmlParseElementInternal(ctxt);
+		res = htmlParseElementNonRecursive(ctxt);
 		if (currentNode != NULL) xmlFree(currentNode);
 
 		currentNode = xmlStrdup(ctxt->name);
 		depth = ctxt->nameNr;
+
+		/*
+		 * If it was an invalid element tag and parser position was
+		 * rewinded to tag start, then consume next input as text.
+		 */
+		if (res == -2) {
+		    NEXT;
+		    htmlParseCharDataInternal(ctxt, '<');
+		}
 	    }
 
 	    /*
@@ -4618,14 +4754,14 @@
  * htmlParseContent:
  * @ctxt:  an HTML parser context
  *
- * Parse a content: comment, sub-element, reference or text.
+ * Parse a content non-recursive: comment, sub-element, reference or text.
  * This is the entry point when called from parser.c
  */
 
 void
 __htmlParseContent(void *ctxt) {
     if (ctxt != NULL)
-	htmlParseContentInternal((htmlParserCtxtPtr) ctxt);
+	htmlParseContentNonRecursive((htmlParserCtxtPtr) ctxt);
 }
 
 /**
@@ -4732,7 +4868,7 @@
     /*
      * Time to start parsing the tree itself
      */
-    htmlParseContentInternal(ctxt);
+    htmlParseContentNonRecursive(ctxt);
 
     /*
      * autoclose
_______________________________________________
xml mailing list, project page  http://xmlsoft.org/
xml@gnome.org
https://mail.gnome.org/mailman/listinfo/xml

Reply via email to