From 58fcef6d3304ee9089a121dbe649141a9594342c Mon Sep 17 00:00:00 2001 From: James Teh Date: Tue, 13 Feb 2018 16:48:16 +1000 Subject: [PATCH] Bug 1437417 part 1: a11y: Fix some issues in IsModuleVersionLessThan and make it reusable. r=MarcoZ 1. Move IsModuleVersionLessThan into the Compatibility class and export it in the header file. 2. The function previously referred to the third component of the version as the minor version; i.e. it was testing major.bbbb.minor.dddd. This is incorrect and might confuse people using this in future code. The minor version is the second component; i.e. major.minor.cccc.dddd. cccc and dddd are often named build and revision, but the naming here is less consistent. 3. Rather than accepting separate version components, the function now accepts a single 64 bit value. This makes comparison easier and also allows for comparison against magic values in other code; e.g. a value meaning "all versions". This value can be created from separate components using the MAKE_FILE_VERSION macro. 4. Previously, it was assumed that a dll path could not be longer than MAX_PATH, but it can actually be longer. The function now handles this. 5. The function previously didn't do any error checking, which could have led to null pointer dereferences and possibly other pain. This was fine when it was only being used for JAWS, which we know always has version info, but this could be problematic for other callers. We return true if there is a failure, assuming that no version info implies an earlier version. 6. The code now uses smart pointers instead of raw pointers, making memory management simpler. 7. Updated the JAWS version check accordingly. MozReview-Commit-ID: 9Y6gUQSX0P5 --HG-- extra : rebase_source : 595408140d8611d38fef1211ef41c53e4b65e90c --- accessible/windows/msaa/Compatibility.cpp | 65 +++++++++++++++-------- accessible/windows/msaa/Compatibility.h | 16 ++++++ 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/accessible/windows/msaa/Compatibility.cpp b/accessible/windows/msaa/Compatibility.cpp index 3f5145816515..00556ed5f069 100644 --- a/accessible/windows/msaa/Compatibility.cpp +++ b/accessible/windows/msaa/Compatibility.cpp @@ -40,32 +40,53 @@ static const wchar_t* ConsumerStringMap[CONSUMERS_ENUM_LEN+1] = { L"\0" }; -/** - * Return true if module version is lesser than the given version. - */ bool -IsModuleVersionLessThan(HMODULE aModuleHandle, DWORD aMajor, DWORD aMinor) +Compatibility::IsModuleVersionLessThan(HMODULE aModuleHandle, + unsigned long long aVersion) { - wchar_t fileName[MAX_PATH]; - ::GetModuleFileNameW(aModuleHandle, fileName, MAX_PATH); + // Get the full path to the dll. + // We start with MAX_PATH, but the path can actually be longer. + DWORD fnSize = MAX_PATH; + UniquePtr fileName; + while (true) { + fileName = MakeUnique(fnSize); + DWORD retLen = ::GetModuleFileNameW(aModuleHandle, fileName.get(), fnSize); + MOZ_ASSERT(retLen != 0); + if (retLen == 0) { + return true; + } + if (retLen == fnSize && + ::GetLastError() == ERROR_INSUFFICIENT_BUFFER) { + // The buffer was too short. Increase the size and try again. + fnSize *= 2; + } + break; // Success! + } - DWORD dummy = 0; - DWORD length = ::GetFileVersionInfoSizeW(fileName, &dummy); + // Get the version info from the file. + DWORD length = ::GetFileVersionInfoSizeW(fileName.get(), nullptr); + if (length == 0) { + return true; + } - LPBYTE versionInfo = new BYTE[length]; - ::GetFileVersionInfoW(fileName, 0, length, versionInfo); + auto versionInfo = MakeUnique(length); + if (!::GetFileVersionInfoW(fileName.get(), 0, length, versionInfo.get())) { + return true; + } UINT uLen; VS_FIXEDFILEINFO* fixedFileInfo = nullptr; - ::VerQueryValueW(versionInfo, L"\\", (LPVOID*)&fixedFileInfo, &uLen); - DWORD dwFileVersionMS = fixedFileInfo->dwFileVersionMS; - DWORD dwFileVersionLS = fixedFileInfo->dwFileVersionLS; - delete [] versionInfo; + if (!::VerQueryValueW(versionInfo.get(), L"\\", (LPVOID*)&fixedFileInfo, + &uLen)) { + return true; + } - DWORD dwLeftMost = HIWORD(dwFileVersionMS); - DWORD dwSecondRight = HIWORD(dwFileVersionLS); - return (dwLeftMost < aMajor || - (dwLeftMost == aMajor && dwSecondRight < aMinor)); + // Combine into a 64 bit value for comparison. + unsigned long long version = + ((unsigned long long)fixedFileInfo->dwFileVersionMS) << 32 | + ((unsigned long long)fixedFileInfo->dwFileVersionLS); + + return version < aVersion; } @@ -166,9 +187,11 @@ uint32_t Compatibility::sConsumers = Compatibility::UNKNOWN; Compatibility::InitConsumers() { HMODULE jawsHandle = ::GetModuleHandleW(L"jhook"); - if (jawsHandle) - sConsumers |= (IsModuleVersionLessThan(jawsHandle, 19, 0)) ? - OLDJAWS : JAWS; + if (jawsHandle) { + sConsumers |= + IsModuleVersionLessThan(jawsHandle, MAKE_FILE_VERSION(19, 0, 0, 0)) ? + OLDJAWS : JAWS; + } if (::GetModuleHandleW(L"gwm32inc")) sConsumers |= WE; diff --git a/accessible/windows/msaa/Compatibility.h b/accessible/windows/msaa/Compatibility.h index eec32f088c20..c9ccab040e4c 100644 --- a/accessible/windows/msaa/Compatibility.h +++ b/accessible/windows/msaa/Compatibility.h @@ -67,6 +67,16 @@ public: */ static bool HasKnownNonUiaConsumer(); + /** + * Return true if a module's version is lesser than the given version. + * Generally, the version should be provided using the MAKE_FILE_VERSION + * macro. + * If the version information cannot be retrieved, true is returned; i.e. + * no version information implies an earlier version. + */ + static bool IsModuleVersionLessThan(HMODULE aModuleHandle, + unsigned long long aVersion); + private: Compatibility(); Compatibility(const Compatibility&); @@ -101,4 +111,10 @@ private: } // a11y namespace } // mozilla namespace +// Convert the 4 (decimal) components of a DLL version number into a +// single unsigned long long, as needed by +// mozilla::a11y::Compatibility::IsModuleVersionLessThan. +#define MAKE_FILE_VERSION(a,b,c,d)\ + ((a##ULL << 48) + (b##ULL << 32) + (c##ULL << 16) + d##ULL) + #endif