Fix uninitialized reads in DnsConfigServiceWin tests.

net/dns/dns_config_service_win_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/dns/dns_config_service_win.h"
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/win/windows_version.h"
 #include "net/dns/dns_protocol.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
 
 namespace {
 
+// Some simple structs without constructors to enable brace style
+// initialization in these tests.
+struct RegStringInit {
+  internal::DnsSystemSettings::RegString toRegString() const {
+    internal::DnsSystemSettings::RegString regString;
+    regString.set = set;
+    regString.value = value;
+    return regString;
+  }
+
+  bool set;
+  base::string16 value;
+};
+
+struct RegDwordInit {
+  internal::DnsSystemSettings::RegDword toRegDword() const {
+    internal::DnsSystemSettings::RegDword regDword;
+    regDword.set = set;
+    regDword.value = value;
+    return regDword;
+  }
+
+  bool set;
+  DWORD value;
+};
+
+struct DevolutionSettingInit {
+  internal::DnsSystemSettings::DevolutionSetting toDevolutionSetting() const {
+    internal::DnsSystemSettings::DevolutionSetting setting;
+    setting.enabled = enabled.toRegDword();
+    setting.level = level.toRegDword();
+    return setting;
+  }
+
+  RegDwordInit enabled;
+  RegDwordInit level;
+};
+
 TEST(DnsConfigServiceWinTest, ParseSearchList) {
   const struct TestCase {
     const wchar_t* input;
     const char* output[4];  // NULL-terminated, empty if expected false
   } cases[] = {
     { L"chromium.org", { "chromium.org", NULL } },
     { L"chromium.org,org", { "chromium.org", "org", NULL } },
     // Empty suffixes terminate the list
     { L"crbug.com,com,,org", { "crbug.com", "com", NULL } },
     // IDN are converted to punycode
@@ skipping 171 matching lines @@
 }
 
 TEST(DnsConfigServiceWinTest, ConvertSuffixSearch) {
   AdapterInfo infos[2] = {
     { IF_TYPE_USB, IfOperStatusUp, L"connection.suffix", { "1.0.0.1" } },
     { 0 },
   };
 
   const struct TestCase {
     struct {
-      internal::DnsSystemSettings::RegString policy_search_list;
-      internal::DnsSystemSettings::RegString tcpip_search_list;
-      internal::DnsSystemSettings::RegString tcpip_domain;
-      internal::DnsSystemSettings::RegString primary_dns_suffix;
-      internal::DnsSystemSettings::DevolutionSetting policy_devolution;
-      internal::DnsSystemSettings::DevolutionSetting dnscache_devolution;
-      internal::DnsSystemSettings::DevolutionSetting tcpip_devolution;
+      RegStringInit policy_search_list;
+      RegStringInit tcpip_search_list;
+      RegStringInit tcpip_domain;
+      RegStringInit primary_dns_suffix;
+      DevolutionSettingInit policy_devolution;
+      DevolutionSettingInit dnscache_devolution;
+      DevolutionSettingInit tcpip_devolution;
     } input_settings;
     std::string expected_search[5];
   } cases[] = {
     {  // Policy SearchList override.
       {
         { true, L"policy.searchlist.a,policy.searchlist.b" },
         { true, L"tcpip.searchlist.a,tcpip.searchlist.b" },
         { true, L"tcpip.domain" },
         { true, L"primary.dns.suffix" },

[Nico, 2015/05/11 04:08:29]

Huh, I thought that using a brace initializer for a struct zero-initializes all
fields after the listed ones to 0 (like `mystruct a = {};` sets all fields to
0). Is that not the case here? Why not?

If not, I'd just add a few `{ { false }, { false } }` lines in these blocks
rather than add all these automatic initializers.

[benwells, 2015/05/11 05:12:38]

The problem was elsewhere, with these changes fallout from the actual fix. See
next comment for details.

       },
       { "policy.searchlist.a", "policy.searchlist.b" },
     },
     {  // User-specified SearchList override.
       {
         { false },
         { true, L"tcpip.searchlist.a,tcpip.searchlist.b" },
         { true, L"tcpip.domain" },
         { true, L"primary.dns.suffix" },
       },
@@ skipping 128 matching lines @@
         { { false }, { true, 3 } },
         { { true, 0 }, { true, 3 } },
       },
       { "a.b.c.d.e", "connection.suffix" },
     },
   };
 
   for (auto& t : cases) {
     internal::DnsSystemSettings settings;
     settings.addresses = CreateAdapterAddresses(infos);
-    settings.policy_search_list = t.input_settings.policy_search_list;
-    settings.tcpip_search_list = t.input_settings.tcpip_search_list;
-    settings.tcpip_domain = t.input_settings.tcpip_domain;
-    settings.primary_dns_suffix = t.input_settings.primary_dns_suffix;
-    settings.policy_devolution = t.input_settings.policy_devolution;
-    settings.dnscache_devolution = t.input_settings.dnscache_devolution;
-    settings.tcpip_devolution = t.input_settings.tcpip_devolution;
+    settings.policy_search_list =
+        t.input_settings.policy_search_list.toRegString();
+    settings.tcpip_search_list =
+        t.input_settings.tcpip_search_list.toRegString();
+    settings.tcpip_domain = t.input_settings.tcpip_domain.toRegString();
+    settings.primary_dns_suffix =
+        t.input_settings.primary_dns_suffix.toRegString();
+    settings.policy_devolution =
+        t.input_settings.policy_devolution.toDevolutionSetting();
+    settings.dnscache_devolution =
+        t.input_settings.dnscache_devolution.toDevolutionSetting();
+    settings.tcpip_devolution =
+        t.input_settings.tcpip_devolution.toDevolutionSetting();
 
     DnsConfig config;
     EXPECT_EQ(internal::CONFIG_PARSE_WIN_OK,
               internal::ConvertSettingsToDnsConfig(settings, &config));
     std::vector<std::string> expected_search;
     for (size_t j = 0; !t.expected_search[j].empty(); ++j) {
       expected_search.push_back(t.expected_search[j]);
     }
     EXPECT_EQ(expected_search, config.search);
   }
 }
 
 TEST(DnsConfigServiceWinTest, AppendToMultiLabelName) {
   AdapterInfo infos[2] = {
     { IF_TYPE_USB, IfOperStatusUp, L"connection.suffix", { "1.0.0.1" } },
     { 0 },
   };
 
   // The default setting was true pre-Vista.
   bool default_value = (base::win::GetVersion() < base::win::VERSION_VISTA);
 
   const struct TestCase {
-    internal::DnsSystemSettings::RegDword input;
+    RegDwordInit input;
     bool expected_output;
   } cases[] = {
     { { true, 0 }, false },
     { { true, 1 }, true },
     { { false, 0 }, default_value },
   };
 
   for (const auto& t : cases) {
     internal::DnsSystemSettings settings;
     settings.addresses = CreateAdapterAddresses(infos);
-    settings.append_to_multi_label_name = t.input;
+    settings.append_to_multi_label_name = t.input.toRegDword();
     DnsConfig config;
     EXPECT_EQ(internal::CONFIG_PARSE_WIN_OK,
               internal::ConvertSettingsToDnsConfig(settings, &config));
     EXPECT_EQ(t.expected_output, config.append_to_multi_label_name);
   }
 }
 
 // Setting have_name_resolution_policy_table should set unhandled_options.
 TEST(DnsConfigServiceWinTest, HaveNRPT) {
   AdapterInfo infos[2] = {
     { IF_TYPE_USB, IfOperStatusUp, L"connection.suffix", { "1.0.0.1" } },
     { 0 },
   };
 
   const struct TestCase {
     bool have_nrpt;
     bool unhandled_options;
     internal::ConfigParseWinResult result;
   } cases[] = {
     { false, false, internal::CONFIG_PARSE_WIN_OK },
     { true, true, internal::CONFIG_PARSE_WIN_UNHANDLED_OPTIONS },
   };
 
   for (const auto& t : cases) {
     internal::DnsSystemSettings settings;
     settings.addresses = CreateAdapterAddresses(infos);
     settings.have_name_resolution_policy = t.have_nrpt;
     DnsConfig config;
     EXPECT_EQ(t.result,
               internal::ConvertSettingsToDnsConfig(settings, &config));

[benwells, 2015/05/11 05:12:38]

This was the line causing the error that I investigated. The problem is that
settings.policy_search_list.set is not initialized, because the new default
initializer for DnsSystemSettings initializes policy_search_list with
policy_search_list().

As RegString has no default values / no constructor this leaves set
uninitialized.

The actual fix for this was to add the "= false" to RegString and RegDword. The
rest of the change was required to make this test compile.

     EXPECT_EQ(t.unhandled_options, config.unhandled_options);
     EXPECT_EQ(t.have_nrpt, config.use_local_ipv6);
   }
 }
 
 
 }  // namespace
 
 }  // namespace net