Bug 1588113: Only consider the major version part when checking for downgrades. r=glandium

Differential Revision: https://phabricator.services.mozilla.com/D70245
This commit is contained in:
Dave Townsend 2020-04-16 19:12:50 +00:00
parent ab80e24ca6
commit 97ea0f945d
2 changed files with 36 additions and 228 deletions

View File

@ -2420,8 +2420,7 @@ static ReturnAbortOnError CheckDowngrade(nsIFile* aProfileDir,
*/
static void ExtractCompatVersionInfo(const nsACString& aCompatVersion,
nsACString& aAppVersion,
nsACString& aAppBuildID,
nsACString& aPlatformBuildID) {
nsACString& aAppBuildID) {
int32_t underscorePos = aCompatVersion.FindChar('_');
int32_t slashPos = aCompatVersion.FindChar('/');
@ -2433,78 +2432,12 @@ static void ExtractCompatVersionInfo(const nsACString& aCompatVersion,
// Fall back to just using the entire string as the version.
aAppVersion = aCompatVersion;
aAppBuildID.Truncate(0);
aPlatformBuildID.Truncate(0);
return;
}
aAppVersion = Substring(aCompatVersion, 0, underscorePos);
aAppBuildID = Substring(aCompatVersion, underscorePos + 1,
slashPos - (underscorePos + 1));
aPlatformBuildID = Substring(aCompatVersion, slashPos + 1);
}
/**
* Compares two build IDs. Returns 0 if they match, < 0 if newID is considered
* newer than oldID and > 0 if the oldID is considered newer than newID.
*/
static int32_t CompareBuildIDs(nsACString& oldID, nsACString& newID) {
// For Mozilla builds the build ID is a numeric date string. But it is too
// large a number for the version comparator to handle so try to just compare
// them as integer values first.
// ToInteger64 succeeds if the strings contain trailing non-digits so first
// check that all the characters are digits.
bool isNumeric = true;
const char* pos = oldID.BeginReading();
const char* end = oldID.EndReading();
while (pos != end) {
if (!IsAsciiDigit(*pos)) {
isNumeric = false;
break;
}
pos++;
}
if (isNumeric) {
pos = newID.BeginReading();
end = newID.EndReading();
while (pos != end) {
if (!IsAsciiDigit(*pos)) {
isNumeric = false;
break;
}
pos++;
}
}
if (isNumeric) {
nsresult rv;
CheckedInt<uint64_t> oldVal = oldID.ToInteger64(&rv);
if (NS_SUCCEEDED(rv) && oldVal.isValid()) {
CheckedInt<uint64_t> newVal = newID.ToInteger64(&rv);
if (NS_SUCCEEDED(rv) && newVal.isValid()) {
// We have simple numbers for both IDs.
if (oldVal.value() == newVal.value()) {
return 0;
}
if (oldVal.value() > newVal.value()) {
return 1;
}
return -1;
}
}
}
// If either could not be parsed as a number then something (likely a Linux
// distribution could have modified the build ID in some way. We don't know
// what format this may be so let's just fall back to assuming that it's a
// valid toolkit version.
return CompareVersions(PromiseFlatCString(oldID).get(),
PromiseFlatCString(newID).get());
}
/**
@ -2514,50 +2447,29 @@ static int32_t CompareBuildIDs(nsACString& oldID, nsACString& newID) {
*/
int32_t CompareCompatVersions(const nsACString& aOldCompatVersion,
const nsACString& aNewCompatVersion) {
// Quick path for the common case.
// The simple case,
if (aOldCompatVersion.Equals(aNewCompatVersion)) {
gLastAppVersion.Assign(gAppData->version);
gLastAppBuildID.Assign(gAppData->buildID);
return 0;
}
// The versions differ for some reason so we will only ever return false from
// here onwards. We just have to figure out if this is a downgrade or not.
// Hardcode the case where the last run was in safe mode (Bug 1556612). We
// cannot tell if this is a downgrade or not so just assume it isn't and let
// the user proceed.
if (aOldCompatVersion.EqualsLiteral("Safe Mode")) {
gLastAppVersion.SetIsVoid(true);
gLastAppBuildID.SetIsVoid(true);
return -1;
}
nsCString oldPlatformBuildID;
ExtractCompatVersionInfo(aOldCompatVersion, gLastAppVersion, gLastAppBuildID,
oldPlatformBuildID);
// Extract the major version part from the version string and only use that
// for version comparison.
int32_t index = aOldCompatVersion.FindChar('.');
const nsACString& oldMajorVersion = Substring(
aOldCompatVersion, 0, index < 0 ? aOldCompatVersion.Length() : index);
index = aNewCompatVersion.FindChar('.');
const nsACString& newMajorVersion = Substring(
aNewCompatVersion, 0, index < 0 ? aNewCompatVersion.Length() : index);
nsCString newVersion;
nsCString newAppBuildID;
nsCString newPlatformBuildID;
ExtractCompatVersionInfo(aNewCompatVersion, newVersion, newAppBuildID,
newPlatformBuildID);
// In most cases the app version will differ and this is an easy check.
int32_t result = CompareVersions(gLastAppVersion.get(), newVersion.get());
if (result != 0) {
return result;
}
// Fall back to build ID comparison.
result = CompareBuildIDs(gLastAppBuildID, newAppBuildID);
if (result != 0) {
return result;
}
return CompareBuildIDs(oldPlatformBuildID, newPlatformBuildID);
return CompareVersions(PromiseFlatCString(oldMajorVersion).get(),
PromiseFlatCString(newMajorVersion).get());
}
/**
@ -2593,12 +2505,18 @@ static bool CheckCompatibility(nsIFile* aProfileDir, const nsCString& aVersion,
return false;
}
int32_t result = CompareCompatVersions(aLastVersion, aVersion);
if (result != 0) {
*aIsDowngrade = result > 0;
int32_t comparison = CompareCompatVersions(aLastVersion, aVersion);
if (comparison != 0) {
*aIsDowngrade = comparison > 0;
ExtractCompatVersionInfo(aLastVersion, gLastAppVersion, gLastAppBuildID);
return false;
}
gLastAppVersion.Assign(gAppData->version);
gLastAppBuildID.Assign(gAppData->buildID);
*aIsDowngrade = false;
nsAutoCString buf;
rv = parser.GetString("Compatibility", "LastOSABI", buf);
if (NS_FAILED(rv) || !aOSABI.Equals(buf)) return false;

View File

@ -21,144 +21,34 @@ void CheckCompatVersionCompare(const nsCString& aOldCompatVersion,
<< "Version downgrade check should match.";
}
void CheckExpectedResult(const char* aOldAppVersion, const char* aOldAppID,
const char* aOldToolkitID, const char* aNewAppVersion,
const char* aNewAppID, const char* aNewToolkitID,
void CheckExpectedResult(const char* aOldAppVersion, const char* aNewAppVersion,
bool aExpectedSame, bool aExpectedDowngrade) {
nsCString oldCompatVersion;
BuildCompatVersion(aOldAppVersion, aOldAppID, aOldToolkitID,
oldCompatVersion);
BuildCompatVersion(aOldAppVersion, "", "", oldCompatVersion);
nsCString newCompatVersion;
BuildCompatVersion(aNewAppVersion, aNewAppID, aNewToolkitID,
newCompatVersion);
BuildCompatVersion(aNewAppVersion, "", "", newCompatVersion);
CheckCompatVersionCompare(oldCompatVersion, newCompatVersion, aExpectedSame,
aExpectedDowngrade);
}
// clang-format off
TEST(CompatVersionCompare, CompareVersionChange) {
TEST(CompatVersionCompare, CompareVersionChange)
{
// Identical
CheckExpectedResult(
"67.0", "20000000000000", "20000000000000",
"67.0", "20000000000000", "20000000000000",
true, false);
// Build ID changes
CheckExpectedResult(
"67.0", "20000000000000", "20000000000001",
"67.0", "20000000000000", "20000000000000",
false, true);
CheckExpectedResult(
"67.0", "20000000000001", "20000000000000",
"67.0", "20000000000000", "20000000000000",
false, true);
CheckExpectedResult(
"67.0", "20000000000000", "20000000000000",
"67.0", "20000000000000", "20000000000001",
false, false);
CheckExpectedResult(
"67.0", "20000000000000", "20000000000000",
"67.0", "20000000000001", "20000000000000",
false, false);
CheckExpectedResult("67.0", "67.0", true, false);
// Version changes
CheckExpectedResult(
"67.0", "20000000000000", "20000000000000",
"68.0", "20000000000000", "20000000000000",
false, false);
CheckExpectedResult(
"68.0", "20000000000000", "20000000000000",
"67.0", "20000000000000", "20000000000000",
false, true);
CheckExpectedResult(
"67.0", "20000000000000", "20000000000000",
"67.0.1", "20000000000000", "20000000000000",
false, false);
CheckExpectedResult(
"67.0.1", "20000000000000", "20000000000000",
"67.0", "20000000000000", "20000000000000",
false, true);
CheckExpectedResult(
"67.0.1", "20000000000000", "20000000000000",
"67.0.1", "20000000000000", "20000000000000",
true, false);
CheckExpectedResult(
"67.0.1", "20000000000000", "20000000000000",
"67.0.2", "20000000000000", "20000000000000",
false, false);
CheckExpectedResult(
"67.0.2", "20000000000000", "20000000000000",
"67.0.1", "20000000000000", "20000000000000",
false, true);
// Unexpected ID formats
CheckExpectedResult(
"67.0.1", "build1", "build1",
"67.0.1", "build2", "build2",
false, false);
CheckExpectedResult(
"67.0.1", "build10", "build10",
"67.0.1", "build2", "build2",
false, true);
CheckExpectedResult(
"67.0.1", "1", "1",
"67.0.1", "10", "10",
false, false);
CheckExpectedResult(
"67.0.1", "10", "10",
"67.0.1", "1", "1",
false, true);
#if !defined(XP_WIN) || !defined(MOZ_ASAN)
// These tests rely on the `errno` behavior in `ns_strtol`, which is broken
// by ASan interceptors on Windows. (https://llvm.org/pr35137)
// These support an upgrade case from a normal-style build ID to the one
// we're suggesting Ubuntu use.
CheckExpectedResult(
"67.0.1", "20000000000000", "20000000000000",
"67.0.1", "1build1", "1build1",
false, false);
CheckExpectedResult(
"67.0.1", "1build1", "1build1",
"67.0.1", "20000000000000", "20000000000000",
false, true);
#endif
// The actual case from bug 1554029:
CheckExpectedResult(
"67.0", "20190516215225", "20190516215225",
"67.0.5", "20190523030228","20190523030228",
false, false);
CheckExpectedResult(
"67.0.5", "20190523030228", "20190523030228",
"67.0", "20190516215225", "20190516215225",
false, true);
// A newer or equal version should not go on to test the build IDs (bug 1556612).
CheckExpectedResult(
"65.0", "30000000000000", "20000000000000",
"66.0", "20000000000000", "20000000000000",
false, false);
CheckExpectedResult(
"65.0", "20000000000000", "30000000000000",
"66.0", "20000000000000", "20000000000000",
false, false);
CheckExpectedResult(
"66.0", "30000000000000", "20000000000000",
"65.0", "20000000000000", "20000000000000",
false, true);
CheckExpectedResult(
"66.0", "20000000000000", "30000000000000",
"65.0", "20000000000000", "20000000000000",
false, true);
CheckExpectedResult("67.0", "68.0", false, false);
CheckExpectedResult("68.0", "67.0", false, true);
CheckExpectedResult("67.0", "67.0.1", true, false);
CheckExpectedResult("67.0.1", "67.0", true, false);
CheckExpectedResult("67.0.1", "67.0.1", true, false);
CheckExpectedResult("67.0.1", "67.0.2", true, false);
CheckExpectedResult("67.0.2", "67.0.1", true, false);
// Check that if the last run was safe mode then we consider this an upgrade.
CheckCompatVersionCompare(
NS_LITERAL_CSTRING("Safe Mode"),
NS_LITERAL_CSTRING("67.0.1_20000000000000/20000000000000"),
false, false);
NS_LITERAL_CSTRING("Safe Mode"),
NS_LITERAL_CSTRING("67.0.1_20000000000000/20000000000000"), false, false);
}
// clang-format on