You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2019/01/15 19:10:28 UTC

[trafficserver] branch master updated: Issue #4637 - Clean up / unify hex conversions Clean up port hex conversion in SSLAddressLookupKey.

This is an automated email from the ASF dual-hosted git repository.

amc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new acc47ef  Issue #4637 - Clean up / unify hex conversions Clean up port hex conversion in SSLAddressLookupKey.
acc47ef is described below

commit acc47efd083bd48ceecdb0beaf75c98ca07e2973
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Sun Nov 25 16:39:04 2018 -0600

    Issue #4637 - Clean up / unify hex conversions
    Clean up port hex conversion in SSLAddressLookupKey.
---
 .gitignore                                       |  1 +
 include/tscore/BufferWriter.h                    | 45 ++++++++++++++++++++++++
 include/tscore/bwf_std_format.h                  |  3 +-
 include/tscore/ink_inet.h                        | 10 ++++++
 iocore/net/SSLCertLookup.cc                      | 33 ++++++++---------
 src/tscore/BufferWriterFormat.cc                 | 41 +++++++++++++--------
 src/tscore/ink_inet.cc                           |  9 +++++
 src/tscore/unit_tests/test_BufferWriterFormat.cc | 20 +++++++++++
 src/tscore/unit_tests/test_ink_inet.cc           |  8 +++++
 9 files changed, 135 insertions(+), 35 deletions(-)

diff --git a/.gitignore b/.gitignore
index a63e882..e56e2fe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,6 +18,7 @@ Makefile.in
 
 .project
 .cproject
+.idea
 
 .diags.log.meta
 
diff --git a/include/tscore/BufferWriter.h b/include/tscore/BufferWriter.h
index 6da3e12..f5fce06 100644
--- a/include/tscore/BufferWriter.h
+++ b/include/tscore/BufferWriter.h
@@ -906,6 +906,51 @@ FixedBufferWriter::printv(BWFormat const &fmt, std::tuple<Args...> const &args)
   return static_cast<self_type &>(this->super_type::printv(fmt, args));
 }
 
+// Basic format wrappers - these are here because they're used internally.
+namespace bwf
+{
+  namespace detail
+  {
+    /** Write out raw memory in hexadecimal wrapper.
+     *
+     * This wrapper indicates the contained view should be dumped as raw memory in hexadecimal format.
+     * This is intended primarily for internal use by other formatting logic.
+     *
+     * @see Hex_Dump
+     */
+    struct MemDump {
+      std::string_view _view;
+
+      /** Dump @a n bytes starting at @a mem as hex.
+       *
+       * @param mem First byte of memory to dump.
+       * @param n Number of bytes.
+       */
+      MemDump(void const *mem, size_t n) : _view(static_cast<char const *>(mem), n) {}
+    };
+  } // namespace detail
+
+  /** Treat @a t as raw memory and dump the memory as hexadecimal.
+   *
+   * @tparam T Type of argument.
+   * @param t Object to dump.
+   * @return @a A wrapper to do a hex dump.
+   *
+   * This is the standard way to do a hexadecimal memory dump of an object.
+   *
+   * @internal This function exists so that other types can overload it for special processing,
+   * which would not be possible with just @c HexDump.
+   */
+  template <typename T>
+  detail::MemDump
+  Hex_Dump(T const &t)
+  {
+    return {&t, sizeof(T)};
+  }
+} // namespace bwf
+
+BufferWriter &bwformat(BufferWriter &w, BWFSpec const &spec, bwf::detail::MemDump const &hex);
+
 } // end namespace ts
 
 namespace std
diff --git a/include/tscore/bwf_std_format.h b/include/tscore/bwf_std_format.h
index ce7f4f7..1e4a267 100644
--- a/include/tscore/bwf_std_format.h
+++ b/include/tscore/bwf_std_format.h
@@ -121,7 +121,8 @@ namespace bwf
       }
     }
   };
-} // namespace bwf
+
+}; // namespace bwf
 
 BufferWriter &bwformat(BufferWriter &w, BWFSpec const &spec, bwf::Errno const &e);
 BufferWriter &bwformat(BufferWriter &w, BWFSpec const &spec, bwf::Date const &date);
diff --git a/include/tscore/ink_inet.h b/include/tscore/ink_inet.h
index 4cbf14c..b72aefa 100644
--- a/include/tscore/ink_inet.h
+++ b/include/tscore/ink_inet.h
@@ -1558,4 +1558,14 @@ bwformat(BufferWriter &w, BWFSpec const &spec, IpEndpoint const &addr)
 {
   return bwformat(w, spec, &addr.sa);
 }
+
+namespace bwf
+{
+  namespace detail
+  {
+    struct MemDump;
+  } // namespace detail
+
+  detail::MemDump Hex_Dump(IpEndpoint const &addr);
+} // namespace bwf
 } // namespace ts
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 2c06ca6..508ea2f 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -31,6 +31,8 @@
 #include "tscore/MatcherUtils.h"
 #include "tscore/Regex.h"
 #include "tscore/Trie.h"
+#include "tscore/BufferWriter.h"
+#include "tscore/bwf_std_format.h"
 #include "tscore/TestBox.h"
 
 #include <unordered_map>
@@ -45,26 +47,19 @@
 #endif /* SSL_CTX_set_tlsext_ticket_key_cb */
 
 struct SSLAddressLookupKey {
-  explicit SSLAddressLookupKey(const IpEndpoint &ip) : sep(0)
+  explicit SSLAddressLookupKey(const IpEndpoint &ip)
   {
-    static const char hextab[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
-
-    int nbytes;
-    uint16_t port = ntohs(ip.port());
-
-    // For IP addresses, the cache key is the hex address with the port concatenated. This makes the lookup
-    // insensitive to address formatting and also allow the longest match semantic to produce different matches
-    // if there is a certificate on the port.
-    nbytes = ats_ip_to_hex(&ip.sa, key, sizeof(key));
-    if (port) {
-      sep           = nbytes;
-      key[nbytes++] = '.';
-      key[nbytes++] = hextab[(port >> 12) & 0x000F];
-      key[nbytes++] = hextab[(port >> 8) & 0x000F];
-      key[nbytes++] = hextab[(port >> 4) & 0x000F];
-      key[nbytes++] = hextab[(port)&0x000F];
+    // For IP addresses, the cache key is the hex address with the port concatenated. This makes the
+    // lookup insensitive to address formatting and also allow the longest match semantic to produce
+    // different matches if there is a certificate on the port.
+
+    ts::FixedBufferWriter w{key, sizeof(key)};
+    w.print("{}", ts::bwf::Hex_Dump(ip)); // dump raw bytes in hex, don't format as IP address.
+    if (in_port_t port = ip.host_order_port(); port) {
+      sep = unsigned(w.size());
+      w.print(".{:x}", port);
     }
-    key[nbytes++] = 0;
+    w.print("\0");
   }
 
   const char *
@@ -85,7 +80,7 @@ struct SSLAddressLookupKey {
 
 private:
   char key[(TS_IP6_SIZE * 2) /* hex addr */ + 1 /* dot */ + 4 /* port */ + 1 /* nullptr */];
-  unsigned char sep; // offset of address/port separator
+  unsigned char sep = 0; // offset of address/port separator
 };
 
 struct SSLContextStorage {
diff --git a/src/tscore/BufferWriterFormat.cc b/src/tscore/BufferWriterFormat.cc
index 7efd5e0..70a5e01 100644
--- a/src/tscore/BufferWriterFormat.cc
+++ b/src/tscore/BufferWriterFormat.cc
@@ -32,6 +32,7 @@
 #include <cmath>
 #include <array>
 #include <chrono>
+#include <exception>
 
 using namespace std::literals;
 
@@ -587,7 +588,7 @@ namespace bw_fmt
 
   /// Write out the @a data as hexadecimal, using @a digits as the conversion.
   void
-  Hex_Dump(BufferWriter &w, std::string_view data, const char *digits)
+  Format_As_Hex(BufferWriter &w, std::string_view data, const char *digits)
   {
     const char *ptr = data.data();
     for (auto n = data.size(); n > 0; --n) {
@@ -608,14 +609,7 @@ bwformat(BufferWriter &w, BWFSpec const &spec, std::string_view sv)
   }
 
   if ('x' == spec._type || 'X' == spec._type) {
-    const char *digits = 'x' == spec._type ? bw_fmt::LOWER_DIGITS : bw_fmt::UPPER_DIGITS;
-    width -= sv.size() * 2;
-    if (spec._radix_lead_p) {
-      w.write('0');
-      w.write(spec._type);
-      width -= 2;
-    }
-    bw_fmt::Write_Aligned(w, [&w, &sv, digits]() { bw_fmt::Hex_Dump(w, sv, digits); }, spec._align, width, spec._fill, 0);
+    return bwformat(w, spec, bwf::detail::MemDump(sv.data(), sv.size()));
   } else {
     width -= sv.size();
     bw_fmt::Write_Aligned(w, [&w, &sv]() { w.write(sv); }, spec._align, width, spec._fill, 0);
@@ -628,12 +622,7 @@ bwformat(BufferWriter &w, BWFSpec const &spec, MemSpan const &span)
 {
   static const BWFormat default_fmt{"{:#x}@{:p}"};
   if (spec._ext.size() && 'd' == spec._ext.front()) {
-    const char *digits = 'X' == spec._type ? bw_fmt::UPPER_DIGITS : bw_fmt::LOWER_DIGITS;
-    if (spec._radix_lead_p) {
-      w.write('0');
-      w.write(digits[33]);
-    }
-    bw_fmt::Hex_Dump(w, span.view(), digits);
+    bwformat(w, spec, bwf::detail::MemDump(span.data(), span.size()));
   } else {
     w.print(default_fmt, span.size(), span.data());
   }
@@ -958,6 +947,28 @@ bwformat(BufferWriter &w, BWFSpec const &spec, bwf::OptionalAffix const &opts)
   return w.write(opts._prefix).write(opts._text).write(opts._suffix);
 }
 
+BufferWriter &
+bwformat(BufferWriter &w, BWFSpec const &spec, bwf::detail::MemDump const &hex)
+{
+  char fmt_type      = spec._type;
+  const char *digits = bw_fmt::UPPER_DIGITS;
+
+  if ('X' != fmt_type) {
+    fmt_type = 'x';
+    digits   = bw_fmt::LOWER_DIGITS;
+  }
+
+  int width = int(spec._min) - hex._view.size() * 2; // amount left to fill.
+  if (spec._radix_lead_p) {
+    w.write('0');
+    w.write(fmt_type);
+    width -= 2;
+  }
+  bw_fmt::Write_Aligned(w, [&w, &hex, digits]() { bw_fmt::Format_As_Hex(w, hex._view, digits); }, spec._align, width, spec._fill,
+                        0);
+  return w;
+}
+
 } // namespace ts
 
 namespace
diff --git a/src/tscore/ink_inet.cc b/src/tscore/ink_inet.cc
index 1252487..c52c316 100644
--- a/src/tscore/ink_inet.cc
+++ b/src/tscore/ink_inet.cc
@@ -937,4 +937,13 @@ bwformat(BufferWriter &w, BWFSpec const &spec, sockaddr const *addr)
   return w;
 }
 
+namespace bwf
+{
+  detail::MemDump
+  Hex_Dump(IpEndpoint const &addr)
+  {
+    return detail::MemDump(ats_ip_addr8_cast(&addr), ats_ip_addr_size(&addr));
+  }
+} // namespace bwf
+
 } // namespace ts
diff --git a/src/tscore/unit_tests/test_BufferWriterFormat.cc b/src/tscore/unit_tests/test_BufferWriterFormat.cc
index 1e3a70b..6e0062e 100644
--- a/src/tscore/unit_tests/test_BufferWriterFormat.cc
+++ b/src/tscore/unit_tests/test_BufferWriterFormat.cc
@@ -25,6 +25,7 @@
 #include "../../../tests/include/catch.hpp"
 #include <chrono>
 #include <iostream>
+#include <netinet/in.h>
 #include "tscore/BufferWriter.h"
 #include "tscore/bwf_std_format.h"
 #include "tscpp/util/MemSpan.h"
@@ -574,6 +575,25 @@ TEST_CASE("bwstring std formats", "[libts][bwprint]")
   REQUIRE(w.view() == "name = Evil Dave");
   w.reset().print("name = {}", ts::bwf::FirstOf(empty, empty, s3, empty, s2, s1));
   REQUIRE(w.view() == "name = Leif");
+
+  unsigned v = ntohl(0xdeadbeef);
+  w.reset().print("{}", ts::bwf::Hex_Dump(v));
+  REQUIRE(w.view() == "deadbeef");
+  w.reset().print("{:x}", ts::bwf::Hex_Dump(v));
+  REQUIRE(w.view() == "deadbeef");
+  w.reset().print("{:X}", ts::bwf::Hex_Dump(v));
+  REQUIRE(w.view() == "DEADBEEF");
+  w.reset().print("{:#X}", ts::bwf::Hex_Dump(v));
+  REQUIRE(w.view() == "0XDEADBEEF");
+  w.reset().print("{} bytes {} digits {}", sizeof(double), std::numeric_limits<double>::digits10, ts::bwf::Hex_Dump(2.718281828));
+  REQUIRE(w.view() == "8 bytes 15 digits 9b91048b0abf0540");
+
+  INK_MD5 md5;
+  w.reset().print("{}", ts::bwf::Hex_Dump(md5));
+  REQUIRE(w.view() == "00000000000000000000000000000000");
+  CryptoContext().hash_immediate(md5, s2.data(), s2.size());
+  w.reset().print("{}", ts::bwf::Hex_Dump(md5));
+  REQUIRE(w.view() == "f240ccd7a95c7ec66d6c111e2925b23e");
 }
 
 // Normally there's no point in running the performance tests, but it's worth keeping the code
diff --git a/src/tscore/unit_tests/test_ink_inet.cc b/src/tscore/unit_tests/test_ink_inet.cc
index bd57498..4fd4747 100644
--- a/src/tscore/unit_tests/test_ink_inet.cc
+++ b/src/tscore/unit_tests/test_ink_inet.cc
@@ -219,6 +219,10 @@ TEST_CASE("inet formatting", "[libts][ink_inet][bwformat]")
   REQUIRE(w.view() == "172. 17. 99.231");
   w.reset().print("{::=a}", ep);
   REQUIRE(w.view() == "172.017.099.231");
+  w.reset().print("{}", ts::bwf::Hex_Dump(ep));
+  REQUIRE(w.view() == "ac1163e7");
+  w.reset().print("{:#X}", ts::bwf::Hex_Dump(ep));
+  REQUIRE(w.view() == "0XAC1163E7");
 
   // Documentation examples
   REQUIRE(0 == ats_ip_pton(addr_7, &ep.sa));
@@ -247,4 +251,8 @@ TEST_CASE("inet formatting", "[libts][ink_inet][bwformat]")
 
   w.reset().print("{:p}", reinterpret_cast<sockaddr const *>(0x1337beef));
   REQUIRE(w.view() == "0x1337beef");
+
+  ats_ip_pton(addr_1, &ep.sa);
+  w.reset().print("{}", ts::bwf::Hex_Dump(ep));
+  REQUIRE(w.view() == "ffee00000000000024c333493cee0143");
 }