Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 499E2200C7F for ; Wed, 10 May 2017 06:58:28 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 45B94160BC3; Wed, 10 May 2017 04:58:28 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 3276B160BCE for ; Wed, 10 May 2017 06:58:27 +0200 (CEST) Received: (qmail 11123 invoked by uid 500); 10 May 2017 04:58:26 -0000 Mailing-List: contact commits-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@mesos.apache.org Delivered-To: mailing list commits@mesos.apache.org Received: (qmail 10678 invoked by uid 99); 10 May 2017 04:58:26 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 10 May 2017 04:58:26 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id CCD77E97EC; Wed, 10 May 2017 04:58:25 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: neilc@apache.org To: commits@mesos.apache.org Date: Wed, 10 May 2017 04:58:30 -0000 Message-Id: In-Reply-To: <8e7b721d966f4abbb90c8cc587e40c87@git.apache.org> References: <8e7b721d966f4abbb90c8cc587e40c87@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [6/8] mesos git commit: Allowed leading zeros in input to stout's Version parser. archived-at: Wed, 10 May 2017 04:58:28 -0000 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 Authored: Tue May 9 17:50:11 2017 -0700 Committer: Neil Conway 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 // [.[.]][-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> parsed = parseLabel(buildString, false); + Try> 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> parsed = parseLabel(prereleaseString, true); + Try> 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 validateIdentifier( - const std::string& identifier, - bool rejectLeadingZero) + // or hyphen. We allow leading zeros in numeric identifiers, which + // inconsistent with the SemVer spec. + static Option 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> parseLabel( - const std::string& label, - bool rejectLeadingZeros) + // single identifier. + static Try> parseLabel(const std::string& label) { if (label.empty()) { return Error("Empty label"); @@ -336,7 +319,7 @@ private: std::vector identifiers = strings::split(label, "."); foreach (const std::string& identifier, identifiers) { - Option error = validateIdentifier(identifier, rejectLeadingZeros); + Option 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 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"