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, 11 insertions, 8 deletions

diff --git a/mdoc_html.c b/mdoc_html.c
index c7f452ec..ea750cdb 100644
--- a/mdoc_html.c
+++ b/mdoc_html.c
@@ -1,4 +1,4 @@
-/*	$Id: mdoc_html.c,v 1.326 2019/01/11 16:36:19 schwarze Exp $ */
+/*	$Id: mdoc_html.c,v 1.327 2019/01/18 14:36:21 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2014-2019 Ingo Schwarze <schwarze@openbsd.org>
@@ -354,13 +354,12 @@ print_mdoc_node(MDOC_ARGS)
 	html_fillmode(h, n->flags & NODE_NOFILL ? ROFF_nf : ROFF_fi);
 
 	child = 1;
-	t = h->tag;
-	if (t->tag == TAG_P || t->tag == TAG_PRE)
-		t = t->next;
-
 	n->flags &= ~NODE_ENDED;
 	switch (n->type) {
 	case ROFFT_TEXT:
+		t = h->tag;
+		t->refcnt++;
+
 		/* No tables in this mode... */
 		assert(NULL == h->tblt);
 
@@ -379,6 +378,8 @@ print_mdoc_node(MDOC_ARGS)
 			h->flags |= HTML_NOSPACE;
 		break;
 	case ROFFT_EQN:
+		t = h->tag;
+		t->refcnt++;
 		print_eqn(h, n->eqn);
 		break;
 	case ROFFT_TBL:
@@ -395,13 +396,14 @@ print_mdoc_node(MDOC_ARGS)
 		 * the "meta" table state.  This will be reopened on the
 		 * next table element.
 		 */
-		if (h->tblt != NULL) {
+		if (h->tblt != NULL)
 			print_tblclose(h);
-			t = h->tag;
-		}
 		assert(h->tblt == NULL);
+		t = h->tag;
+		t->refcnt++;
 		if (n->tok < ROFF_MAX) {
 			roff_html_pre(h, n);
+			t->refcnt--;
 			print_stagq(h, t);
 			return;
 		}
@@ -421,6 +423,7 @@ print_mdoc_node(MDOC_ARGS)
 	if (child && n->child != NULL)
 		print_mdoc_nodelist(meta, n->child, h);
 
+	t->refcnt--;
 	print_stagq(h, t);
 
 	switch (n->type) {