Fix varint decode. (#424)

* Split out varint code.
* Add tests.

Author: g-easy

opencensus/common/internal/BUILD

@@ -70,6 +70,14 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "varint",
+    srcs = ["varint.cc"],
+    hdrs = ["varint.h"],
+    copts = DEFAULT_COPTS,
+    deps = ["@com_google_absl//absl/strings"],
+)
+
 # Tests
 # ========================================================================= #
 
@@ -142,3 +150,14 @@ cc_test(
         "@com_google_protobuf//:protobuf",
     ],
 )
+
+cc_test(
+    name = "varint_test",
+    srcs = ["varint_test.cc"],
+    copts = TEST_COPTS,
+    deps = [
+        ":varint",
+        "@com_google_absl//absl/strings",
+        "@com_google_googletest//:gtest_main",
+    ],
+)

opencensus/common/internal/CMakeLists.txt

@@ -31,11 +31,19 @@ target_compile_definitions(opencensus_common_stats_object INTERFACE
 
 opencensus_lib(common_string_vector_hash DEPS absl::hash)
 
+opencensus_lib(common_varint SRCS varint.cc DEPS absl::strings)
+
+# Tests.
+
 opencensus_test(common_hostname_test hostname_test.cc common_hostname)
 
 opencensus_test(common_random_test random_test.cc common_random)
 
-opencensus_benchmark(common_random_benchmark random_benchmark.cc common_random)
-
 opencensus_test(common_stats_object_test stats_object_test.cc
                 common_stats_object absl::strings absl::span)
+
+opencensus_test(common_varint_test varint_test.cc common_varint)
+
+# Benchmarks.
+
+opencensus_benchmark(common_random_benchmark random_benchmark.cc common_random)

opencensus/common/internal/varint.cc

@@ -0,0 +1,59 @@
+// Copyright 2019, OpenCensus Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <cstdint>
+#include <string>
+
+#include "absl/strings/string_view.h"
+#include "opencensus/common/internal/varint.h"
+
+namespace opencensus {
+namespace common {
+
+void AppendVarint32(uint32_t i, std::string* out) {
+  do {
+    // Encode 7 bits.
+    uint8_t c = i & 0x7F;
+    i = i >> 7;
+    if (i != 0) {
+      c |= 0x80;
+    }
+    out->push_back(c);
+  } while (i != 0);
+}
+
+bool ParseVarint32(absl::string_view* input, uint32_t* out) {
+  absl::string_view s = *input;
+  uint32_t i = 0;
+  uint8_t c;
+  int shift = 0;
+  do {
+    if (s.empty()) {
+      return false;  // Too short.
+    }
+    c = s[0];
+    s = s.substr(1);
+    if (shift == 28 && c > 0x0f) {
+      return false;  // Out of range for uint32_t.
+    }
+    i |= (c & 0x7F) << shift;
+    shift += 7;
+  } while (c & 0x80);
+  *input = s;
+  *out = i;
+  return true;
+}
+
+}  // namespace common
+}  // namespace opencensus

opencensus/common/internal/varint.h

@@ -0,0 +1,36 @@
+// Copyright 2019, OpenCensus Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef OPENCENSUS_COMMON_INTERNAL_VARINT_H_
+#define OPENCENSUS_COMMON_INTERNAL_VARINT_H_
+
+#include <cstdint>
+#include <string>
+
+#include "absl/strings/string_view.h"
+
+namespace opencensus {
+namespace common {
+
+// Appends a variable-length encoded integer to the destination string.
+void AppendVarint32(uint32_t i, std::string* out);
+
+// Parses a variable-length encoded integer from the input. Returns false on
+// failure. Returns true and consumes the bytes from the input, on success.
+bool ParseVarint32(absl::string_view* input, uint32_t* out);
+
+}  // namespace common
+}  // namespace opencensus
+
+#endif  // OPENCENSUS_COMMON_INTERNAL_VARINT_H_

opencensus/common/internal/varint_test.cc

@@ -0,0 +1,75 @@
+// Copyright 2019, OpenCensus Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "opencensus/common/internal/varint.h"
+
+#include <iostream>
+
+#include "absl/strings/escaping.h"
+#include "absl/strings/string_view.h"
+#include "gtest/gtest.h"
+
+namespace opencensus {
+namespace common {
+namespace {
+
+TEST(Varint, EncodeDecode) {
+  auto test = [](uint32_t i) {
+    std::string s;
+    AppendVarint32(i, &s);
+    std::cout << " int " << i << " encoded to hex " << absl::BytesToHexString(s)
+              << "\n";
+    absl::string_view sv(s);
+    uint32_t j = i + 1;
+    EXPECT_TRUE(ParseVarint32(&sv, &j));
+    EXPECT_EQ(i, j);
+  };
+  test(0);
+  test(1);
+  test(10);
+  test(100);
+
+  test(127);
+  test(128);
+  test(129);
+
+  test(255);
+  test(256);
+  test(257);
+
+  test(16383);
+  test(16384);
+  test(16385);
+
+  test(2097151);
+  test(2097152);
+  test(2097153);
+
+  test(268435455);
+  test(268435456);
+  test(268435457);
+
+  test(4294967295);
+}
+
+TEST(Varint, DecodeOutOfRange) {
+  constexpr uint8_t input[] = {0xff, 0xff, 0xff, 0xff, 0x10};
+  absl::string_view sv(reinterpret_cast<const char*>(input), sizeof(input));
+  uint32_t i;
+  EXPECT_FALSE(ParseVarint32(&sv, &i));
+}
+
+}  // namespace
+}  // namespace common
+}  // namespace opencensus

opencensus/tags/BUILD

@@ -63,6 +63,7 @@ cc_library(
     visibility = ["//visibility:public"],
     deps = [
         ":tags",
+        "//opencensus/common/internal:varint",
         "@com_google_absl//absl/strings",
     ],
 )

opencensus/tags/CMakeLists.txt

@@ -40,6 +40,7 @@ opencensus_lib(
   internal/grpc_tags_bin.cc
   DEPS
   tags
+  common_varint
   absl::strings)
 
 opencensus_lib(

opencensus/tags/internal/grpc_tags_bin.cc

@@ -20,51 +20,22 @@
 #include <vector>
 
 #include "absl/strings/string_view.h"
+#include "opencensus/common/internal/varint.h"
 #include "opencensus/tags/tag_key.h"
 #include "opencensus/tags/tag_map.h"
 
+using opencensus::common::AppendVarint32;
+using opencensus::common::ParseVarint32;
+
 namespace opencensus {
 namespace tags {
 namespace propagation {
-
 namespace {
 
 constexpr char kVersionId = '\0';
 constexpr char kTagFieldId = '\0';
 constexpr int kMaxLen = 8192;
 
-// Appends a variable-length encoded integer to the destination string.
-void AppendVarint(unsigned int i, std::string* out) {
-  do {
-    // Encode 7 bits.
-    uint8_t c = i & 0x7F;
-    i = i >> 7;
-    if (i != 0) {
-      c |= 0x80;
-    }
-    out->push_back(c);
-  } while (i != 0);
-}
-
-// Parses a variable-length encoded integer from the input. Returns false on
-// failure. Returns true and consumes the bytes from the input, on success.
-bool ParseVarint(absl::string_view* input, int* out) {
-  absl::string_view s = *input;
-  int i = 0;
-  uint8_t c;
-  do {
-    if (s.empty()) {
-      return false;  // Too short.
-    }
-    c = s[0];
-    s = s.substr(1);
-    i = (i << 7) | (c & 0x7F);
-  } while (c & 0x80);
-  *input = s;
-  *out = i;
-  return true;
-}
-
 }  // namespace
 
 bool FromGrpcTagsBinHeader(absl::string_view header, TagMap* out) {
@@ -89,8 +60,8 @@ bool FromGrpcTagsBinHeader(absl::string_view header, TagMap* out) {
     // Parse key.
     absl::string_view key;
     {
-      int key_len;
-      if (!ParseVarint(&header, &key_len)) {
+      uint32_t key_len;
+      if (!ParseVarint32(&header, &key_len)) {
         return false;  // Invalid key_len.
       }
       if (key_len > header.length()) {
@@ -103,8 +74,8 @@ bool FromGrpcTagsBinHeader(absl::string_view header, TagMap* out) {
     // Parse val.
     absl::string_view val;
     {
-      int val_len;
-      if (!ParseVarint(&header, &val_len)) {
+      uint32_t val_len;
+      if (!ParseVarint32(&header, &val_len)) {
         return false;  // Invalid val_len.
       }
       if (val_len > header.length()) {
@@ -138,9 +109,9 @@ std::string ToGrpcTagsBinHeader(const TagMap& tags) {
     const auto& key = key_val.first;
     const auto& val = key_val.second;
     out.push_back(kTagFieldId);
-    AppendVarint(key.name().length(), &out);
+    AppendVarint32(key.name().length(), &out);
     out.append(key.name());
-    AppendVarint(val.length(), &out);
+    AppendVarint32(val.length(), &out);
     // Encoded value must be UTF-8.
     out.append(val);
     if (out.size() > kMaxLen) {

opencensus/tags/internal/grpc_tags_bin_test.cc

@@ -140,6 +140,14 @@ TEST(GrpcTagsBinTest, Serialize) {
             ToGrpcTagsBinHeader(m));
 }
 
+TEST(GrpcTagsBinTest, SerializeLong) {
+  TagMap m1({{TagKey::Register(std::string(300, 'A')), std::string(400, 'B')}});
+  const std::string s = ToGrpcTagsBinHeader(m1);
+  TagMap m2({});
+  EXPECT_TRUE(FromGrpcTagsBinHeader(s, &m2));
+  EXPECT_THAT(m1.tags(), ::testing::ContainerEq(m2.tags()));
+}
+
 TEST(GrpcTagsBinTest, SerializeTooLong) {
   std::vector<std::pair<opencensus::tags::TagKey, std::string>> tags;
   constexpr int kValLen = 20;