xCAT::SvrUtils::sendmsg() does not properly handle ":" in the error message text

[gurevichmark]

There seems to be a problem how xCAT::SvrUtils::sendmsg() parses the error message text containing :

For example:

1. If message is error type and there is NO : in the text, it gets displayed properly: Coded message: xCAT::SvrUtils::sendmsg([ 1, "Mark1-Mark2"], $callback, $node); Output: fs2vm105: Error: Mark1-Mark2

2. If message is error type and there is : in the text, it is NOT displayed properly: Coded message: xCAT::SvrUtils::sendmsg([ 1, "Mark1:Mark2"], $callback, $node); Output: fs2vm105: Error: Mark2

3. If message is NOT error type and there is : in the text, it gets displayed properly (but an extra space is inserted after the : : Coded message: xCAT::SvrUtils::sendmsg([ 0, "Mark1:Mark2"], $callback, $node); Output: fs2vm105: Mark1: Mark2

[immarvin]

hi @gurevichmark ,

The response format in xCAT defined in perl-xCAT/xCAT/Client.pm is like this:

1071 # Format of the response hash:
1072 #  {data => [ 'data str1', 'data str2', '...' ] }
1073 #
1074 #    Results are printed as:
1075 #       data str1
1076 #       data str2
1077 #
1078 # or:
1079 #  {data => [ {desc => [ 'desc1' ],
1080 #              contents => [ 'contents1' ] },
1081 #             {desc => [ 'desc2 ],
1082 #              contents => [ 'contents2' ] }
1083 #                :
1084 #            ] }
1085 #    NOTE:  In this format, only the data array can have more than one
1086 #           element. All other arrays are assumed to be a single element.
1087 #    Results are printed as:
1088 #       desc1: contents1
1089 #       desc2: contents2
1090 #
1091 # or:
1092 #  {node => [ {name => ['node1'],
1093 #              data => [ {desc => [ 'node1 desc' ],
1094 #                         contents => [ 'node1 contents' ] } ] },
1095 #             {name => ['node2'],
1096 #              data => [ {desc => [ 'node2 desc' ],
1097 #                         contents => [ 'node2 contents' ] } ] },
1098 #                :
1099 #             ] }
1100 #    NOTE:  Only the node array can have more than one element.
1101 #           All other arrays are assumed to be a single element.

looked into xCAT::SvrUtils::sendmsg() code, : is used as the delimiter between desc and contents with format desc: contents, for error type message with :, the splited desc is overwritten by desc Error , this is why you see problem 2.

1550 #-------------------------------------------------------------------------------------------
1551 # Common method to send info back to the client
1552 # The last two args are optional, though $allerrornodes will unlikely be there without $node
1553 # TODO: investigate possibly removing this and using MsgUtils instead
1554 #
1555 #--------------------------------------------------------------------------------------------
1556 sub sendmsg {
1557     my $text          = shift;
1558     my $callback      = shift;
1559     my $node          = shift;
1560     my %allerrornodes = shift;
1561     my $descr;
1562     my $rc;
1563     if (ref $text eq 'HASH') {
1564         die "not right now";
1565     } elsif (ref $text eq 'ARRAY') {
1566         $rc   = $text->[0];
1567         $text = $text->[1];
1568     }
1569     if ($text =~ /:/) {
1570         ($descr, $text) = split /:/, $text, 2;
1571     }
1572     $text =~ s/^ *//;
1573     $text =~ s/ *$//;
1574     my $msg;
1575     my $curptr;
1576     if ($node) {
1577         $msg->{node} = [ { name => [$node] } ];
1578         $curptr = $msg->{node}->[0];
1579     } else {
1580         $msg    = {};
1581         $curptr = $msg;
1582     }
1583     if ($rc) {
1584         $curptr->{errorcode} = [$rc];
1585         $curptr->{error}     = [$text];
1586         $curptr              = $curptr->{error}->[0];
1587         if (defined $node && %allerrornodes) {
1588             $allerrornodes{$node} = 1;
1589         }
1590     } else {
1591         $curptr->{data} = [ { contents => [$text] } ];
1592         $curptr = $curptr->{data}->[0];
1593         if ($descr) { $curptr->{desc} = [$descr]; }
1594     }
1595
1596     #        print $outfd freeze([$msg]);
1597     #        print $outfd "\nENDOFFREEZE6sK4ci\n";
1598     #        yield;
1599     #        waitforack($outfd);
1600     $callback->($msg);
}

you can try to create a fix for this, or just as the description of xCAT::SvrUtils::sendmsg() mentions:

#-------------------------------------------------------------------------------------------
# Common method to send info back to the client
# The last two args are optional, though $allerrornodes will unlikely be there without $node
# TODO: investigate possibly removing this and using MsgUtils instead
#
#--------------------------------------------------------------------------------------------

the xCAT::SvrUtils::sendmsg() will be deprecated in the future release, so another option is to xCAT::MsgUtils->message instead.

thanks

[gurevichmark]

@immarvin Yes, xCAT::MsgUtils->message handles : properly, so best to use that instead of xCAT::SvrUtils::sendmsg(). I am not sure I want to make changes to xCAT::SvrUtils::sendmsg(), because it is used so much in out code, and would be so easy to break something.

I can see the problem in SvrUtils.pm. On line 1570, we split the text on :. The part of the string before : gets put into $descr. Then if it is not error message we use it, but if it is an error message on line 1583, we never use $descr. Not sure if there is an easy way to fix it.

[immarvin]

hi @gurevichmark , maybe a simple fix can be :

backup the original $text to $text_origin before

1569     if ($text =~ /:/) {
1570         ($descr, $text) = split /:/, $text, 2;
1571     }

then if $rc!=0, it is an error message, push $text_origin to $curptr->{error} instead of

1585         $curptr->{error}     = [$text];

[gurevichmark]

@immarvin Thanks for the suggestion. I tested it and it works as expected. Added in PR #2983