update_engine: Store and test kern/fw versions as 4 values
- Refactors the handling of _firmware_version and _kernel_version
attributes to split as 4x 16 bit values.
- Based on discussion, we are trying to avoid using the client
specific version format in the network protocol.
- Each of firmware and kernel versions will be stored separately
as uint16_t rather than combined as a uint32_t.
- In the Omaha response the two fields of each version type will
be encoded as key_version.version
- Adds tests for the actual parsing of the response.
BUG=chromium:840432
TEST=cros_run_unit_tests --board=samus --packages update_engine
Change-Id: I4da02e3f4715a132db7e96e55c30139fff6fe6ea
Reviewed-on: https://chromium-review.googlesource.com/1123106
Commit-Ready: Marton Hunyady <hunyadym@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/common/utils.cc b/common/utils.cc
index b06954b..1a8fd53 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -61,6 +61,7 @@
using base::Time;
using base::TimeDelta;
using std::min;
+using std::numeric_limits;
using std::pair;
using std::string;
using std::vector;
@@ -1082,6 +1083,36 @@
return value;
}
+void ParseRollbackKeyVersion(const string& raw_version,
+ uint16_t* high_version,
+ uint16_t* low_version) {
+ DCHECK(high_version);
+ DCHECK(low_version);
+ *high_version = numeric_limits<uint16_t>::max();
+ *low_version = numeric_limits<uint16_t>::max();
+
+ vector<string> parts = base::SplitString(
+ raw_version, ".", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+ if (parts.size() != 2) {
+ // The version string must have exactly one period.
+ return;
+ }
+
+ int high;
+ int low;
+ if (!(base::StringToInt(parts[0], &high) &&
+ base::StringToInt(parts[1], &low))) {
+ // Both parts of the version could not be parsed correctly.
+ return;
+ }
+
+ if (high >= 0 && high < numeric_limits<uint16_t>::max() && low >= 0 &&
+ low < numeric_limits<uint16_t>::max()) {
+ *high_version = static_cast<uint16_t>(high);
+ *low_version = static_cast<uint16_t>(low);
+ }
+}
+
} // namespace utils
} // namespace chromeos_update_engine
diff --git a/common/utils.h b/common/utils.h
index cbc5eb9..ecb97a3 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -21,6 +21,7 @@
#include <unistd.h>
#include <algorithm>
+#include <limits>
#include <map>
#include <memory>
#include <set>
@@ -322,6 +323,13 @@
// first section of |version| is invalid (e.g. not a number).
int VersionPrefix(const std::string& version);
+// Parses a string in the form high.low, where high and low are 16 bit unsigned
+// integers. If there is more than 1 dot, or if either of the two parts are
+// not valid 16 bit unsigned numbers, then 0xffff is returned for both.
+void ParseRollbackKeyVersion(const std::string& raw_version,
+ uint16_t* high_version,
+ uint16_t* low_version);
+
} // namespace utils
diff --git a/common/utils_unittest.cc b/common/utils_unittest.cc
index 16fa481..1590eeb 100644
--- a/common/utils_unittest.cc
+++ b/common/utils_unittest.cc
@@ -22,6 +22,7 @@
#include <sys/stat.h>
#include <sys/types.h>
+#include <limits>
#include <string>
#include <vector>
@@ -32,6 +33,7 @@
#include "update_engine/common/test_utils.h"
+using std::numeric_limits;
using std::string;
using std::vector;
@@ -450,6 +452,22 @@
*ret = true;
}
+static void ExpectParseRollbackKeyVersion(const string& version,
+ uint16_t expected_high,
+ uint16_t expected_low) {
+ uint16_t actual_high;
+ uint16_t actual_low;
+ utils::ParseRollbackKeyVersion(version, &actual_high, &actual_low);
+ EXPECT_EQ(expected_high, actual_high);
+ EXPECT_EQ(expected_low, actual_low);
+}
+
+static void ExpectInvalidParseRollbackKeyVersion(const string& version) {
+ ExpectParseRollbackKeyVersion(version,
+ numeric_limits<uint16_t>::max(),
+ numeric_limits<uint16_t>::max());
+}
+
TEST(UtilsTest, TestMacros) {
bool void_test = false;
VoidMacroTestHelper(&void_test);
@@ -523,4 +541,29 @@
EXPECT_EQ(-1, utils::VersionPrefix("x.1"));
}
+TEST(UtilsTest, ParseDottedVersion) {
+ // Valid case.
+ ExpectParseRollbackKeyVersion("2.3", 2, 3);
+ ExpectParseRollbackKeyVersion("65535.65535", 65535, 65535);
+
+ // Zero is technically allowed but never actually used.
+ ExpectParseRollbackKeyVersion("0.0", 0, 0);
+
+ // Invalid cases.
+ ExpectInvalidParseRollbackKeyVersion("");
+ ExpectInvalidParseRollbackKeyVersion("2");
+ ExpectInvalidParseRollbackKeyVersion("2.");
+ ExpectInvalidParseRollbackKeyVersion(".2");
+ ExpectInvalidParseRollbackKeyVersion("2.2.");
+ ExpectInvalidParseRollbackKeyVersion("2.2.3");
+ ExpectInvalidParseRollbackKeyVersion(".2.2");
+ ExpectInvalidParseRollbackKeyVersion("a.b");
+ ExpectInvalidParseRollbackKeyVersion("1.b");
+ ExpectInvalidParseRollbackKeyVersion("a.2");
+ ExpectInvalidParseRollbackKeyVersion("65536.65536");
+ ExpectInvalidParseRollbackKeyVersion("99999.99999");
+ ExpectInvalidParseRollbackKeyVersion("99999.1");
+ ExpectInvalidParseRollbackKeyVersion("1.99999");
+}
+
} // namespace chromeos_update_engine