The .UR and .MT blocks in man(7) are represented by <a> elements

which establish phrasing context, but they can contain paragraph
breaks (which is relevant for terminal formatting, so we can't just
change the structure of the syntax tree), which are respresented
by <p> elements and cannot occur inside <a>.

Fix this by prematurely closing the <a> element in the HTML formatter.
This menas that the clickable text in HTML output is shorter than
what is represented as the link text in terminal output, but in
HTML, it is frankly impossible to have the clickable area of a
hyperlink extend across a paragraph break.  The difference in
presentation is not a major problem, and besides, paragraph breaks
inside .UR are rather poor style in the first place.

The implementation is quite tricky.  Naively closing out the <a>
prematurely would result in accessing a stale pointer when later
reaching the physical end of the .UR block.  So this commit separates
visual and structural closing of "struct tag" stack items.  Visual
closing means that the HTML element is closed but the "struct tag"
remains on the stack, to avoid later access to a stale pointer and
to avoid closing the same HTML element a second time later.

This also needs reference counting of pointers to "struct tag" stack
items because often more than one child holds a pointer to the same
parent item, and only the outermost child can safely do the physical
closing.

In the whole corpus of nearly half a million manual pages on
man.openbsd.org, this problem occurs in exactly one page: the
groff(1) version 1.20.1 manual contained in DragonFly-3.8.2, which
contains a formatting error triggering the bug.

1 files changed, 49 insertions, 41 deletions

diff --git a/html.c b/html.c
index 4668f6cf..e4955fd7 100644
--- a/html.c
+++ b/html.c
@@ -271,11 +271,19 @@ html_close_paragraph(struct html *h)
 {
 	struct tag	*t;
 
-	for (t = h->tag; t != NULL; t = t->next) {
-		if (t->tag == TAG_P || t->tag == TAG_PRE) {
+	for (t = h->tag; t != NULL && t->closed == 0; t = t->next) {
+		switch(t->tag) {
+		case TAG_P:
+		case TAG_PRE:
 			print_tagq(h, t);
 			break;
+		case TAG_A:
+			print_tagq(h, t);
+			continue;
+		default:
+			continue;
 		}
+		break;
 	}
 }
 
@@ -579,6 +587,8 @@ print_otag(struct html *h, enum htmltag tag, const char *fmt, ...)
 		t = mandoc_malloc(sizeof(struct tag));
 		t->tag = tag;
 		t->next = h->tag;
+		t->refcnt = 0;
+		t->closed = 0;
 		h->tag = t;
 	} else
 		t = NULL;
@@ -711,33 +721,32 @@ print_ctag(struct html *h, struct tag *tag)
 {
 	int	 tflags;
 
-	/*
-	 * Remember to close out and nullify the current
-	 * meta-font and table, if applicable.
-	 */
-	if (tag == h->metaf)
-		h->metaf = NULL;
-	if (tag == h->tblt)
-		h->tblt = NULL;
-
-	tflags = htmltags[tag->tag].flags;
-
-	if (tflags & HTML_INDENT)
-		h->indent--;
-	if (tflags & HTML_NOINDENT)
-		h->noindent--;
-	if (tflags & HTML_NLEND)
-		print_endline(h);
-	print_indent(h);
-	print_byte(h, '<');
-	print_byte(h, '/');
-	print_word(h, htmltags[tag->tag].name);
-	print_byte(h, '>');
-	if (tflags & HTML_NLAFTER)
-		print_endline(h);
-
-	h->tag = tag->next;
-	free(tag);
+	if (tag->closed == 0) {
+		tag->closed = 1;
+		if (tag == h->metaf)
+			h->metaf = NULL;
+		if (tag == h->tblt)
+			h->tblt = NULL;
+
+		tflags = htmltags[tag->tag].flags;
+		if (tflags & HTML_INDENT)
+			h->indent--;
+		if (tflags & HTML_NOINDENT)
+			h->noindent--;
+		if (tflags & HTML_NLEND)
+			print_endline(h);
+		print_indent(h);
+		print_byte(h, '<');
+		print_byte(h, '/');
+		print_word(h, htmltags[tag->tag].name);
+		print_byte(h, '>');
+		if (tflags & HTML_NLAFTER)
+			print_endline(h);
+	}
+	if (tag->refcnt == 0) {
+		h->tag = tag->next;
+		free(tag);
+	}
 }
 
 void
@@ -824,12 +833,11 @@ print_text(struct html *h, const char *word)
 void
 print_tagq(struct html *h, const struct tag *until)
 {
-	struct tag	*tag;
+	struct tag	*this, *next;
 
-	while ((tag = h->tag) != NULL) {
-		print_ctag(h, tag);
-		if (tag == until)
-			return;
+	for (this = h->tag; this != NULL; this = next) {
+		next = this == until ? NULL : this->next;
+		print_ctag(h, this);
 	}
 }
 
@@ -841,14 +849,14 @@ print_tagq(struct html *h, const struct tag *until)
 void
 print_stagq(struct html *h, const struct tag *suntil)
 {
-	struct tag	*tag;
+	struct tag	*this, *next;
 
-	while ((tag = h->tag) != NULL) {
-		if (tag == suntil ||
-		    (tag->next == suntil &&
-		     (tag->tag == TAG_P || tag->tag == TAG_PRE)))
-			return;
-		print_ctag(h, tag);
+	for (this = h->tag; this != NULL; this = next) {
+		next = this->next;
+		if (this == suntil || (next == suntil &&
+		    (this->tag == TAG_P || this->tag == TAG_PRE)))
+			break;
+		print_ctag(h, this);
 	}
 }