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.

4 files changed, 74 insertions, 59 deletions

diff --git a/usr.bin/mandoc/html.c b/usr.bin/mandoc/html.c
index b7a66ebbbfb..23f52e10cb0 100644
--- a/usr.bin/mandoc/html.c
+++ b/usr.bin/mandoc/html.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: html.c,v 1.121 2019/01/11 12:44:10 schwarze Exp $ */
+/*	$OpenBSD: html.c,v 1.122 2019/01/18 14:36:16 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2011-2015, 2017-2019 Ingo Schwarze <schwarze@openbsd.org>
@@ -269,11 +269,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;
 	}
 }
 
@@ -577,6 +585,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;
@@ -709,33 +719,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
@@ -822,12 +831,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);
 	}
 }
 
@@ -839,14 +847,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);
 	}
 }
 
diff --git a/usr.bin/mandoc/html.h b/usr.bin/mandoc/html.h
index cbb19a148c7..7f23401ad97 100644
--- a/usr.bin/mandoc/html.h
+++ b/usr.bin/mandoc/html.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: html.h,v 1.62 2019/01/07 06:51:37 schwarze Exp $ */
+/*	$OpenBSD: html.h,v 1.63 2019/01/18 14:36:16 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2017, 2018, 2019 Ingo Schwarze <schwarze@openbsd.org>
@@ -79,6 +79,8 @@ enum	htmlfont {
 
 struct	tag {
 	struct tag	 *next;
+	int		  refcnt;
+	int		  closed;
 	enum htmltag	  tag;
 };
 
diff --git a/usr.bin/mandoc/man_html.c b/usr.bin/mandoc/man_html.c
index 278b1ff9494..bfe9ca34319 100644
--- a/usr.bin/mandoc/man_html.c
+++ b/usr.bin/mandoc/man_html.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: man_html.c,v 1.122 2019/01/11 16:35:39 schwarze Exp $ */
+/*	$OpenBSD: man_html.c,v 1.123 2019/01/18 14:36:16 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2012, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2013-2015, 2017-2019 Ingo Schwarze <schwarze@openbsd.org>
@@ -166,10 +166,6 @@ print_man_node(MAN_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;
-
 	switch (n->type) {
 	case ROFFT_TEXT:
 		if (*n->string == '\0') {
@@ -181,9 +177,13 @@ print_man_node(MAN_ARGS)
 			print_endline(h);
 		else if (n->flags & NODE_DELIMC)
 			h->flags |= HTML_NOSPACE;
+		t = h->tag;
+		t->refcnt++;
 		print_text(h, n->string);
 		break;
 	case ROFFT_EQN:
+		t = h->tag;
+		t->refcnt++;
 		print_eqn(h, n->eqn);
 		break;
 	case ROFFT_TBL:
@@ -209,12 +209,13 @@ print_man_node(MAN_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;
-		}
+		t = h->tag;
+		t->refcnt++;
 		if (n->tok < ROFF_MAX) {
 			roff_html_pre(h, n);
+			t->refcnt--;
 			print_stagq(h, t);
 			return;
 		}
@@ -229,6 +230,7 @@ print_man_node(MAN_ARGS)
 		print_man_nodelist(man, n->child, h);
 
 	/* This will automatically close out any font scope. */
+	t->refcnt--;
 	print_stagq(h, t);
 
 	if (n->flags & NODE_NOFILL && n->tok != MAN_YS &&
diff --git a/usr.bin/mandoc/mdoc_html.c b/usr.bin/mandoc/mdoc_html.c
index 92b9ba3689e..ad42d59ecf9 100644
--- a/usr.bin/mandoc/mdoc_html.c
+++ b/usr.bin/mandoc/mdoc_html.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mdoc_html.c,v 1.201 2019/01/11 16:35:39 schwarze Exp $ */
+/*	$OpenBSD: mdoc_html.c,v 1.202 2019/01/18 14:36:16 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2014-2019 Ingo Schwarze <schwarze@openbsd.org>
@@ -352,13 +352,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);
 
@@ -377,6 +376,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:
@@ -393,13 +394,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;
 		}
@@ -419,6 +421,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) {