From 744f0d1f2775bd5c51bfbf47d4d89a29001aafcf Mon Sep 17 00:00:00 2001
From: Bill Erickson <berickxx@gmail.com>
Date: Wed, 19 Sep 2018 15:34:49 -0400
Subject: [PATCH] LP#1703411 XMPP opensrf sub-element repairs

* Message template typo repair -- missing "'"
* XPath repair on path to opensrf sub-element for Perl
* Move 'type' attribute get/set back up to the <message> for Perl.
* Clean up code duplication in the C message building libs.
* Squash a centuries-old memory leak where xmlFree(sender) was only
  called if a router_from was not supplied.

Signed-off-by: Bill Erickson <berickxx@gmail.com>
Signed-off-by: Jason Stephenson <jason@sigio.com>
Signed-off-by: Mike Rylander <mrylander@gmail.com>
---
 src/libopensrf/transport_message.c            | 71 +++++--------------
 .../Transport/SlimJabber/XMPPMessage.pm       |  6 +-
 .../Transport/SlimJabber/XMPPReader.pm        |  4 +-
 3 files changed, 22 insertions(+), 59 deletions(-)

diff --git a/src/libopensrf/transport_message.c b/src/libopensrf/transport_message.c
index f95debc..332b622 100644
--- a/src/libopensrf/transport_message.c
+++ b/src/libopensrf/transport_message.c
@@ -120,25 +120,16 @@ transport_message* new_message_from_xml( const char* msg_xml ) {
 	xmlChar* recipient      = xmlGetProp( root, BAD_CAST "to");
 	xmlChar* subject        = xmlGetProp( root, BAD_CAST "subject");
 	xmlChar* thread         = xmlGetProp( root, BAD_CAST "thread" );
-	xmlChar* router_from    = xmlGetProp( root, BAD_CAST "router_from" );
-	xmlChar* router_to      = xmlGetProp( root, BAD_CAST "router_to" );
-	xmlChar* router_class   = xmlGetProp( root, BAD_CAST "router_class" );
-	xmlChar* router_command = xmlGetProp( root, BAD_CAST "router_command" );
-	xmlChar* broadcast      = xmlGetProp( root, BAD_CAST "broadcast" );
-	xmlChar* osrf_xid       = xmlGetProp( root, BAD_CAST "osrf_xid" );
-
-	if( osrf_xid ) {
-		message_set_osrf_xid( new_msg, (char*) osrf_xid);
-		xmlFree(osrf_xid);
-	}
-
-	if( router_from ) {
-		new_msg->sender     = strdup((const char*)router_from);
-	} else {
-		if( sender ) {
-			new_msg->sender = strdup((const char*)sender);
-			xmlFree(sender);
-		}
+	xmlChar* router_from    = NULL;
+	xmlChar* router_to      = NULL;
+	xmlChar* router_class   = NULL;
+	xmlChar* router_command = NULL;
+	xmlChar* broadcast      = NULL;
+	xmlChar* osrf_xid       = NULL;
+
+	if( sender ) {
+		new_msg->sender = strdup((const char*)sender);
+		xmlFree(sender);
 	}
 
 	if( recipient ) {
@@ -156,34 +147,8 @@ transport_message* new_message_from_xml( const char* msg_xml ) {
 		xmlFree(thread);
 	}
 
-	if(router_from) {
-		new_msg->router_from = strdup((const char*)router_from);
-		xmlFree(router_from);
-	}
-
-	if(router_to) {
-		new_msg->router_to  = strdup((const char*)router_to);
-		xmlFree(router_to);
-	}
-
-	if(router_class) {
-		new_msg->router_class = strdup((const char*)router_class);
-		xmlFree(router_class);
-	}
-
-	if(router_command) {
-		new_msg->router_command = strdup((const char*)router_command);
-		xmlFree(router_command);
-	}
-
-	if(broadcast) {
-		if(strcmp((const char*) broadcast,"0") )
-			new_msg->broadcast = 1;
-		xmlFree(broadcast);
-	}
-
-	/* Within the message element, find the child nodes for "thread", "subject", and */
-	/* "body".  Extract their textual content into the corresponding members. */
+	/* Within the message element, find the child nodes for "thread", "subject" */
+	/* "body", and "opensrf".  Extract their textual content into the corresponding members. */
 	xmlNodePtr search_node = root->children;
 	while( search_node != NULL ) {
 
@@ -211,15 +176,13 @@ transport_message* new_message_from_xml( const char* msg_xml ) {
 			}
 
 			if( router_from ) {
-				new_msg->sender     = strdup((const char*)router_from);
-			} else {
-				if( sender ) {
-					new_msg->sender = strdup((const char*)sender);
-					xmlFree(sender);
+				if (new_msg->sender) {
+					// Sender value applied above.  Clear it and
+					// use the router value instead.
+					free(new_msg->sender);
 				}
-			}
 
-			if(router_from) {
+				new_msg->sender = strdup((const char*)router_from);
 				new_msg->router_from = strdup((const char*)router_from);
 				xmlFree(router_from);
 			}
diff --git a/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPMessage.pm b/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPMessage.pm
index 44e2040..0fc4124 100644
--- a/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPMessage.pm
+++ b/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPMessage.pm
@@ -6,7 +6,7 @@ use strict; use warnings;
 use XML::LibXML;
 
 use constant JABBER_MESSAGE =>
-    "<message to='%s' from='%s>".
+    "<message to='%s' from='%s'>".
     "<opensrf router_command='%s' router_class='%s' osrf_xid='%s'/>".
     "<thread>%s</thread><body>%s</body></message>";
 
@@ -121,7 +121,7 @@ sub parse_xml {
     throw $err if $err;
 
     my $root = $doc->documentElement;
-    my $osrf_node = $root->findnodes('/opensrf')->shift;
+    my $osrf_node = $root->findnodes('/message/opensrf')->shift;
 
     $self->{body} = $root->findnodes('/message/body').'';
     $self->{thread} = $root->findnodes('/message/thread').'';
@@ -131,7 +131,7 @@ sub parse_xml {
 
     $self->{to} = $root->getAttribute('to');
 
-    $self->{type} = $osrf_node->getAttribute('type');
+    $self->{type} = $root->getAttribute('type');
     $self->{osrf_xid} = $osrf_node->getAttribute('osrf_xid');
 }
 
diff --git a/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPReader.pm b/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPReader.pm
index 737cf96..0a84ae1 100644
--- a/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPReader.pm
+++ b/src/perl/lib/OpenSRF/Transport/SlimJabber/XMPPReader.pm
@@ -307,14 +307,14 @@ sub start_element {
         my $msg = $self->{message};
         $msg->{to} = $attrs{'to'};
         $msg->{from} = $attrs{from};
+        $msg->{type} = $attrs{type};
 
     } elsif($name eq 'opensrf') {
 
         # These will be authoritative if they exist
         my $msg = $self->{message};
-        $msg->{from} = $attrs{router_from};
+        $msg->{from} = $attrs{router_from} if $attrs{router_from};
         $msg->{osrf_xid} = $attrs{'osrf_xid'};
-        $msg->{type} = $attrs{type};
 
     } elsif($name eq 'body') {
         $self->{xml_state} = IN_BODY;
-- 
2.43.2