discarded the in-house implementation of the hex (base16) encoding/decoding.

we can use the generalized framework used for base64/base33hex and get less
buggy behavior.  some more tests are added, and the bug in the in-house
implementation was fixed.


git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac256@2485 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/dns/Makefile.am

@@ -63,7 +63,7 @@ libdns___la_SOURCES += util/binary_from_base32hex.h
 libdns___la_SOURCES += buffer.h
 libdns___la_SOURCES += dnssectime.h dnssectime.cc
 libdns___la_SOURCES += exceptions.h exceptions.cc
-libdns___la_SOURCES += util/hex.h util/hex.cc
+libdns___la_SOURCES += util/hex.h
 libdns___la_SOURCES += message.h message.cc
 libdns___la_SOURCES += messagerenderer.h messagerenderer.cc
 libdns___la_SOURCES += name.h name.cc

src/lib/dns/tests/hex_unittest.cc

@@ -19,14 +19,14 @@
 #include <vector>
 #include <string>
 
+#include <exceptions/exceptions.h>
+
 #include <dns/util/hex.h>
 
 #include <gtest/gtest.h>
 
-#include "unittest_util.h"
-
-using isc::UnitTestUtil;
 using namespace std;
+using namespace isc;
 using namespace isc::dns;
 
 namespace {
@@ -34,7 +34,14 @@ const string hex_txt("DEADBEEFDECADE");
 const string hex_txt_space("DEAD BEEF DECADE");
 const string hex_txt_lower("deadbeefdecade");
 
-TEST(HexTest, encodeHex) {
+class HexTest : public ::testing::Test {
+protected:
+    HexTest() : encoding_chars("0123456789ABCDEF") {}
+    vector<uint8_t> decoded_data;
+    const string encoding_chars;
+};
+
+TEST_F(HexTest, encodeHex) {
     std::vector<uint8_t> data;
 
     data.push_back(0xde);
@@ -58,7 +65,7 @@ compareData(const std::vector<uint8_t>& data) {
     EXPECT_EQ(0xde, data[6]);
 }
 
-TEST(HexTest, decodeHex) {
+TEST_F(HexTest, decodeHex) {
     std::vector<uint8_t> result;
 
     decodeHex(hex_txt, result);
@@ -76,11 +83,39 @@ TEST(HexTest, decodeHex) {
 
     // Bogus input: should fail
     result.clear();
-    EXPECT_THROW(decodeHex("1x", result), BadHexString);
+    EXPECT_THROW(decodeHex("1x", result), BadValue);
 
     // Bogus input: encoded string must have an even number of characters.
     result.clear();
-    EXPECT_THROW(decodeHex("dea", result), BadHexString);
+    EXPECT_THROW(decodeHex("dea", result), BadValue);
+}
+
+// For Hex encode/decode we use handmade mappings, so it's prudent to test the
+// entire mapping table explicitly.
+TEST_F(HexTest, decodeMap) {
+    string input("00");       // input placeholder
+
+    for (int i = 0; i < 256; ++i) {
+        input[1] = i;
+
+        const char ch = toupper(i);
+        const size_t pos = encoding_chars.find(ch);
+        if (pos == string::npos) {
+            EXPECT_THROW(decodeHex(input, decoded_data), BadValue);
+        } else {
+            decodeHex(input, decoded_data);
+            EXPECT_EQ(1, decoded_data.size());
+            EXPECT_EQ(pos, decoded_data[0]);
+        }
+    }
+}
+
+TEST_F(HexTest, encodeMap) {
+    for (int i = 0; i < 16; ++i) {
+        decoded_data.clear();
+        decoded_data.push_back(i);
+        EXPECT_EQ(encoding_chars[i], encodeHex(decoded_data)[1]);
+    }
 }
 
 }

src/lib/dns/tests/rdata_nsec3_unittest.cc

@@ -48,30 +48,33 @@ public:
     string nsec3_txt;
 };
 
-TEST_F(Rdata_NSEC3_Test, toText)
-{
+TEST_F(Rdata_NSEC3_Test, toText) {
     const generic::NSEC3 rdata_nsec3(nsec3_txt);
     EXPECT_EQ(nsec3_txt, rdata_nsec3.toText());
 }
 
 TEST_F(Rdata_NSEC3_Test, badText) {
-    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1 1 ADDAFEE "
+    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1 1 ADDAFEEE "
                                             "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                             "BIFF POW SPOON"),
                  InvalidRdataText);
     EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1 1 ADDAFEE "
                                             "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW "
                                             "A NS SOA"),
+                 BadValue);     // bad hex
+    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1 1 ADDAFEEE "
+                                            "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW "
+                                            "A NS SOA"),
                  BadValue);     // bad base32hex
-    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1000000 1 1 ADDAFEE "
+    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1000000 1 1 ADDAFEEE "
                                             "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                             "A NS SOA"),
                  InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1000000 1 ADDAFEE "
+    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1000000 1 ADDAFEEE "
                                             "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                             "A NS SOA"),
                  InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1 1000000 ADDAFEE "
+    EXPECT_THROW(generic::NSEC3 rdata_nsec3("1 1 1000000 ADDAFEEE "
                                             "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                             "A NS SOA"),
                  InvalidRdataText);
@@ -83,8 +86,7 @@ TEST_F(Rdata_NSEC3_Test, DISABLED_badText) { // this currently fails
                      "NS SOA RRSIG DNSKEY NSEC3PARAM"), InvalidRdataText);
 }
 
-TEST_F(Rdata_NSEC3_Test, createFromWire)
-{
+TEST_F(Rdata_NSEC3_Test, createFromWire) {
     const generic::NSEC3 rdata_nsec3(nsec3_txt);
     EXPECT_EQ(0, rdata_nsec3.compare(
                   *rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
@@ -101,8 +103,7 @@ TEST_F(Rdata_NSEC3_Test, createFromWire)
                  DNSMessageFORMERR);
 }
 
-TEST_F(Rdata_NSEC3_Test, toWireRenderer)
-{
+TEST_F(Rdata_NSEC3_Test, toWireRenderer) {
     renderer.skip(2);
     const generic::NSEC3 rdata_nsec3(nsec3_txt);
     rdata_nsec3.toWire(renderer);
@@ -114,14 +115,12 @@ TEST_F(Rdata_NSEC3_Test, toWireRenderer)
                         obuffer.getLength() - 2, &data[2], data.size() - 2);
 }
 
-TEST_F(Rdata_NSEC3_Test, toWireBuffer)
-{
+TEST_F(Rdata_NSEC3_Test, toWireBuffer) {
     const generic::NSEC3 rdata_nsec3(nsec3_txt);
     rdata_nsec3.toWire(obuffer);
 }
 
-TEST_F(Rdata_NSEC3_Test, assign)
-{
+TEST_F(Rdata_NSEC3_Test, assign) {
     generic::NSEC3 rdata_nsec3(nsec3_txt);
     generic::NSEC3 other_nsec3 = rdata_nsec3;
     EXPECT_EQ(0, rdata_nsec3.compare(other_nsec3));

src/lib/dns/tests/rdata_nsec3param_unittest.cc

@@ -16,6 +16,8 @@
 
 #include <string>
 
+#include <exceptions/exceptions.h>
+
 #include <dns/util/base32hex.h>
 #include <dns/buffer.h>
 #include <dns/util/hex.h>
@@ -32,6 +34,7 @@
 
 using isc::UnitTestUtil;
 using namespace std;
+using namespace isc;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
@@ -49,7 +52,7 @@ TEST_F(Rdata_NSEC3PARAM_Test, toText)
 
 TEST_F(Rdata_NSEC3PARAM_Test, badText)
 {
-    EXPECT_THROW(generic::NSEC3PARAM("1 1 1 SPORK"), BadHexString);
+    EXPECT_THROW(generic::NSEC3PARAM("1 1 1 SPORK"), BadValue); // bad hex
     EXPECT_THROW(generic::NSEC3PARAM("100000 1 1 ADDAFEE"), InvalidRdataText);
     EXPECT_THROW(generic::NSEC3PARAM("1 100000 1 ADDAFEE"), InvalidRdataText);
     EXPECT_THROW(generic::NSEC3PARAM("1 1 100000 ADDAFEE"), InvalidRdataText);

src/lib/dns/util/base_n.cc

@@ -28,6 +28,9 @@
 #include <dns/util/base32hex_from_binary.h>
 #include <dns/util/binary_from_base32hex.h>
 
+#include <dns/util/base16_from_binary.h>
+#include <dns/util/binary_from_base16.h>
+
 #include <exceptions/exceptions.h>
 
 #include <dns/util/base32hex.h>
@@ -279,6 +282,17 @@ transform_width<binary_from_base32hex<DecodeNormalizer, char>, 8, 5, char>
 base32hex_decoder;
 typedef BaseNTransformer<5, '0', base32hex_encoder, base32hex_decoder>
 Base32HexTransformer;
+
+//
+// Instantiation for BASE-16 (HEX)
+//
+typedef
+base16_from_binary<transform_width<EncodeNormalizer, 4, 8> > base16_encoder;
+typedef
+transform_width<binary_from_base16<DecodeNormalizer, char>, 8, 4, char>
+base16_decoder;
+typedef BaseNTransformer<4, '0', base16_encoder, base16_decoder>
+Base16Transformer;
 }
 
 string
@@ -301,5 +315,15 @@ decodeBase32Hex(const string& input, vector<uint8_t>& result) {
     Base32HexTransformer::decode("base32hex", input, result);
 }
 
+string
+encodeHex(const vector<uint8_t>& binary) {
+    return (Base16Transformer::encode(binary));
+}
+
+void
+decodeHex(const string& input, vector<uint8_t>& result) {
+    Base16Transformer::decode("base16", input, result);
+}
+
 }
 }

src/lib/dns/util/hex.cc

@@ -1,108 +0,0 @@
-// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
-//
-// Permission to use, copy, modify, and/or distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-// $Id$
-
-#include <cassert>
-#include <iterator>
-#include <iomanip>
-#include <iostream>
-#include <sstream>
-#include <string>
-#include <vector>
-
-
-#include <exceptions/exceptions.h>
-#include <boost/foreach.hpp>
-#include <ctype.h>
-#include <stdint.h>
-
-#include <dns/util/hex.h>
-
-using namespace std;
-
-namespace isc {
-namespace dns {
-
-namespace {
-const char hexdigits[] = "0123456789ABCDEF";
-}
-
-std::string
-encodeHex(const std::vector<uint8_t>& binary) {
-    // calculate the resulting length.  it should be twice the
-    // original data length
-    const size_t len = (binary.size() * 2);
-    std::ostringstream hex;
-
-    BOOST_FOREACH(uint8_t octet, binary) {
-        hex << hexdigits[octet >> 4] << hexdigits[octet & 0xf];
-    }
-    assert(len >= hex.str().length());
-    return (hex.str());
-}
-
-void
-decodeHex(const std::string& hex, std::vector<uint8_t>& result) {
-    ostringstream comp;
-
-    // compress input by removing whitespace
-    const size_t len = hex.length();
-    for (int i = 0; i < len; ++i) {
-        char c = hex.at(i);
-        if (c == ' ' || c == '\t' || c == '\r' || c == '\n') {
-            continue;
-        }
-        comp << c;
-    }
-
-    istringstream iss(comp.str());
-    result.clear();
-    char c1, c2;
-
-    iss.width(1);
-    if ((comp.str().length() % 2) == 1) {
-        c2 = '0';
-        iss >> c2;
-
-        const char* pos = strchr(hexdigits, toupper(c2));
-        if (pos == NULL) {
-            isc_throw(BadHexString, "Invalid hex digit");
-        }
-
-        if (!iss.eof() && !iss.bad() && !iss.fail()) {
-            result.push_back(pos - hexdigits);
-        }
-    }
-    while (!iss.eof()) {
-        c1 = c2 = '0';
-        iss >> c1 >> c2;;
-
-        if (iss.eof() || iss.fail() || iss.bad()) {
-            break;
-        }
-
-        const char* pos1 = strchr(hexdigits, toupper(c1));
-        const char* pos2 = strchr(hexdigits, toupper(c2));
-        if (!pos1 || !pos2) {
-            isc_throw(BadHexString, "Invalid hex digit");
-        }
-
-        const uint8_t n = ((pos1 - hexdigits) << 4) | (pos2 - hexdigits);
-        result.push_back(n);
-    }
-}
-
-}
-}

src/lib/dns/util/hex.h

@@ -17,11 +17,10 @@
 #ifndef __HEX_H
 #define __HEX_H 1
 
+#include <stdint.h>
 #include <string>
 #include <vector>
 
-#include <exceptions/exceptions.h>
-
 //
 // Note: this helper module isn't specific to the DNS protocol per se.
 // We should probably move this to somewhere else, possibly in some common
@@ -30,19 +29,31 @@
 
 namespace isc {
 namespace dns {
-
+/// \brief Encode binary data in the base16 ('hex') format.
 ///
-/// \brief A standard DNS (or ISC) module exception that is thrown if a hex
-/// decoder encounters an invalid input.
+/// The underlying implementation is shared with \c encodeBase64, and most of
+/// the description except the format (base16) equally applies.
+/// Another notable exception is that the base16 encoding doesn't require
+/// padding, so padding related considerations and the notion of canonical
+/// encoding don't apply.
 ///
-class BadHexString : public Exception {
-public:
-    BadHexString(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) {}
-};
-
+/// \param binary A vector object storing the data to be encoded. 
+/// \return A newly created string that stores base16 encoded value for
+/// binary.
 std::string encodeHex(const std::vector<uint8_t>& binary);
-void decodeHex(const std::string& hex, std::vector<uint8_t>& result);
+
+/// \brief Decode a text encoded in the base16 ('hex') format into the
+/// original %data.
+///
+/// The underlying implementation is shared with \c decodeBase64, and most
+/// of the description except the format (base16) equally applies.
+/// Another notable exception is that the base16 encoding doesn't require
+/// padding, so padding related considerations and the notion of canonical
+/// encoding don't apply.
+///
+/// \param input A text encoded in the base16 format.
+/// \param result A vector in which the decoded %data is to be stored.
+void decodeHex(const std::string& input, std::vector<uint8_t>& result);
 }
 }