From 6798315327690fdbe93add15159be5b925779bfe Mon Sep 17 00:00:00 2001
From: Tao Bao <tbao@google.com>
Date: Sat, 4 Nov 2017 00:08:08 -0700
Subject: otautil: Remove the aborts in RangeSet::Parse().

We used to CHECK and abort on parsing errors. While it works fine for
the updater use case (because recovery starts updater in a forked
process and collects the process exit code), it's difficult for other
clients to use RangeSet as a library (e.g. update_verifier).

This CL switches the aborts to returning empty RangeSet instead. Callers
need to check the parsing results explicitly.

The CL also separates RangeSet::PushBack() into a function, and moves
SortedRangeSet::Clear() into RangeSet.

Test: recovery_unit_test
Test: Sideload an OTA package with the new updater on angler.
Test: Sideload an OTA package with injected range string errors. The
      updater aborts from the explicit checks.
Change-Id: If2b7f6f41dc93af917a21c7877a83e98dc3fd016
---
 otautil/rangeset.cpp | 85 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 56 insertions(+), 29 deletions(-)

(limited to 'otautil/rangeset.cpp')

diff --git a/otautil/rangeset.cpp b/otautil/rangeset.cpp
index a121a4efc..532cba4a8 100644
--- a/otautil/rangeset.cpp
+++ b/otautil/rangeset.cpp
@@ -16,8 +16,10 @@
 
 #include "otautil/rangeset.h"
 
+#include <limits.h>
 #include <stddef.h>
 
+#include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ -28,47 +30,79 @@
 #include <android-base/strings.h>
 
 RangeSet::RangeSet(std::vector<Range>&& pairs) {
-  CHECK_NE(pairs.size(), static_cast<size_t>(0)) << "Invalid number of tokens";
+  blocks_ = 0;
+  if (pairs.empty()) {
+    LOG(ERROR) << "Invalid number of tokens";
+    return;
+  }
 
-  // Sanity check the input.
-  size_t result = 0;
   for (const auto& range : pairs) {
-    CHECK_LT(range.first, range.second) << "Empty or negative range: " << range.first << ", "
-                                        << range.second;
-    size_t sz = range.second - range.first;
-    CHECK_LE(result, SIZE_MAX - sz) << "RangeSet size overflow";
-    result += sz;
+    if (!PushBack(range)) {
+      Clear();
+      return;
+    }
   }
-
-  ranges_ = pairs;
-  blocks_ = result;
 }
 
 RangeSet RangeSet::Parse(const std::string& range_text) {
   std::vector<std::string> pieces = android::base::Split(range_text, ",");
-  CHECK_GE(pieces.size(), static_cast<size_t>(3)) << "Invalid range text: " << range_text;
+  if (pieces.size() < 3) {
+    LOG(ERROR) << "Invalid range text: " << range_text;
+    return {};
+  }
 
   size_t num;
-  CHECK(android::base::ParseUint(pieces[0], &num, static_cast<size_t>(INT_MAX)))
-      << "Failed to parse the number of tokens: " << range_text;
-
-  CHECK_NE(num, static_cast<size_t>(0)) << "Invalid number of tokens: " << range_text;
-  CHECK_EQ(num % 2, static_cast<size_t>(0)) << "Number of tokens must be even: " << range_text;
-  CHECK_EQ(num, pieces.size() - 1) << "Mismatching number of tokens: " << range_text;
+  if (!android::base::ParseUint(pieces[0], &num, static_cast<size_t>(INT_MAX))) {
+    LOG(ERROR) << "Failed to parse the number of tokens: " << range_text;
+    return {};
+  }
+  if (num == 0) {
+    LOG(ERROR) << "Invalid number of tokens: " << range_text;
+    return {};
+  }
+  if (num % 2 != 0) {
+    LOG(ERROR) << "Number of tokens must be even: " << range_text;
+    return {};
+  }
+  if (num != pieces.size() - 1) {
+    LOG(ERROR) << "Mismatching number of tokens: " << range_text;
+    return {};
+  }
 
   std::vector<Range> pairs;
   for (size_t i = 0; i < num; i += 2) {
     size_t first;
-    CHECK(android::base::ParseUint(pieces[i + 1], &first, static_cast<size_t>(INT_MAX)));
     size_t second;
-    CHECK(android::base::ParseUint(pieces[i + 2], &second, static_cast<size_t>(INT_MAX)));
-
+    if (!android::base::ParseUint(pieces[i + 1], &first, static_cast<size_t>(INT_MAX)) ||
+        !android::base::ParseUint(pieces[i + 2], &second, static_cast<size_t>(INT_MAX))) {
+      return {};
+    }
     pairs.emplace_back(first, second);
   }
-
   return RangeSet(std::move(pairs));
 }
 
+bool RangeSet::PushBack(Range range) {
+  if (range.first >= range.second) {
+    LOG(ERROR) << "Empty or negative range: " << range.first << ", " << range.second;
+    return false;
+  }
+  size_t sz = range.second - range.first;
+  if (blocks_ >= SIZE_MAX - sz) {
+    LOG(ERROR) << "RangeSet size overflow";
+    return false;
+  }
+
+  ranges_.push_back(std::move(range));
+  blocks_ += sz;
+  return true;
+}
+
+void RangeSet::Clear() {
+  ranges_.clear();
+  blocks_ = 0;
+}
+
 std::string RangeSet::ToString() const {
   if (ranges_.empty()) {
     return "";
@@ -114,8 +148,6 @@ bool RangeSet::Overlaps(const RangeSet& other) const {
   return false;
 }
 
-static constexpr size_t kBlockSize = 4096;
-
 // Ranges in the the set should be mutually exclusive; and they're sorted by the start block.
 SortedRangeSet::SortedRangeSet(std::vector<Range>&& pairs) : RangeSet(std::move(pairs)) {
   std::sort(ranges_.begin(), ranges_.end());
@@ -158,11 +190,6 @@ void SortedRangeSet::Insert(size_t start, size_t len) {
   Insert(to_insert);
 }
 
-void SortedRangeSet::Clear() {
-  blocks_ = 0;
-  ranges_.clear();
-}
-
 bool SortedRangeSet::Overlaps(size_t start, size_t len) const {
   RangeSet rs({ { start / kBlockSize, (start + len - 1) / kBlockSize + 1 } });
   return Overlaps(rs);
-- 
cgit v1.2.3