mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ne...@apache.org
Subject [6/8] mesos git commit: Allowed leading zeros in input to stout's Version parser.
Date Wed, 10 May 2017 04:58:30 GMT
Allowed leading zeros in input to stout's Version parser.

The original behavior was to allow leading zeros. When Version was
enhanced with more complete support for SemVer [1], it was also changed
to reject leading zeros in numeric components (version numbers and
prerelease identifiers). Although this change was consistent with the
SemVer spec (which mandates that such version strings "MUST" be
considered invalid), it breaks compatibility with the versions used by
some common software packages (e.g., Docker).

This commit reverts the change in behavior, so that leading zeros are
once again allowed. In the future, we might consider adding a "strict"
mode to the Version parser, and/or supporting an "options" scheme that
would allow the user to customize the version parser's behavior.

[1] 287556284d76e03c11cff3fc076224fe966096e0

Review: https://reviews.apache.org/r/59105/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d43d7453
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d43d7453
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d43d7453

Branch: refs/heads/1.3.x
Commit: d43d745353eaf9f27cd11956cf578b95fac2c3dd
Parents: 1af90df
Author: Neil Conway <neil.conway@gmail.com>
Authored: Tue May 9 17:50:11 2017 -0700
Committer: Neil Conway <neil.conway@gmail.com>
Committed: Tue May 9 21:29:26 2017 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/version.hpp | 63 ++++++++-------------------
 3rdparty/stout/tests/version_tests.cpp   | 14 +++---
 2 files changed, 26 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d43d7453/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index 4be7222..bff6e4c 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -30,7 +30,14 @@
 
 // This class provides convenience routines for working with version
 // numbers.  We support the SemVer 2.0.0 (http://semver.org) format,
-// with minor extensions.
+// with two differences:
+//
+//   (1) Numeric components with leading zeros are allowed.
+//
+//   (2) Missing version components are allowed and treated as zero.
+//
+// TODO(neilc): Consider providing a "strict" variant that does not
+// allow these extensions.
 //
 // Ideally, the version components would be called simply "major",
 // "minor", and "patch". However, GNU libstdc++ already defines these
@@ -42,11 +49,6 @@ struct Version
   //   <major>[.<minor>[.<patch>]][-prerelease][+build]
   //
   // Missing `minor` or `patch` components are treated as zero.
-  // Allowing some version numbers to be omitted is an extension to
-  // SemVer, albeit one that several other SemVer libraries implement.
-  //
-  // TODO(neilc): Consider providing a "strict" variant that does not
-  // allow version numbers to be omitted.
   //
   // `prerelease` is a prerelease label (e.g., "beta", "rc.1");
   // `build` is a build metadata label. Both `prerelease` and `build`
@@ -69,9 +71,7 @@ struct Version
     if (buildParts.size() == 2) {
       const std::string& buildString = buildParts.back();
 
-      // NOTE: Build metadata identifiers can contain leading zeros
-      // (unlike numeric prerelease identifiers; see below).
-      Try<std::vector<std::string>> parsed = parseLabel(buildString, false);
+      Try<std::vector<std::string>> parsed = parseLabel(buildString);
       if (parsed.isError()) {
         return Error("Invalid build label: " + parsed.error());
       }
@@ -92,8 +92,7 @@ struct Version
     if (prereleaseParts.size() == 2) {
       const std::string& prereleaseString = prereleaseParts.back();
 
-      // Prerelease identifiers cannot contain leading zeros.
-      Try<std::vector<std::string>> parsed = parseLabel(prereleaseString, true);
+      Try<std::vector<std::string>> parsed = parseLabel(prereleaseString);
       if (parsed.isError()) {
         return Error("Invalid prerelease label: " + parsed.error());
       }
@@ -121,11 +120,6 @@ struct Version
                      ": " + result.error());
       }
 
-      if (hasLeadingZero(numericComponents[i])) {
-        return Error("Invalid version component '" + numericComponents[i] + "'"
-                     ": cannot contain leading zero");
-      }
-
       versionNumbers[i] = result.get();
     }
 
@@ -153,11 +147,11 @@ struct Version
         // valid prerelease and build identifiers.
 
         foreach (const std::string& identifier, prerelease) {
-          CHECK_NONE(validateIdentifier(identifier, true));
+          CHECK_NONE(validateIdentifier(identifier));
         }
 
         foreach (const std::string& identifier, build) {
-          CHECK_NONE(validateIdentifier(identifier, false));
+          CHECK_NONE(validateIdentifier(identifier));
         }
       }
 
@@ -290,10 +284,9 @@ struct Version
 private:
   // Check that a string contains a valid identifier. An identifier is
   // a non-empty string; each character must be an ASCII alphanumeric
-  // or hyphen.
-  static Option<Error> validateIdentifier(
-      const std::string& identifier,
-      bool rejectLeadingZero)
+  // or hyphen. We allow leading zeros in numeric identifiers, which
+  // inconsistent with the SemVer spec.
+  static Option<Error> validateIdentifier(const std::string& identifier)
   {
     if (identifier.empty()) {
       return Error("Empty identifier");
@@ -311,23 +304,13 @@ private:
                    "'" + stringify(*firstInvalid) + "'");
     }
 
-    // If requested, disallow identifiers that contain a leading
-    // zero. Note that this only applies to numeric identifiers, and
-    // that zero-valued identifiers are allowed.
-    if (rejectLeadingZero && hasLeadingZero(identifier)) {
-      return Error("Identifier contains leading zero");
-    }
-
     return None();
   }
 
   // Parse a string containing a series of dot-separated identifiers
   // into a vector of strings; each element of the vector contains a
-  // single identifier. If `rejectLeadingZeros` is true, we reject any
-  // numeric identifier in the label that contains a leading zero.
-  static Try<std::vector<std::string>> parseLabel(
-      const std::string& label,
-      bool rejectLeadingZeros)
+  // single identifier.
+  static Try<std::vector<std::string>> parseLabel(const std::string& label)
   {
     if (label.empty()) {
       return Error("Empty label");
@@ -336,7 +319,7 @@ private:
     std::vector<std::string> identifiers = strings::split(label, ".");
 
     foreach (const std::string& identifier, identifiers) {
-      Option<Error> error = validateIdentifier(identifier, rejectLeadingZeros);
+      Option<Error> error = validateIdentifier(identifier);
       if (error.isSome()) {
         return error.get();
       }
@@ -345,16 +328,6 @@ private:
     return identifiers;
   }
 
-  // Checks whether the input string is numeric and contains a leading
-  // zero. Note that "0" by itself is not considered a "leading zero".
-  static bool hasLeadingZero(const std::string& identifier) {
-    Try<uint32_t> numericIdentifier = parseNumericIdentifier(identifier);
-
-    return numericIdentifier.isSome() &&
-      numericIdentifier.get() != 0 &&
-      strings::startsWith(identifier, '0');
-  }
-
   // Try to parse the given string as a numeric identifier. According
   // to the SemVer spec, identifiers that begin with hyphens are
   // considered non-numeric.

http://git-wip-us.apache.org/repos/asf/mesos/blob/d43d7453/3rdparty/stout/tests/version_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/version_tests.cpp b/3rdparty/stout/tests/version_tests.cpp
index bce185e..a532d0c 100644
--- a/3rdparty/stout/tests/version_tests.cpp
+++ b/3rdparty/stout/tests/version_tests.cpp
@@ -124,7 +124,13 @@ TEST(VersionTest, ParseValid)
     {"1-2-3.4+5.6-7",
      {Version(1, 0, 0, {"2-3", "4"}, {"5", "6-7"}), "1.0.0-2-3.4+5.6-7"}},
     {"1-2.-3+4.-5",
-     {Version(1, 0, 0, {"2", "-3"}, {"4", "-5"}), "1.0.0-2.-3+4.-5"}}
+     {Version(1, 0, 0, {"2", "-3"}, {"4", "-5"}), "1.0.0-2.-3+4.-5"}},
+    // Allow leading zeros: in violation of the SemVer spec, but we
+    // tolerate it for compatibility with common practice (e.g., Docker).
+    {"01.2.3", {Version(1, 2, 3), "1.2.3"}},
+    {"1.02.3", {Version(1, 2, 3), "1.2.3"}},
+    {"1.2.03", {Version(1, 2, 3), "1.2.3"}},
+    {"1.2.3-alpha.001", {Version(1, 2, 3, {"alpha", "001"}), "1.2.3-alpha.001"}}
   };
 
   foreachpair (const string& input, const ExpectedValue& expected, testCases) {
@@ -152,11 +158,8 @@ TEST(VersionTest, ParseInvalid)
     ".1.2",
     "0.1.-2",
     "0.-1.2",
-    "0.1.2.3",
+    "1.2.3.4",
     "-1.1.2",
-    "01.2.3",
-    "1.02.3",
-    "1.2.03",
     "1.1.2-",
     "1.1.2+",
     "1.1.2-+",
@@ -169,7 +172,6 @@ TEST(VersionTest, ParseInvalid)
     "1.1.2+.foo",
     "1.1.2-al^pha",
     "1.1.2+exp;",
-    "1.1.2-alpha.001",
     "-foo",
     "+foo",
     u8"1.0.0-b\u00e9ta"


Mime
View raw message