From e6ae5735713f172eff39f9b8bc75b478b50aee7f Mon Sep 17 00:00:00 2001 From: Spencer Low Date: Tue, 8 Sep 2015 17:13:04 -0700 Subject: [PATCH] adb: win32: make adb_getenv() case-insensitive adb_getenv() should be case-insensitive just like the real getenv() on Windows. Added a unittest for adb_getenv(). In the process, made adb_test link with -municode so that the environment block is Unicode. Move wmain() from main.cpp to sysdeps_win32.cpp so that adb_test could also use it. Because wmain() moved, it wasn't as easy to do the runtime check to verify that -municode was used, so do that check in _ensure_env_setup() since adb_getenv() is called early in adb anyway. Added a utility ToLower() which is good enough for env vars whose keys are probably always ASCII to begin with. Change-Id: I082f7fdee9dfe2c7f76b878528d2f7863df6d8d1 Signed-off-by: Spencer Low --- adb/Android.mk | 6 ++++ adb/client/main.cpp | 26 -------------- adb/sysdeps_win32.cpp | 48 ++++++++++++++++++++++---- adb/sysdeps_win32_test.cpp | 69 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 32 deletions(-) create mode 100755 adb/sysdeps_win32_test.cpp diff --git a/adb/Android.mk b/adb/Android.mk index 543f1eb2a..a64f48c77 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -78,6 +78,9 @@ LIBADB_TEST_linux_SRCS := \ LIBADB_TEST_darwin_SRCS := \ fdevent_test.cpp \ +LIBADB_TEST_windows_SRCS := \ + sysdeps_win32_test.cpp \ + include $(CLEAR_VARS) LOCAL_CLANG := true LOCAL_MODULE := libadbd @@ -153,6 +156,7 @@ LOCAL_SRC_FILES := \ LOCAL_SRC_FILES_linux := $(LIBADB_TEST_linux_SRCS) LOCAL_SRC_FILES_darwin := $(LIBADB_TEST_darwin_SRCS) +LOCAL_SRC_FILES_windows := $(LIBADB_TEST_windows_SRCS) LOCAL_SANITIZE := $(adb_host_sanitize) LOCAL_SHARED_LIBRARIES := libbase LOCAL_STATIC_LIBRARIES := \ @@ -160,6 +164,8 @@ LOCAL_STATIC_LIBRARIES := \ libcrypto_static \ libcutils \ +# Set entrypoint to wmain from sysdeps_win32.cpp instead of main +LOCAL_LDFLAGS_windows := -municode LOCAL_LDLIBS_linux := -lrt -ldl -lpthread LOCAL_LDLIBS_darwin := -framework CoreFoundation -framework IOKit LOCAL_LDLIBS_windows := -lws2_32 -luserenv diff --git a/adb/client/main.cpp b/adb/client/main.cpp index 6e27c0fb2..47234eee0 100644 --- a/adb/client/main.cpp +++ b/adb/client/main.cpp @@ -169,34 +169,8 @@ int adb_main(int is_daemon, int server_port, int ack_reply_fd) { return 0; } -#ifdef _WIN32 -static bool _argv_is_utf8 = false; -#endif - int main(int argc, char** argv) { -#ifdef _WIN32 - if (!_argv_is_utf8) { - fatal("_argv_is_utf8 is not set, suggesting that wmain was not " - "called. Did you forget to link with -municode?"); - } -#endif - adb_sysdeps_init(); adb_trace_init(argv); return adb_commandline(argc - 1, const_cast(argv + 1)); } - -#ifdef _WIN32 - -extern "C" -int wmain(int argc, wchar_t **argv) { - // Set diagnostic flag to try to detect if the build system was not - // configured to call wmain. - _argv_is_utf8 = true; - - // Convert args from UTF-16 to UTF-8 and pass that to main(). - NarrowArgs narrow_args(argc, argv); - return main(argc, narrow_args.data()); -} - -#endif diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index b53cd7722..c5aad0230 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -3776,6 +3777,29 @@ FILE* adb_fopen(const char* f, const char* m) { return _wfopen(widen(f).c_str(), widen(m).c_str()); } +// Return a lowercase version of the argument. Uses C Runtime tolower() on +// each byte which is not UTF-8 aware, and theoretically uses the current C +// Runtime locale (which in practice is not changed, so this becomes a ASCII +// conversion). +static std::string ToLower(const std::string& anycase) { + // copy string + std::string str(anycase); + // transform the copy + std::transform(str.begin(), str.end(), str.begin(), tolower); + return str; +} + +extern "C" int main(int argc, char** argv); + +// Link with -municode to cause this wmain() to be used as the program +// entrypoint. It will convert the args from UTF-16 to UTF-8 and call the +// regular main() with UTF-8 args. +extern "C" int wmain(int argc, wchar_t **argv) { + // Convert args from UTF-16 to UTF-8 and pass that to main(). + NarrowArgs narrow_args(argc, argv); + return main(argc, narrow_args.data()); +} + // Shadow UTF-8 environment variable name/value pairs that are created from // _wenviron the first time that adb_getenv() is called. Note that this is not // currently updated if putenv, setenv, unsetenv are called. Note that no @@ -3790,6 +3814,13 @@ static void _ensure_env_setup() { return; } + if (_wenviron == nullptr) { + // If _wenviron is null, then -municode probably wasn't used. That + // linker flag will cause the entry point to setup _wenviron. It will + // also require an implementation of wmain() (which we provide above). + fatal("_wenviron is not set, did you link with -municode?"); + } + // Read name/value pairs from UTF-16 _wenviron and write new name/value // pairs to UTF-8 g_environ_utf8. Note that it probably does not make sense // to use the D() macro here because that tracing only works if the @@ -3803,21 +3834,26 @@ static void _ensure_env_setup() { continue; } - const std::string name_utf8(narrow(std::wstring(*env, equal - *env))); + // Store lowercase name so that we can do case-insensitive searches. + const std::string name_utf8(ToLower(narrow( + std::wstring(*env, equal - *env)))); char* const value_utf8 = strdup(narrow(equal + 1).c_str()); - // Overwrite any duplicate name, but there shouldn't be a dup in the - // first place. - g_environ_utf8[name_utf8] = value_utf8; + // Don't overwrite a previus env var with the same name. In reality, + // the system probably won't let two env vars with the same name exist + // in _wenviron. + g_environ_utf8.insert({name_utf8, value_utf8}); } } // Version of getenv() that takes a UTF-8 environment variable name and -// retrieves a UTF-8 value. +// retrieves a UTF-8 value. Case-insensitive to match getenv() on Windows. char* adb_getenv(const char* name) { _ensure_env_setup(); - const auto it = g_environ_utf8.find(std::string(name)); + // Case-insensitive search by searching for lowercase name in a map of + // lowercase names. + const auto it = g_environ_utf8.find(ToLower(std::string(name))); if (it == g_environ_utf8.end()) { return nullptr; } diff --git a/adb/sysdeps_win32_test.cpp b/adb/sysdeps_win32_test.cpp new file mode 100755 index 000000000..cc3ac5c16 --- /dev/null +++ b/adb/sysdeps_win32_test.cpp @@ -0,0 +1,69 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "sysdeps.h" + +TEST(sysdeps_win32, adb_getenv) { + // Insert all test env vars before first call to adb_getenv() which will + // read the env var block only once. + ASSERT_EQ(0, _putenv("SYSDEPS_WIN32_TEST_UPPERCASE=1")); + ASSERT_EQ(0, _putenv("sysdeps_win32_test_lowercase=2")); + ASSERT_EQ(0, _putenv("Sysdeps_Win32_Test_MixedCase=3")); + + // UTF-16 value + ASSERT_EQ(0, _wputenv(L"SYSDEPS_WIN32_TEST_UNICODE=\u00a1\u0048\u006f\u006c" + L"\u0061\u0021\u03b1\u03b2\u03b3\u0061\u006d\u0062" + L"\u0075\u006c\u014d\u043f\u0440\u0438\u0432\u0435" + L"\u0442")); + + // Search for non-existant env vars. + EXPECT_STREQ(nullptr, adb_getenv("SYSDEPS_WIN32_TEST_NONEXISTANT")); + + // Search for existing env vars. + + // There is no test for an env var with a value of a zero-length string + // because _putenv() does not support inserting such an env var. + + // Search for env var that is uppercase. + EXPECT_STREQ("1", adb_getenv("SYSDEPS_WIN32_TEST_UPPERCASE")); + EXPECT_STREQ("1", adb_getenv("sysdeps_win32_test_uppercase")); + EXPECT_STREQ("1", adb_getenv("Sysdeps_Win32_Test_Uppercase")); + + // Search for env var that is lowercase. + EXPECT_STREQ("2", adb_getenv("SYSDEPS_WIN32_TEST_LOWERCASE")); + EXPECT_STREQ("2", adb_getenv("sysdeps_win32_test_lowercase")); + EXPECT_STREQ("2", adb_getenv("Sysdeps_Win32_Test_Lowercase")); + + // Search for env var that is mixed-case. + EXPECT_STREQ("3", adb_getenv("SYSDEPS_WIN32_TEST_MIXEDCASE")); + EXPECT_STREQ("3", adb_getenv("sysdeps_win32_test_mixedcase")); + EXPECT_STREQ("3", adb_getenv("Sysdeps_Win32_Test_MixedCase")); + + // Check that UTF-16 was converted to UTF-8. + EXPECT_STREQ("\xc2\xa1\x48\x6f\x6c\x61\x21\xce\xb1\xce\xb2\xce\xb3\x61\x6d" + "\x62\x75\x6c\xc5\x8d\xd0\xbf\xd1\x80\xd0\xb8\xd0\xb2\xd0\xb5" + "\xd1\x82", + adb_getenv("SYSDEPS_WIN32_TEST_UNICODE")); + + // Check an env var that should always be set. + const char* path_val = adb_getenv("PATH"); + EXPECT_NE(nullptr, path_val); + if (path_val != nullptr) { + EXPECT_GT(strlen(path_val), 0); + } +}