[#2844] pretty print options with binary data whenever possible

Author: Razvan Becheriu

src/lib/dhcp/option.cc

@@ -236,6 +236,12 @@ std::string Option::toText(int indent) const {
         output << setfill('0') << setw(2) << hex
                << static_cast<unsigned short>(data_[i]);
     }
+    if (!data_.size()) {
+        output << "(no data)";
+    } else if (str::isPrintable(data_)) {
+        std::string printable(data_.cbegin(), data_.cend());
+        output << " (" << printable << ")";
+    }
 
     // Append suboptions.
     output << suboptionsToText(indent + 2);

src/lib/dhcp/option_custom.cc

@@ -9,9 +9,11 @@
 #include <dhcp/option_data_types.h>
 #include <dhcp/option_custom.h>
 #include <exceptions/isc_assert.h>
+#include <util/str.h>
 #include <util/encode/encode.h>
 
 using namespace isc::asiolink;
+using namespace isc::util;
 
 namespace isc {
 namespace dhcp {
@@ -398,8 +400,19 @@ OptionCustom::dataFieldToText(const OptionDataType data_type,
     // Get the value of the data field.
     switch (data_type) {
     case OPT_BINARY_TYPE:
-        text << util::encode::encodeHex(readBinary(index));
+    {
+        auto data = readBinary(index);
+        if (data.empty()) {
+            text << "(no data)";
+        } else {
+            text << util::encode::encodeHex(data);
+            if (str::isPrintable(data)) {
+                std::string printable(data.cbegin(), data.cend());
+                text << " (" << printable << ") ";
+            }
+        }
         break;
+    }
     case OPT_BOOLEAN_TYPE:
         text << (readBoolean(index) ? "true" : "false");
         break;
@@ -438,6 +451,7 @@ OptionCustom::dataFieldToText(const OptionDataType data_type,
     {
         PSIDTuple t = readPsid(index);
         text << "len=" << t.first.asUnsigned() << ",psid=" << t.second.asUint16();
+        break;
     }
     default:
         ;

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -1284,14 +1284,27 @@ TEST_F(Pkt4Test, toText) {
     pkt.addOption(OptionPtr(new Option4AddrLst(123, IOAddress("192.0.2.3"))));
     pkt.addOption(OptionPtr(new OptionUint32(Option::V4, 156, 123456)));
     pkt.addOption(OptionPtr(new OptionString(Option::V4, 87, "lorem ipsum")));
+    OptionBuffer data = { 'a', 'b', 'c', 'd', 'e', 'f' };
+    OptionPtr opt(new Option(Option::V4, 231, data));
+    pkt.addOption(opt);
+    OptionBuffer data_sub = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0' };
+    OptionPtr sub_opt(new Option(Option::V4, 1, data_sub));
+    opt->addOption(sub_opt);
+    data_sub.clear();
+    sub_opt.reset(new Option(Option::V4, 2, data_sub));
+    opt->addOption(sub_opt);
 
     EXPECT_EQ("local_address=192.0.2.34:67, remote_address=192.10.33.4:68,\n"
               "msg_type=DHCPDISCOVER (1), trans_id=0x9ef,\n"
               "options:\n"
               "  type=053, len=001: 1 (uint8)\n"
               "  type=087, len=011: \"lorem ipsum\" (string)\n"
               "  type=123, len=004: 192.0.2.3\n"
-              "  type=156, len=004: 123456 (uint32)",
+              "  type=156, len=004: 123456 (uint32)\n"
+              "  type=231, len=021: 61:62:63:64:65:66 (abcdef),\n"
+              "options:\n"
+              "    type=001, len=011: 30:31:32:33:34:35:36:37:38:39:30 (01234567890)\n"
+              "    type=002, len=000: (no data)",
               pkt.toText());
 
     // Now remove all options, including Message Type and check if the
@@ -1300,6 +1313,7 @@ TEST_F(Pkt4Test, toText) {
     pkt.delOption(156);
     pkt.delOption(87);
     pkt.delOption(53);
+    pkt.delOption(231);
 
     EXPECT_EQ("local_address=192.0.2.34:67, remote_address=192.10.33.4:68,\n"
               "msg_type=(missing), trans_id=0x9ef,\n"

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -1297,12 +1297,12 @@ TEST_F(Pkt6Test, toText) {
         "2 relay(s):\n"
         "relay[0]: msg-type=12(RELAY_FORWARD), hop-count=1,\n"
         "link-address=2001:888:db8:1::, peer-address=fe80::200:21ff:fe5c:18a9, 2 option(s)\n"
-        "type=00018, len=00028: 49:53:41:4d:31:34:34:7c:32:39:39:7c:69:70:76:36:7c:6e:74:3a:76:70:3a:31:3a:31:31:30\n"
+        "type=00018, len=00028: 49:53:41:4d:31:34:34:7c:32:39:39:7c:69:70:76:36:7c:6e:74:3a:76:70:3a:31:3a:31:31:30 (ISAM144|299|ipv6|nt:vp:1:110)\n"
         "type=00037, len=00018: 6527 (uint32) 0001000118B033410000215C18A9 (binary)\n"
         "relay[1]: msg-type=12(RELAY_FORWARD), hop-count=0,\n"
         "link-address=::, peer-address=fe80::200:21ff:fe5c:18a9, 2 option(s)\n"
-        "type=00018, len=00021: 49:53:41:4d:31:34:34:20:65:74:68:20:31:2f:31:2f:30:35:2f:30:31\n"
-        "type=00037, len=00004: 3561 (uint32)  (binary)\n";
+        "type=00018, len=00021: 49:53:41:4d:31:34:34:20:65:74:68:20:31:2f:31:2f:30:35:2f:30:31 (ISAM144 eth 1/1/05/01)\n"
+        "type=00037, len=00004: 3561 (uint32) (no data) (binary)\n";
 
     EXPECT_EQ(expected, msg->toText());
 }