From 870d34ad03109c7b62bf9c2dd56510b593b6eb09 Mon Sep 17 00:00:00 2001
From: David Anderson <davea@ssl.berkeley.edu>
Date: Mon, 14 Aug 2017 14:46:30 -0700
Subject: [PATCH] Fix parsing of AM reply messages - use std::string instead of
 malloced array for ACCT_MGR_OP::global_prefs_xml - use
 copy_element_contents() instead of dup_element_contents()   to get global
 prefs.   The latter uses fgets instead of fgetc,   so it requires that close
 tag be on a line by itself.   TODO: don't use fgets anywhere in XML parsing.
 - fix a bug in copy_element_contents() where it consumes an extra character

---
 client/acct_mgr.cpp   | 14 ++++++--------
 client/acct_mgr.h     |  4 ++--
 client/client_state.h |  2 +-
 client/cs_prefs.cpp   |  2 +-
 lib/parse.cpp         | 15 ++++++++-------
 5 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/client/acct_mgr.cpp b/client/acct_mgr.cpp
index 402368841d..82121ddddf 100644
--- a/client/acct_mgr.cpp
+++ b/client/acct_mgr.cpp
@@ -66,10 +66,7 @@ int ACCT_MGR_OP::do_rpc(
     error_num = ERR_IN_PROGRESS;
     error_str = "";
     via_gui = _via_gui;
-    if (global_prefs_xml) {
-        free(global_prefs_xml);
-        global_prefs_xml = 0;
-    }
+    global_prefs_xml = "";
 
     // if null URL, detach from current AMS
     //
@@ -406,10 +403,10 @@ int ACCT_MGR_OP::parse(FILE* f) {
             continue;
         }
         if (xp.match_tag("global_preferences")) {
-            retval = dup_element_contents(
+            retval = copy_element_contents(
                 f,
                 "</global_preferences>",
-                &global_prefs_xml
+                global_prefs_xml
             );
             if (retval) {
                 msg_printf(NULL, MSG_INTERNAL_ERROR,
@@ -434,6 +431,7 @@ int ACCT_MGR_OP::parse(FILE* f) {
         if (xp.match_tag("user_keywords")) {
             retval = ami.user_keywords.parse(xp);
             if (retval) return retval;
+            continue;
         }
         if (log_flags.unparsed_xml) {
             msg_printf(NULL, MSG_INFO,
@@ -705,9 +703,9 @@ void ACCT_MGR_OP::handle_reply(int http_op_retval) {
 
         // process prefs if any
         //
-        if (global_prefs_xml) {
+        if (!global_prefs_xml.empty()) {
             retval = gstate.save_global_prefs(
-                global_prefs_xml, ami.master_url, ami.master_url
+                global_prefs_xml.c_str(), ami.master_url, ami.master_url
             );
             if (retval) {
                 msg_printf(NULL, MSG_INTERNAL_ERROR, "Can't save global prefs");
diff --git a/client/acct_mgr.h b/client/acct_mgr.h
index e71c2ca71a..2cdc13ff80 100644
--- a/client/acct_mgr.h
+++ b/client/acct_mgr.h
@@ -145,7 +145,7 @@ struct ACCT_MGR_OP: public GUI_HTTP_OP {
     std::string error_str;
     std::vector<AM_ACCOUNT> accounts;
     double repeat_sec;
-    char* global_prefs_xml;
+    std::string global_prefs_xml;
     char host_venue[256];
     bool got_rss_feeds;
     std::vector<RSS_FEED>rss_feeds;
@@ -162,7 +162,7 @@ struct ACCT_MGR_OP: public GUI_HTTP_OP {
         via_gui = false;
         error_num = BOINC_SUCCESS;
         repeat_sec = 60.0;
-        global_prefs_xml = 0;
+        global_prefs_xml = "";
         safe_strcpy(host_venue, "");
         got_rss_feeds = false;
     }
diff --git a/client/client_state.h b/client/client_state.h
index b59ef0674b..364598c420 100644
--- a/client/client_state.h
+++ b/client/client_state.h
@@ -418,7 +418,7 @@ struct CLIENT_STATE {
         const char* fname = GLOBAL_PREFS_FILE_NAME,
         const char* override_fname = GLOBAL_PREFS_OVERRIDE_FILE
     );
-    int save_global_prefs(char* prefs, char* url, char* sched);
+    int save_global_prefs(const char* prefs, char* url, char* sched);
     double available_ram();
     double max_available_ram();
     int check_suspend_processing();
diff --git a/client/cs_prefs.cpp b/client/cs_prefs.cpp
index 2843c96c39..3d9a76dec6 100644
--- a/client/cs_prefs.cpp
+++ b/client/cs_prefs.cpp
@@ -722,7 +722,7 @@ void CLIENT_STATE::read_global_prefs(
 }
 
 int CLIENT_STATE::save_global_prefs(
-    char* global_prefs_xml, char* master_url, char* scheduler_url
+    const char* global_prefs_xml, char* master_url, char* scheduler_url
 ) {
     FILE* f = boinc_fopen(GLOBAL_PREFS_FILE_NAME, "w");
     if (!f) return ERR_FOPEN;
diff --git a/lib/parse.cpp b/lib/parse.cpp
index d2987fa4da..a38bb24788 100644
--- a/lib/parse.cpp
+++ b/lib/parse.cpp
@@ -256,15 +256,16 @@ int copy_element_contents(FILE* in, const char* end_tag, string& str) {
     while (1) {
         c = fgetc(in);
         if (c == EOF) break;
-        if (n >= end_tag_len) {
-            const char* p = str.c_str() + n - end_tag_len;
-            if (!strcmp(p, end_tag)) {
-                str.erase(n-end_tag_len, end_tag_len);
-                return 0;
-            }
-        }
         str += c;
         n++;
+        if (n < end_tag_len) {
+            continue;
+        }
+        const char* p = str.c_str() + n - end_tag_len;
+        if (!strcmp(p, end_tag)) {
+            str.erase(n-end_tag_len, end_tag_len);
+            return 0;
+        }
     }
     return ERR_XML_PARSE;
 }