From 7056ae08bfa5cafeec9c454cb40aefa7553af6df Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Thu, 16 Jul 2020 12:53:24 -0400

Subject: [PATCH 1/4] Fix: libcrmcommon: Set out->priv to NULL in free_priv.

init won't do anything if priv is not NULL, so when the private data is

freed, also set it to NULL.  This prevents segfaults when reset is

called.

---

 lib/common/output_html.c | 1 +

 lib/common/output_log.c  | 1 +

 lib/common/output_text.c | 1 +

 lib/common/output_xml.c  | 1 +

 tools/crm_mon_curses.c   | 1 +

 5 files changed, 5 insertions(+)

diff --git a/lib/common/output_html.c b/lib/common/output_html.c

index c8f0088..fc06641 100644

--- a/lib/common/output_html.c

+++ b/lib/common/output_html.c

@@ -72,6 +72,7 @@ html_free_priv(pcmk__output_t *out) {

     g_queue_free(priv->parent_q);

     g_slist_free(priv->errors);

     free(priv);

+    out->priv = NULL;

 }

 static bool

diff --git a/lib/common/output_log.c b/lib/common/output_log.c

index 5b45ce4..0208046 100644

--- a/lib/common/output_log.c

+++ b/lib/common/output_log.c

@@ -44,6 +44,7 @@ log_free_priv(pcmk__output_t *out) {

     g_queue_free(priv->prefixes);

     free(priv);

+    out->priv = NULL;

 }

 static bool

diff --git a/lib/common/output_text.c b/lib/common/output_text.c

index 54c409a..8f15849 100644

--- a/lib/common/output_text.c

+++ b/lib/common/output_text.c

@@ -43,6 +43,7 @@ text_free_priv(pcmk__output_t *out) {

     g_queue_free(priv->parent_q);

     free(priv);

+    out->priv = NULL;

 }

 static bool

diff --git a/lib/common/output_xml.c b/lib/common/output_xml.c

index 8565bfe..858da3f 100644

--- a/lib/common/output_xml.c

+++ b/lib/common/output_xml.c

@@ -54,6 +54,7 @@ xml_free_priv(pcmk__output_t *out) {

     g_queue_free(priv->parent_q);

     g_slist_free(priv->errors);

     free(priv);

+    out->priv = NULL;

 }

 static bool

diff --git a/tools/crm_mon_curses.c b/tools/crm_mon_curses.c

index d93b847..e9cc023 100644

--- a/tools/crm_mon_curses.c

+++ b/tools/crm_mon_curses.c

@@ -46,6 +46,7 @@ curses_free_priv(pcmk__output_t *out) {

     g_queue_free(priv->parent_q);

     free(priv);

+    out->priv = NULL;

 }

 static bool

-- 

1.8.3.1

From 3779152993ca0e88dc407c918882568217f1b630 Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Thu, 16 Jul 2020 13:50:24 -0400

Subject: [PATCH 2/4] Fix: libcrmcommon: Make reset and finish work more

 similarly.

When finish is called for HTML and XML output formats, various extra

nodes and headers are added, errors are added, etc.  None of this stuff

happens on reset.  For the HTML format, this also means things like the

CGI headers and title don't get added when reset is called.  Make these

two functions much more similar.

Regression in 2.0.3.

See: rhbz#1857728

---

 lib/common/output_html.c | 26 ++++++++++++++++----------

 lib/common/output_xml.c  | 30 ++++++++++++++++--------------

 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/lib/common/output_html.c b/lib/common/output_html.c

index fc06641..6127df2 100644

--- a/lib/common/output_html.c

+++ b/lib/common/output_html.c

@@ -113,18 +113,11 @@ add_error_node(gpointer data, gpointer user_data) {

 }

 static void

-html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {

+finish_reset_common(pcmk__output_t *out, crm_exit_t exit_status, bool print) {

     private_data_t *priv = out->priv;

     htmlNodePtr head_node = NULL;

     htmlNodePtr charset_node = NULL;

-    /* If root is NULL, html_init failed and we are being called from pcmk__output_free

-     * in the pcmk__output_new path.

-     */

-    if (priv == NULL || priv->root == NULL) {

-        return;

-    }

-

     if (cgi_output && print) {

         fprintf(out->dest, "Content-Type: text/html\n\n");

     }

@@ -174,6 +167,20 @@ html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy

     if (print) {

         htmlDocDump(out->dest, priv->root->doc);

     }

+}

+

+static void

+html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {

+    private_data_t *priv = out->priv;

+

+    /* If root is NULL, html_init failed and we are being called from pcmk__output_free

+     * in the pcmk__output_new path.

+     */

+    if (priv == NULL || priv->root == NULL) {

+        return;

+    }

+

+    finish_reset_common(out, exit_status, print);

     if (copy_dest != NULL) {

         *copy_dest = copy_xml(priv->root);

@@ -185,8 +192,7 @@ html_reset(pcmk__output_t *out) {

     CRM_ASSERT(out != NULL);

     if (out->priv != NULL) {

-        private_data_t *priv = out->priv;

-        htmlDocDump(out->dest, priv->root->doc);

+        finish_reset_common(out, CRM_EX_OK, true);

     }

     html_free_priv(out);

diff --git a/lib/common/output_xml.c b/lib/common/output_xml.c

index 858da3f..b64a71d 100644

--- a/lib/common/output_xml.c

+++ b/lib/common/output_xml.c

@@ -106,17 +106,10 @@ add_error_node(gpointer data, gpointer user_data) {

 }

 static void

-xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {

+finish_reset_common(pcmk__output_t *out, crm_exit_t exit_status, bool print) {

     xmlNodePtr node;

     private_data_t *priv = out->priv;

-    /* If root is NULL, xml_init failed and we are being called from pcmk__output_free

-     * in the pcmk__output_new path.

-     */

-    if (priv == NULL || priv->root == NULL) {

-        return;

-    }

-

     if (legacy_xml) {

         GSList *node = priv->errors;

@@ -148,6 +141,20 @@ xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_

         fprintf(out->dest, "%s", buf);

         free(buf);

     }

+}

+

+static void

+xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_dest) {

+    private_data_t *priv = out->priv;

+

+    /* If root is NULL, xml_init failed and we are being called from pcmk__output_free

+     * in the pcmk__output_new path.

+     */

+    if (priv == NULL || priv->root == NULL) {

+        return;

+    }

+

+    finish_reset_common(out, exit_status, print);

     if (copy_dest != NULL) {

         *copy_dest = copy_xml(priv->root);

@@ -156,15 +163,10 @@ xml_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy_

 static void

 xml_reset(pcmk__output_t *out) {

-    char *buf = NULL;

-

     CRM_ASSERT(out != NULL);

     if (out->priv != NULL) {

-        private_data_t *priv = out->priv;

-        buf = dump_xml_formatted_with_text(priv->root);

-        fprintf(out->dest, "%s", buf);

-        free(buf);

+        finish_reset_common(out, CRM_EX_OK, true);

     }

     xml_free_priv(out);

-- 

1.8.3.1

From 0f8e4ca5d9a429c934f1e91a1bdf572efd07e0db Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Thu, 16 Jul 2020 16:09:08 -0400

Subject: [PATCH 3/4] Fix: tools, libcrmcommon: Reopen the output dest on

 reset.

This is needed when running crm_mon as a daemon.  When we do a reset,

we need to clear out any existing output destination and start writing

again from the beginning.  This really only matters when the destination

is a file.

The extra freopen at the end of crm_mon is to handle when crm_mon is

killed.  We need to reset the output file to its beginning before

calling finish.

---

 lib/common/output_html.c | 3 +++

 lib/common/output_log.c  | 3 +++

 lib/common/output_text.c | 3 +++

 lib/common/output_xml.c  | 3 +++

 tools/crm_mon.c          | 9 +++++++++

 5 files changed, 21 insertions(+)

diff --git a/lib/common/output_html.c b/lib/common/output_html.c

index 6127df2..6e21031 100644

--- a/lib/common/output_html.c

+++ b/lib/common/output_html.c

@@ -191,6 +191,9 @@ static void

 html_reset(pcmk__output_t *out) {

     CRM_ASSERT(out != NULL);

+    out->dest = freopen(NULL, "w", out->dest);

+    CRM_ASSERT(out->dest != NULL);

+

     if (out->priv != NULL) {

         finish_reset_common(out, CRM_EX_OK, true);

     }

diff --git a/lib/common/output_log.c b/lib/common/output_log.c

index 0208046..8422ac2 100644

--- a/lib/common/output_log.c

+++ b/lib/common/output_log.c

@@ -72,6 +72,9 @@ static void

 log_reset(pcmk__output_t *out) {

     CRM_ASSERT(out != NULL);

+    out->dest = freopen(NULL, "w", out->dest);

+    CRM_ASSERT(out->dest != NULL);

+

     log_free_priv(out);

     log_init(out);

 }

diff --git a/lib/common/output_text.c b/lib/common/output_text.c

index 8f15849..2f7e5b0 100644

--- a/lib/common/output_text.c

+++ b/lib/common/output_text.c

@@ -75,6 +75,9 @@ static void

 text_reset(pcmk__output_t *out) {

     CRM_ASSERT(out != NULL);

+    out->dest = freopen(NULL, "w", out->dest);

+    CRM_ASSERT(out->dest != NULL);

+

     text_free_priv(out);

     text_init(out);

 }

diff --git a/lib/common/output_xml.c b/lib/common/output_xml.c

index b64a71d..9f8e01b 100644

--- a/lib/common/output_xml.c

+++ b/lib/common/output_xml.c

@@ -165,6 +165,9 @@ static void

 xml_reset(pcmk__output_t *out) {

     CRM_ASSERT(out != NULL);

+    out->dest = freopen(NULL, "w", out->dest);

+    CRM_ASSERT(out->dest != NULL);

+

     if (out->priv != NULL) {

         finish_reset_common(out, CRM_EX_OK, true);

     }

diff --git a/tools/crm_mon.c b/tools/crm_mon.c

index b2e143b..10624c1 100644

--- a/tools/crm_mon.c

+++ b/tools/crm_mon.c

@@ -2014,6 +2014,10 @@ mon_refresh_display(gpointer user_data)

             break;

     }

+    if (options.daemonize) {

+        out->reset(out);

+    }

+

     stonith_history_free(stonith_history);

     stonith_history = NULL;

     pe_reset_working_set(mon_data_set);

@@ -2179,6 +2183,11 @@ clean_up(crm_exit_t exit_code)

      * crm_mon to be able to do so.

      */

     if (out != NULL) {

+        if (options.daemonize) {

+            out->dest = freopen(NULL, "w", out->dest);

+            CRM_ASSERT(out->dest != NULL);

+        }

+

         switch (output_format) {

             case mon_output_cgi:

             case mon_output_html:

-- 

1.8.3.1

From b655c039414d2c7af77c3532222b04684ef1f3d0 Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Fri, 17 Jul 2020 10:58:32 -0400

Subject: [PATCH 4/4] Fix: tools: Add the http-equiv header to crm_mon at the

 right time.

This header is only getting added on termination, which is not a lot of

help if you're running crm_mon in daemonize mode.  Putting this header

in at the right time requires a couple changes:

* pcmk__html_add_header doesn't need a parent argument.  It was not

being used in the first place.

* The extra_headers list in output_html.c should not be freed in the

reset function.  This means it would get freed after every time the

daemonized output is refreshed, which means the header would have to be

added every time too.  extra_headers will now only be freed when the

program exits.  This is a behavior change, but I can't see why it's

going to be a problem.

* To support that, we need to copy each item in the extra_headers list

when it gets added to the output XML document.  This prevents segfaults

when we later free that document.

* handle_html_output no longer needs to exist.  That function only

existed to add the http-equiv header at the end, which is wrong.

---

 include/crm/common/output.h |  5 ++---

 lib/common/output_html.c    |  7 ++++---

 tools/crm_mon.c             | 26 +++++++-------------------

 3 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/include/crm/common/output.h b/include/crm/common/output.h

index e7c9417..186bcfe 100644

--- a/include/crm/common/output.h

+++ b/include/crm/common/output.h

@@ -703,15 +703,14 @@ pcmk__output_create_html_node(pcmk__output_t *out, const char *element_name, con

  * the following code would generate the tag "<meta http-equiv='refresh' content='19'>":

  *

  * \code

- * pcmk__html_add_header(parent, "meta", "http-equiv", "refresh", "content", "19", NULL);

+ * pcmk__html_add_header("meta", "http-equiv", "refresh", "content", "19", NULL);

  * \endcode

  *

- * \param[in,out] parent The node that will be the parent of the new node.

  * \param[in]     name   The HTML tag for the new node.

  * \param[in]     ...    A NULL-terminated key/value list of attributes.

  */

 void

-pcmk__html_add_header(xmlNodePtr parent, const char *name, ...)

+pcmk__html_add_header(const char *name, ...)

 G_GNUC_NULL_TERMINATED;

 #ifdef __cplusplus

diff --git a/lib/common/output_html.c b/lib/common/output_html.c

index 6e21031..259e412 100644

--- a/lib/common/output_html.c

+++ b/lib/common/output_html.c

@@ -139,7 +139,7 @@ finish_reset_common(pcmk__output_t *out, crm_exit_t exit_status, bool print) {

     /* Add any extra header nodes the caller might have created. */

     for (int i = 0; i < g_slist_length(extra_headers); i++) {

-        xmlAddChild(head_node, g_slist_nth_data(extra_headers, i));

+        xmlAddChild(head_node, xmlCopyNode(g_slist_nth_data(extra_headers, i), 1));

     }

     /* Stylesheets are included two different ways.  The first is via a built-in

@@ -185,6 +185,8 @@ html_finish(pcmk__output_t *out, crm_exit_t exit_status, bool print, void **copy

     if (copy_dest != NULL) {

         *copy_dest = copy_xml(priv->root);

     }

+

+    g_slist_free_full(extra_headers, (GDestroyNotify) xmlFreeNode);

 }

 static void

@@ -199,7 +201,6 @@ html_reset(pcmk__output_t *out) {

     }

     html_free_priv(out);

-    g_slist_free_full(extra_headers, (GDestroyNotify) xmlFreeNode);

     html_init(out);

 }

@@ -412,7 +413,7 @@ pcmk__output_create_html_node(pcmk__output_t *out, const char *element_name, con

 }

 void

-pcmk__html_add_header(xmlNodePtr parent, const char *name, ...) {

+pcmk__html_add_header(const char *name, ...) {

     htmlNodePtr header_node;

     va_list ap;

diff --git a/tools/crm_mon.c b/tools/crm_mon.c

index 10624c1..7fd2b9c 100644

--- a/tools/crm_mon.c

+++ b/tools/crm_mon.c

@@ -1346,6 +1346,12 @@ main(int argc, char **argv)

         options.mon_ops |= mon_op_print_timing | mon_op_inactive_resources;

     }

+    if ((output_format == mon_output_html || output_format == mon_output_cgi) &&

+        out->dest != stdout) {

+        pcmk__html_add_header("meta", "http-equiv", "refresh", "content",

+                              crm_itoa(options.reconnect_msec/1000), NULL);

+    }

+

     crm_info("Starting %s", crm_system_name);

     if (cib) {

@@ -2106,15 +2112,6 @@ clean_up_connections(void)

     }

 }

-static void

-handle_html_output(crm_exit_t exit_code) {

-    xmlNodePtr html = NULL;

-

-    pcmk__html_add_header(html, "meta", "http-equiv", "refresh", "content",

-                          crm_itoa(options.reconnect_msec/1000), NULL);

-    out->finish(out, exit_code, true, (void **) &html);

-}

-

 /*

  * De-init ncurses, disconnect from the CIB manager, disconnect fencing,

  * deallocate memory and show usage-message if requested.

@@ -2188,16 +2185,7 @@ clean_up(crm_exit_t exit_code)

             CRM_ASSERT(out->dest != NULL);

         }

-        switch (output_format) {

-            case mon_output_cgi:

-            case mon_output_html:

-                handle_html_output(exit_code);

-                break;

-

-            default:

-                out->finish(out, exit_code, true, NULL);

-                break;

-        }

+        out->finish(out, exit_code, true, NULL);

         pcmk__output_free(out);

         pcmk__unregister_formats();

-- 

1.8.3.1