diff options
Diffstat (limited to 'chromium/net/dns')
59 files changed, 1327 insertions, 886 deletions
diff --git a/chromium/net/dns/OWNERS b/chromium/net/dns/OWNERS index 6dc6cd3e522..d2b5e8663ca 100644 --- a/chromium/net/dns/OWNERS +++ b/chromium/net/dns/OWNERS @@ -1 +1,2 @@ ericorth@chromium.org +dmcardle@chromium.org diff --git a/chromium/net/dns/address_info.h b/chromium/net/dns/address_info.h index 23d68f50d9c..f7eba2402dd 100644 --- a/chromium/net/dns/address_info.h +++ b/chromium/net/dns/address_info.h @@ -9,6 +9,7 @@ #include <string> #include <tuple> +#include "base/memory/raw_ptr.h" #include "build/build_config.h" #include "net/base/address_family.h" #include "net/base/net_export.h" @@ -45,7 +46,7 @@ class NET_EXPORT_PRIVATE AddressInfo { private: // Owned by AddressInfo. - const addrinfo* ai_; + raw_ptr<const addrinfo> ai_; }; // Constructors diff --git a/chromium/net/dns/address_sorter_posix_unittest.cc b/chromium/net/dns/address_sorter_posix_unittest.cc index 9a4a3198f74..6bd613d5689 100644 --- a/chromium/net/dns/address_sorter_posix_unittest.cc +++ b/chromium/net/dns/address_sorter_posix_unittest.cc @@ -43,7 +43,7 @@ IPAddress ParseIP(const std::string& str) { class TestUDPClientSocket : public DatagramClientSocket { public: explicit TestUDPClientSocket(const AddressMapping* mapping) - : mapping_(mapping), connected_(false) {} + : mapping_(mapping) {} TestUDPClientSocket(const TestUDPClientSocket&) = delete; TestUDPClientSocket& operator=(const TestUDPClientSocket&) = delete; @@ -139,7 +139,7 @@ class TestUDPClientSocket : public DatagramClientSocket { private: NetLogWithSource net_log_; raw_ptr<const AddressMapping> mapping_; - bool connected_; + bool connected_ = false; IPEndPoint local_endpoint_; }; diff --git a/chromium/net/dns/address_sorter_win.cc b/chromium/net/dns/address_sorter_win.cc index 5220047d472..1f3fdc7fa28 100644 --- a/chromium/net/dns/address_sorter_win.cc +++ b/chromium/net/dns/address_sorter_win.cc @@ -71,8 +71,7 @@ class AddressSorterWin : public AddressSorter { input_buffer_( reinterpret_cast<SOCKET_ADDRESS_LIST*>(malloc(buffer_size_))), output_buffer_( - reinterpret_cast<SOCKET_ADDRESS_LIST*>(malloc(buffer_size_))), - success_(false) { + reinterpret_cast<SOCKET_ADDRESS_LIST*>(malloc(buffer_size_))) { input_buffer_->iAddressCount = base::checked_cast<INT>(endpoints.size()); SOCKADDR_STORAGE* storage = reinterpret_cast<SOCKADDR_STORAGE*>( input_buffer_->Address + input_buffer_->iAddressCount); @@ -140,7 +139,7 @@ class AddressSorterWin : public AddressSorter { const DWORD buffer_size_; std::unique_ptr<SOCKET_ADDRESS_LIST, base::FreeDeleter> input_buffer_; std::unique_ptr<SOCKET_ADDRESS_LIST, base::FreeDeleter> output_buffer_; - bool success_; + bool success_ = false; }; }; diff --git a/chromium/net/dns/context_host_resolver.cc b/chromium/net/dns/context_host_resolver.cc index c29c19e8314..b398e85b8f2 100644 --- a/chromium/net/dns/context_host_resolver.cc +++ b/chromium/net/dns/context_host_resolver.cc @@ -130,7 +130,6 @@ void ContextHostResolver::SetRequestContext( DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); resolve_context_->set_url_request_context(request_context); - manager_->RemoveResolveContextRegistrationIfNeeded(resolve_context_.get()); } HostResolverManager* ContextHostResolver::GetManagerForTesting() { @@ -141,6 +140,12 @@ const URLRequestContext* ContextHostResolver::GetContextForTesting() const { return resolve_context_ ? resolve_context_->url_request_context() : nullptr; } +NetworkChangeNotifier::NetworkHandle +ContextHostResolver::GetTargetNetworkForTesting() const { + return resolve_context_ ? resolve_context_->GetTargetNetwork() + : NetworkChangeNotifier::kInvalidNetworkHandle; +} + size_t ContextHostResolver::LastRestoredCacheSize() const { return resolve_context_->host_cache() ? resolve_context_->host_cache()->last_restore_size() diff --git a/chromium/net/dns/context_host_resolver.h b/chromium/net/dns/context_host_resolver.h index db881697e22..40f52618729 100644 --- a/chromium/net/dns/context_host_resolver.h +++ b/chromium/net/dns/context_host_resolver.h @@ -72,6 +72,8 @@ class NET_EXPORT ContextHostResolver : public HostResolver { void SetRequestContext(URLRequestContext* request_context) override; HostResolverManager* GetManagerForTesting() override; const URLRequestContext* GetContextForTesting() const override; + NetworkChangeNotifier::NetworkHandle GetTargetNetworkForTesting() + const override; // Returns the number of host cache entries that were restored, or 0 if there // is no cache. @@ -81,6 +83,9 @@ class NET_EXPORT ContextHostResolver : public HostResolver { void SetProcParamsForTesting(const ProcTaskParams& proc_params); void SetTickClockForTesting(const base::TickClock* tick_clock); + ResolveContext* resolve_context_for_testing() { + return resolve_context_.get(); + } private: const raw_ptr<HostResolverManager> manager_; diff --git a/chromium/net/dns/context_host_resolver_unittest.cc b/chromium/net/dns/context_host_resolver_unittest.cc index 8e80732d621..74a742359ee 100644 --- a/chromium/net/dns/context_host_resolver_unittest.cc +++ b/chromium/net/dns/context_host_resolver_unittest.cc @@ -48,6 +48,11 @@ #include "url/gurl.h" #include "url/scheme_host_port.h" +#if BUILDFLAG(IS_ANDROID) +#include "base/android/build_info.h" +#include "net/android/network_change_notifier_factory_android.h" +#endif // BUILDFLAG(IS_ANDROID) + namespace net { namespace { @@ -916,25 +921,44 @@ TEST_F(ContextHostResolverTest, NotExistingNetworkBoundLookup) { } // Test that the underlying HostCache does not receive invalidations when its -// ResolveContext is bound to a network. -TEST_F(ContextHostResolverTest, BoundResolveContextHostCacheInvalidation) { +// ResolveContext/HostResolverManager is bound to a network. +TEST_F(ContextHostResolverTest, NetworkBoundResolverCacheInvalidation) { +#if BUILDFLAG(IS_ANDROID) + auto scoped_mock_network_change_notifier = + std::make_unique<test::ScopedMockNetworkChangeNotifier>(); + test::MockNetworkChangeNotifier* mock_ncn = + scoped_mock_network_change_notifier->mock_network_change_notifier(); + mock_ncn->ForceNetworkHandlesSupported(); + + // The actual network handle doesn't really matter, this test just wants to + // check that all the pieces are in place and configured correctly. + constexpr NetworkChangeNotifier::NetworkHandle network = 2; + manager_ = HostResolverManager::CreateNetworkBoundHostResolverManager( + HostResolver::ManagerOptions(), network, nullptr /* net_log */); + manager_->SetLastIPv6ProbeResultForTesting(true); // Set empty MockDnsClient rules to ensure DnsClient is mocked out. MockDnsClientRuleList rules; SetMockDnsRules(std::move(rules)); - auto context = CreateTestURLRequestContextBuilder()->Build(); auto resolve_context = std::make_unique<NetworkBoundResolveContext>( - context.get(), true /* enable_caching */, - 2 /* != kInvalidNetworkHandle */); + nullptr /* url_request_context */, true /* enable_caching */, network); ResolveContext* resolve_context_ptr = resolve_context.get(); auto resolver = std::make_unique<ContextHostResolver>( manager_.get(), std::move(resolve_context)); - // There should be no invalidations before and after the call to - // InvalidateCachesForTesting. - ASSERT_EQ(resolve_context_ptr->host_cache()->network_changes(), 0); - manager_->InvalidateCachesForTesting(); - EXPECT_EQ(resolve_context_ptr->host_cache()->network_changes(), 0); + // Network events should not trigger cache invalidations + auto network_changes_before_events = + resolve_context_ptr->host_cache()->network_changes(); + NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests(); + NetworkChangeNotifier::NotifyObserversOfConnectionTypeChangeForTests( + NetworkChangeNotifier::CONNECTION_NONE); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(network_changes_before_events, + resolve_context_ptr->host_cache()->network_changes()); +#else // !BUILDFLAG(IS_ANDROID) + GTEST_SKIP() + << "Network-bound HostResolverManagers are supported only on Android"; +#endif // BUILDFLAG(IS_ANDROID) } } // namespace net diff --git a/chromium/net/dns/dns_client.cc b/chromium/net/dns/dns_client.cc index a3855b15609..179f857d54e 100644 --- a/chromium/net/dns/dns_client.cc +++ b/chromium/net/dns/dns_client.cc @@ -15,6 +15,8 @@ #include "base/ranges/algorithm.h" #include "base/values.h" #include "net/base/address_list.h" +#include "net/base/ip_address.h" +#include "net/base/ip_endpoint.h" #include "net/dns/address_sorter.h" #include "net/dns/dns_session.h" #include "net/dns/dns_transaction.h" @@ -191,8 +193,13 @@ class DnsClientImpl : public DnsClient { }); if (it == servers.end()) return absl::nullopt; - // TODO(crbug.com/1200908): Read preset IPs from the server config. - return absl::nullopt; + std::vector<IPEndPoint> combined; + for (const IPAddressList& ips : it->endpoints()) { + for (const IPAddress& ip : ips) { + combined.emplace_back(ip, endpoint.port()); + } + } + return AddressList(std::move(combined)); } DnsTransactionFactory* GetTransactionFactory() override { diff --git a/chromium/net/dns/dns_client_unittest.cc b/chromium/net/dns/dns_client_unittest.cc index 30c8728683f..17bce032793 100644 --- a/chromium/net/dns/dns_client_unittest.cc +++ b/chromium/net/dns/dns_client_unittest.cc @@ -23,6 +23,7 @@ #include "net/url_request/url_request_test_util.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "url/scheme_host_port.h" namespace net { @@ -282,6 +283,35 @@ TEST_F(DnsClientTest, FallbackFromInsecureTransactionPreferred_Failures) { EXPECT_FALSE(client_->FallbackFromInsecureTransactionPreferred()); } +TEST_F(DnsClientTest, GetPresetAddrs) { + DnsConfig config; + config.doh_config = *net::DnsOverHttpsConfig::FromString(R"( + { + "servers": [{ + "template": "https://www.doh.com/", + "endpoints": [{ + "ips": ["4.3.2.1"] + }, { + "ips": ["4.3.2.2"] + }] + }] + } + )"); + client_->SetSystemConfig(config); + + EXPECT_FALSE(client_->GetPresetAddrs( + url::SchemeHostPort("https", "otherdomain.com", 443))); + EXPECT_FALSE( + client_->GetPresetAddrs(url::SchemeHostPort("http", "www.doh.com", 443))); + EXPECT_FALSE(client_->GetPresetAddrs( + url::SchemeHostPort("https", "www.doh.com", 9999))); + + AddressList expected({{{4, 3, 2, 1}, 443}, {{4, 3, 2, 2}, 443}}); + EXPECT_THAT( + client_->GetPresetAddrs(url::SchemeHostPort("https", "www.doh.com", 443)), + testing::Optional(expected)); +} + TEST_F(DnsClientTest, Override) { client_->SetSystemConfig(BasicValidConfig()); EXPECT_THAT(client_->GetEffectiveConfig(), diff --git a/chromium/net/dns/dns_config.cc b/chromium/net/dns/dns_config.cc index 3f05133a375..2a24a13bbc3 100644 --- a/chromium/net/dns/dns_config.cc +++ b/chromium/net/dns/dns_config.cc @@ -6,6 +6,7 @@ #include <utility> +#include "base/numerics/safe_conversions.h" #include "base/values.h" #include "net/dns/public/dns_over_https_config.h" @@ -20,18 +21,7 @@ DnsConfig::DnsConfig(const DnsConfig& other) = default; DnsConfig::DnsConfig(DnsConfig&& other) = default; DnsConfig::DnsConfig(std::vector<IPEndPoint> nameservers) - : nameservers(std::move(nameservers)), - dns_over_tls_active(false), - unhandled_options(false), - append_to_multi_label_name(true), - ndots(1), - fallback_period(kDnsDefaultFallbackPeriod), - attempts(2), - doh_attempts(1), - rotate(false), - use_local_ipv6(false), - secure_dns_mode(SecureDnsMode::kOff), - allow_dns_over_https_upgrade(false) {} + : nameservers(std::move(nameservers)) {} DnsConfig::~DnsConfig() = default; @@ -84,34 +74,34 @@ void DnsConfig::CopyIgnoreHosts(const DnsConfig& d) { } base::Value DnsConfig::ToValue() const { - base::Value dict(base::Value::Type::DICTIONARY); + base::Value::Dict dict; - base::Value list(base::Value::Type::LIST); + base::Value::List nameserver_list; for (const auto& nameserver : nameservers) - list.Append(nameserver.ToString()); - dict.SetKey("nameservers", std::move(list)); + nameserver_list.Append(nameserver.ToString()); + dict.Set("nameservers", std::move(nameserver_list)); - dict.SetBoolKey("dns_over_tls_active", dns_over_tls_active); - dict.SetStringKey("dns_over_tls_hostname", dns_over_tls_hostname); + dict.Set("dns_over_tls_active", dns_over_tls_active); + dict.Set("dns_over_tls_hostname", dns_over_tls_hostname); - list = base::Value(base::Value::Type::LIST); + base::Value::List suffix_list; for (const auto& suffix : search) - list.Append(suffix); - dict.SetKey("search", std::move(list)); - dict.SetBoolKey("unhandled_options", unhandled_options); - dict.SetBoolKey("append_to_multi_label_name", append_to_multi_label_name); - dict.SetIntKey("ndots", ndots); - dict.SetDoubleKey("timeout", fallback_period.InSecondsF()); - dict.SetIntKey("attempts", attempts); - dict.SetIntKey("doh_attempts", doh_attempts); - dict.SetBoolKey("rotate", rotate); - dict.SetBoolKey("use_local_ipv6", use_local_ipv6); - dict.SetIntKey("num_hosts", hosts.size()); - dict.SetKey("doh_config", doh_config.ToValue()); - dict.SetIntKey("secure_dns_mode", static_cast<int>(secure_dns_mode)); - dict.SetBoolKey("allow_dns_over_https_upgrade", allow_dns_over_https_upgrade); - - return dict; + suffix_list.Append(suffix); + dict.Set("search", std::move(suffix_list)); + dict.Set("unhandled_options", unhandled_options); + dict.Set("append_to_multi_label_name", append_to_multi_label_name); + dict.Set("ndots", ndots); + dict.Set("timeout", fallback_period.InSecondsF()); + dict.Set("attempts", attempts); + dict.Set("doh_attempts", doh_attempts); + dict.Set("rotate", rotate); + dict.Set("use_local_ipv6", use_local_ipv6); + dict.Set("num_hosts", static_cast<int>(hosts.size())); + dict.Set("doh_config", doh_config.ToValue()); + dict.Set("secure_dns_mode", base::strict_cast<int>(secure_dns_mode)); + dict.Set("allow_dns_over_https_upgrade", allow_dns_over_https_upgrade); + + return base::Value(std::move(dict)); } } // namespace net diff --git a/chromium/net/dns/dns_config.h b/chromium/net/dns/dns_config.h index 601ddc339ac..19fbd97fca9 100644 --- a/chromium/net/dns/dns_config.h +++ b/chromium/net/dns/dns_config.h @@ -55,7 +55,7 @@ struct NET_EXPORT DnsConfig { std::vector<IPEndPoint> nameservers; // Status of system DNS-over-TLS (DoT). - bool dns_over_tls_active; + bool dns_over_tls_active = false; std::string dns_over_tls_hostname; // Suffix search list; used on first lookup when number of dots in given name @@ -66,33 +66,33 @@ struct NET_EXPORT DnsConfig { // True if there are options set in the system configuration that are not yet // supported by DnsClient. - bool unhandled_options; + bool unhandled_options = false; // AppendToMultiLabelName: is suffix search performed for multi-label names? // True, except on Windows where it can be configured. - bool append_to_multi_label_name; + bool append_to_multi_label_name = true; // Resolver options; see man resolv.conf. // Minimum number of dots before global resolution precedes |search|. - int ndots; + int ndots = 1; // Time between retransmissions, see res_state.retrans. // Used by Chrome as the initial transaction attempt fallback period (before // exponential backoff and dynamic period determination based on previous // attempts.) - base::TimeDelta fallback_period; + base::TimeDelta fallback_period = kDnsDefaultFallbackPeriod; // Maximum number of attempts, see res_state.retry. - int attempts; + int attempts = 2; // Maximum number of times a DoH server is attempted per attempted per DNS // transaction. This is separate from the global failure limit. - int doh_attempts; + int doh_attempts = 1; // Round robin entries in |nameservers| for subsequent requests. - bool rotate; + bool rotate = false; // Indicates system configuration uses local IPv6 connectivity, e.g., // DirectAccess. This is exposed for HostResolver to skip IPv6 probes, // as it may cause them to return incorrect results. - bool use_local_ipv6; + bool use_local_ipv6 = false; // DNS over HTTPS server configuration. DnsOverHttpsConfig doh_config; @@ -101,12 +101,12 @@ struct NET_EXPORT DnsConfig { // overridden for individual requests (such as requests to resolve a DoH // server hostname) using |HostResolver::ResolveHostParameters:: // secure_dns_mode_override|. - SecureDnsMode secure_dns_mode; + SecureDnsMode secure_dns_mode = SecureDnsMode::kOff; // If set to |true|, we will attempt to upgrade the user's DNS configuration // to use DoH server(s) operated by the same provider(s) when the user is // in AUTOMATIC mode and has not pre-specified DoH servers. - bool allow_dns_over_https_upgrade; + bool allow_dns_over_https_upgrade = false; }; } // namespace net diff --git a/chromium/net/dns/dns_config_service.cc b/chromium/net/dns/dns_config_service.cc index 1aafa73664a..d1df61f0a92 100644 --- a/chromium/net/dns/dns_config_service.cc +++ b/chromium/net/dns/dns_config_service.cc @@ -30,12 +30,7 @@ const base::TimeDelta DnsConfigService::kInvalidationTimeout = DnsConfigService::DnsConfigService( base::FilePath::StringPieceType hosts_file_path, absl::optional<base::TimeDelta> config_change_delay) - : watch_failed_(false), - have_config_(false), - have_hosts_(false), - need_update_(false), - last_sent_empty_(true), - config_change_delay_(config_change_delay), + : config_change_delay_(config_change_delay), hosts_file_path_(hosts_file_path) { DETACH_FROM_SEQUENCE(sequence_checker_); } diff --git a/chromium/net/dns/dns_config_service.h b/chromium/net/dns/dns_config_service.h index 6afc7c06dda..d4aa18027e7 100644 --- a/chromium/net/dns/dns_config_service.h +++ b/chromium/net/dns/dns_config_service.h @@ -42,14 +42,6 @@ class NET_EXPORT_PRIVATE DnsConfigService { // reading system DNS settings is not supported on the current platform. static std::unique_ptr<DnsConfigService> CreateSystemService(); - // On detecting config change, will post and wait `config_change_delay` before - // triggering refreshes. Will trigger refreshes synchronously on nullopt. - // Useful for platforms where multiple changes may be made and detected before - // the config is stabilized and ready to be read. - explicit DnsConfigService(base::FilePath::StringPieceType hosts_file_path, - absl::optional<base::TimeDelta> - config_change_delay = base::Milliseconds(50)); - DnsConfigService(const DnsConfigService&) = delete; DnsConfigService& operator=(const DnsConfigService&) = delete; @@ -168,6 +160,14 @@ class NET_EXPORT_PRIVATE DnsConfigService { const base::FilePath hosts_file_path_; }; + // On detecting config change, will post and wait `config_change_delay` before + // triggering refreshes. Will trigger refreshes synchronously on nullopt. + // Useful for platforms where multiple changes may be made and detected before + // the config is stabilized and ready to be read. + explicit DnsConfigService(base::FilePath::StringPieceType hosts_file_path, + absl::optional<base::TimeDelta> + config_change_delay = base::Milliseconds(50)); + // Immediately attempts to read the current configuration. virtual void ReadConfigNow() = 0; virtual void ReadHostsNow(); @@ -205,15 +205,15 @@ class NET_EXPORT_PRIVATE DnsConfigService { // True if any of the necessary watchers failed. In that case, the service // will communicate changes via OnTimeout, but will only send empty DnsConfig. - bool watch_failed_; + bool watch_failed_ = false; // True after On*Read, before Invalidate*. Tells if the config is complete. - bool have_config_; - bool have_hosts_; + bool have_config_ = false; + bool have_hosts_ = false; // True if receiver needs to be updated when the config becomes complete. - bool need_update_; + bool need_update_ = false; // True if the last config sent was empty (instead of |dns_config_|). // Set when |timer_| expires. - bool last_sent_empty_; + bool last_sent_empty_ = true; const absl::optional<base::TimeDelta> config_change_delay_; const base::FilePath hosts_file_path_; diff --git a/chromium/net/dns/dns_config_service_win.cc b/chromium/net/dns/dns_config_service_win.cc index a3cab6e494d..e2d17692567 100644 --- a/chromium/net/dns/dns_config_service_win.cc +++ b/chromium/net/dns/dns_config_service_win.cc @@ -8,6 +8,7 @@ #include <algorithm> #include <memory> +#include <set> #include <string> #include "base/bind.h" @@ -20,6 +21,7 @@ #include "base/memory/free_deleter.h" #include "base/memory/raw_ptr.h" #include "base/memory/weak_ptr.h" +#include "base/metrics/histogram_functions.h" #include "base/sequence_checker.h" #include "base/strings/string_piece.h" #include "base/strings/string_split.h" @@ -54,6 +56,30 @@ const wchar_t kDnscachePath[] = const wchar_t kPolicyPath[] = L"SOFTWARE\\Policies\\Microsoft\\Windows NT\\DNSClient"; +// These values are persisted to logs. Entries should not be renumbered and +// numeric values should never be reused. +enum class DnsWindowsCompatibility { + kCompatible = 0, + kIncompatibleResolutionPolicy = 1, + kIncompatibleProxy = 1 << 1, + kIncompatibleVpn = 1 << 2, + kIncompatibleAdapterSpecificNameserver = 1 << 3, + + KAllIncompatibleFlags = (1 << 4) - 1, + kMaxValue = KAllIncompatibleFlags +}; + +inline constexpr DnsWindowsCompatibility operator|(DnsWindowsCompatibility a, + DnsWindowsCompatibility b) { + return static_cast<DnsWindowsCompatibility>(static_cast<int>(a) | + static_cast<int>(b)); +} + +inline DnsWindowsCompatibility& operator|=(DnsWindowsCompatibility& a, + DnsWindowsCompatibility b) { + return a = a | b; +} + // Wrapper for GetAdaptersAddresses to get unicast addresses. // Returns nullptr if failed. std::unique_ptr<IP_ADAPTER_ADDRESSES, base::FreeDeleter> @@ -283,6 +309,48 @@ void ConfigureSuffixSearch(const WinDnsSystemSettings& settings, } } +absl::optional<std::vector<IPEndPoint>> GetNameServers( + const IP_ADAPTER_ADDRESSES* adapter) { + std::vector<IPEndPoint> nameservers; + for (const IP_ADAPTER_DNS_SERVER_ADDRESS* address = + adapter->FirstDnsServerAddress; + address != nullptr; address = address->Next) { + IPEndPoint ipe; + if (ipe.FromSockAddr(address->Address.lpSockaddr, + address->Address.iSockaddrLength)) { + if (WinDnsSystemSettings::IsStatelessDiscoveryAddress(ipe.address())) + continue; + // Override unset port. + if (!ipe.port()) + ipe = IPEndPoint(ipe.address(), dns_protocol::kDefaultPort); + nameservers.push_back(ipe); + } else { + return absl::nullopt; + } + } + return nameservers; +} + +bool CheckAndRecordCompatibility(bool have_name_resolution_policy, + bool have_proxy, + bool uses_vpn, + bool has_adapter_specific_nameservers) { + DnsWindowsCompatibility compatibility = DnsWindowsCompatibility::kCompatible; + if (have_name_resolution_policy) + compatibility |= DnsWindowsCompatibility::kIncompatibleResolutionPolicy; + if (have_proxy) + compatibility |= DnsWindowsCompatibility::kIncompatibleProxy; + if (uses_vpn) + compatibility |= DnsWindowsCompatibility::kIncompatibleVpn; + if (has_adapter_specific_nameservers) { + compatibility |= + DnsWindowsCompatibility::kIncompatibleAdapterSpecificNameserver; + } + base::UmaHistogramEnumeration("Net.DNS.DnsConfig.Windows.Compatibility", + compatibility); + return compatibility == DnsWindowsCompatibility::kCompatible; +} + } // namespace std::string ParseDomainASCII(base::WStringPiece widestr) { @@ -336,9 +404,12 @@ std::vector<std::string> ParseSearchList(base::WStringPiece value) { absl::optional<DnsConfig> ConvertSettingsToDnsConfig( const WinDnsSystemSettings& settings) { bool uses_vpn = false; + bool has_adapter_specific_nameservers = false; DnsConfig dns_config; + std::set<IPEndPoint> previous_nameservers_set; + // Use GetAdapterAddresses to get effective DNS server order and // connection-specific DNS suffix. Ignore disconnected and loopback adapters. // The order of adapters is the network binding order, so stick to the @@ -351,6 +422,22 @@ absl::optional<DnsConfig> ConvertSettingsToDnsConfig( uses_vpn = true; } + absl::optional<std::vector<IPEndPoint>> nameservers = + GetNameServers(adapter); + if (!nameservers) + return absl::nullopt; + + if (!nameservers->empty() && (adapter->OperStatus == IfOperStatusUp)) { + // Check if the |adapter| has adapter specific nameservers. + std::set<IPEndPoint> nameservers_set(nameservers->begin(), + nameservers->end()); + if (!previous_nameservers_set.empty() && + (previous_nameservers_set != nameservers_set)) { + has_adapter_specific_nameservers = true; + } + previous_nameservers_set = std::move(nameservers_set); + } + // Skip disconnected and loopback adapters. If a good configuration was // previously found, skip processing another adapter. if (adapter->OperStatus != IfOperStatusUp || @@ -358,22 +445,7 @@ absl::optional<DnsConfig> ConvertSettingsToDnsConfig( !dns_config.nameservers.empty()) continue; - for (const IP_ADAPTER_DNS_SERVER_ADDRESS* address = - adapter->FirstDnsServerAddress; - address != nullptr; address = address->Next) { - IPEndPoint ipe; - if (ipe.FromSockAddr(address->Address.lpSockaddr, - address->Address.iSockaddrLength)) { - if (WinDnsSystemSettings::IsStatelessDiscoveryAddress(ipe.address())) - continue; - // Override unset port. - if (!ipe.port()) - ipe = IPEndPoint(ipe.address(), dns_protocol::kDefaultPort); - dns_config.nameservers.push_back(ipe); - } else { - return absl::nullopt; - } - } + dns_config.nameservers = std::move(*nameservers); // IP_ADAPTER_ADDRESSES in Vista+ has a search list at |FirstDnsSuffix|, // but it came up empty in all trials. @@ -403,8 +475,11 @@ absl::optional<DnsConfig> ConvertSettingsToDnsConfig( dns_config.use_local_ipv6 = true; } - if (settings.have_name_resolution_policy || settings.have_proxy || uses_vpn) + if (!CheckAndRecordCompatibility(settings.have_name_resolution_policy, + settings.have_proxy, uses_vpn, + has_adapter_specific_nameservers)) { dns_config.unhandled_options = true; + } ConfigureSuffixSearch(settings, dns_config); return dns_config; diff --git a/chromium/net/dns/dns_config_service_win_unittest.cc b/chromium/net/dns/dns_config_service_win_unittest.cc index 111eb6e4ed1..45af232c8f4 100644 --- a/chromium/net/dns/dns_config_service_win_unittest.cc +++ b/chromium/net/dns/dns_config_service_win_unittest.cc @@ -416,7 +416,7 @@ TEST(DnsConfigServiceWinTest, AppendToMultiLabelName) { } } -// Setting have_name_resolution_policy_table should set unhandled_options. +// 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" } }, @@ -443,7 +443,7 @@ TEST(DnsConfigServiceWinTest, HaveNRPT) { } } -// Setting have_proxy should set unhandled_options. +// Setting have_proxy should set `unhandled_options`. TEST(DnsConfigServiceWinTest, HaveProxy) { AdapterInfo infos[2] = { {IF_TYPE_USB, IfOperStatusUp, L"connection.suffix", {"1.0.0.1"}}, @@ -469,7 +469,7 @@ TEST(DnsConfigServiceWinTest, HaveProxy) { } } -// Setting uses_vpn should set unhandled_options. +// Setting uses_vpn should set `unhandled_options`. TEST(DnsConfigServiceWinTest, UsesVpn) { AdapterInfo infos[3] = { {IF_TYPE_USB, IfOperStatusUp, L"connection.suffix", {"1.0.0.1"}}, @@ -484,6 +484,49 @@ TEST(DnsConfigServiceWinTest, UsesVpn) { testing::IsTrue()))); } +// Setting adapter specific nameservers should set `unhandled_options`. +TEST(DnsConfigServiceWinTest, AdapterSpecificNameservers) { + AdapterInfo infos[3] = { + {IF_TYPE_FASTETHER, + IfOperStatusUp, + L"example.com", + {"1.0.0.1", "fec0:0:0:ffff::2", "8.8.8.8"}}, + {IF_TYPE_USB, + IfOperStatusUp, + L"chromium.org", + {"10.0.0.10", "2001:FFFF::1111"}}, + {0}, + }; + + WinDnsSystemSettings settings; + settings.addresses = CreateAdapterAddresses(infos); + EXPECT_THAT(internal::ConvertSettingsToDnsConfig(settings), + testing::Optional(testing::Field(&DnsConfig::unhandled_options, + testing::IsTrue()))); +} + +// Setting adapter specific nameservers for non operational adapter should not +// set `unhandled_options`. +TEST(DnsConfigServiceWinTest, AdapterSpecificNameserversForNo) { + AdapterInfo infos[3] = { + {IF_TYPE_FASTETHER, + IfOperStatusUp, + L"example.com", + {"1.0.0.1", "fec0:0:0:ffff::2", "8.8.8.8"}}, + {IF_TYPE_USB, + IfOperStatusDown, + L"chromium.org", + {"10.0.0.10", "2001:FFFF::1111"}}, + {0}, + }; + + WinDnsSystemSettings settings; + settings.addresses = CreateAdapterAddresses(infos); + EXPECT_THAT(internal::ConvertSettingsToDnsConfig(settings), + testing::Optional(testing::Field(&DnsConfig::unhandled_options, + testing::IsFalse()))); +} + } // namespace } // namespace net diff --git a/chromium/net/dns/dns_config_watcher_mac.cc b/chromium/net/dns/dns_config_watcher_mac.cc index f48dc70c0b5..9d5398a261b 100644 --- a/chromium/net/dns/dns_config_watcher_mac.cc +++ b/chromium/net/dns/dns_config_watcher_mac.cc @@ -20,10 +20,7 @@ class DnsInfoApi { typedef dns_config_t* (*dns_configuration_copy_t)(); typedef void (*dns_configuration_free_t)(dns_config_t*); - DnsInfoApi() - : dns_configuration_notify_key(NULL), - dns_configuration_copy(NULL), - dns_configuration_free(NULL) { + DnsInfoApi() { handle_ = dlopen("/usr/lib/libSystem.dylib", RTLD_LAZY | RTLD_NOLOAD); if (!handle_) @@ -44,9 +41,9 @@ class DnsInfoApi { dlclose(handle_); } - dns_configuration_notify_key_t dns_configuration_notify_key; - dns_configuration_copy_t dns_configuration_copy; - dns_configuration_free_t dns_configuration_free; + dns_configuration_notify_key_t dns_configuration_notify_key = nullptr; + dns_configuration_copy_t dns_configuration_copy = nullptr; + dns_configuration_free_t dns_configuration_free = nullptr; private: void* handle_; diff --git a/chromium/net/dns/dns_hosts.cc b/chromium/net/dns/dns_hosts.cc index 06541a73a97..ab2cf0d065e 100644 --- a/chromium/net/dns/dns_hosts.cc +++ b/chromium/net/dns/dns_hosts.cc @@ -29,8 +29,6 @@ class HostsParser { : text_(text), data_(text.data()), end_(text.size()), - pos_(0), - token_is_ip_(false), comma_mode_(comma_mode) {} HostsParser(const HostsParser&) = delete; @@ -127,9 +125,9 @@ class HostsParser { const char* data_; const size_t end_; - size_t pos_; + size_t pos_ = 0; StringPiece token_; - bool token_is_ip_; + bool token_is_ip_ = false; const ParseHostsCommaMode comma_mode_; }; diff --git a/chromium/net/dns/dns_test_util.cc b/chromium/net/dns/dns_test_util.cc index 0f07bb3dd84..5d401ae9935 100644 --- a/chromium/net/dns/dns_test_util.cc +++ b/chromium/net/dns/dns_test_util.cc @@ -410,9 +410,7 @@ class MockDnsTransactionFactory::MockTransaction SecureDnsMode secure_dns_mode, ResolveContext* resolve_context, bool fast_timeout) - : result_(MockDnsClientRule::ResultType::kFail), - hostname_(std::move(hostname)), - qtype_(qtype) { + : hostname_(std::move(hostname)), qtype_(qtype) { // Do not allow matching any rules if transaction is secure and no DoH // servers are available. if (!secure || force_doh_server_available || @@ -546,30 +544,28 @@ class MockDnsTransactionFactory::MockTransaction case MockDnsClientRule::ResultType::kFail: { int error = result_.net_error.value_or(ERR_NAME_NOT_RESOLVED); DCHECK_NE(error, OK); - std::move(callback_).Run( - error, base::OptionalOrNullptr(result_.response), absl::nullopt); + std::move(callback_).Run(error, + base::OptionalOrNullptr(result_.response)); break; } case MockDnsClientRule::ResultType::kEmpty: case MockDnsClientRule::ResultType::kOk: case MockDnsClientRule::ResultType::kMalformed: DCHECK(!result_.net_error.has_value()); - std::move(callback_).Run(OK, base::OptionalOrNullptr(result_.response), - absl::nullopt); + std::move(callback_).Run(OK, base::OptionalOrNullptr(result_.response)); break; case MockDnsClientRule::ResultType::kTimeout: DCHECK(!result_.net_error.has_value()); - std::move(callback_).Run(ERR_DNS_TIMED_OUT, nullptr, absl::nullopt); + std::move(callback_).Run(ERR_DNS_TIMED_OUT, /*response=*/nullptr); break; case MockDnsClientRule::ResultType::kSlow: if (result_.response) { std::move(callback_).Run( result_.net_error.value_or(OK), - result_.response ? &result_.response.value() : nullptr, - absl::nullopt); + result_.response ? &result_.response.value() : nullptr); } else { DCHECK(!result_.net_error.has_value()); - std::move(callback_).Run(ERR_DNS_TIMED_OUT, nullptr, absl::nullopt); + std::move(callback_).Run(ERR_DNS_TIMED_OUT, /*response=*/nullptr); } break; case MockDnsClientRule::ResultType::kUnexpected: @@ -581,7 +577,7 @@ class MockDnsTransactionFactory::MockTransaction void SetRequestPriority(RequestPriority priority) override {} - MockDnsClientRule::Result result_; + MockDnsClientRule::Result result_{MockDnsClientRule::ResultType::kFail}; const std::string hostname_; const uint16_t qtype_; ResponseCallback callback_; diff --git a/chromium/net/dns/dns_transaction.cc b/chromium/net/dns/dns_transaction.cc index 0a07cccaf4d..b41ce26b909 100644 --- a/chromium/net/dns/dns_transaction.cc +++ b/chromium/net/dns/dns_transaction.cc @@ -72,9 +72,6 @@ #include "net/socket/stream_socket.h" #include "net/third_party/uri_template/uri_template.h" #include "net/traffic_annotation/network_traffic_annotation.h" -#include "net/url_request/url_fetcher.h" -#include "net/url_request/url_fetcher_delegate.h" -#include "net/url_request/url_fetcher_response_writer.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_builder.h" #include "third_party/abseil-cpp/absl/types/optional.h" @@ -126,10 +123,10 @@ bool IsIPLiteral(const std::string& hostname) { } base::Value NetLogStartParams(const std::string& hostname, uint16_t qtype) { - base::Value dict(base::Value::Type::DICTIONARY); - dict.SetStringKey("hostname", hostname); - dict.SetIntKey("query_type", qtype); - return dict; + base::Value::Dict dict; + dict.Set("hostname", hostname); + dict.Set("query_type", qtype); + return base::Value(std::move(dict)); } // ---------------------------------------------------------------------------- @@ -169,23 +166,23 @@ class DnsAttempt { // to the NetLog source source of the UDP socket used. The request must have // completed before this is called. base::Value NetLogResponseParams(NetLogCaptureMode capture_mode) const { - base::Value dict(base::Value::Type::DICTIONARY); + base::Value::Dict dict; if (GetResponse()) { DCHECK(GetResponse()->IsValid()); - dict.SetIntKey("rcode", GetResponse()->rcode()); - dict.SetIntKey("answer_count", GetResponse()->answer_count()); - dict.SetIntKey("additional_answer_count", - GetResponse()->additional_answer_count()); + dict.Set("rcode", GetResponse()->rcode()); + dict.Set("answer_count", static_cast<int>(GetResponse()->answer_count())); + dict.Set("additional_answer_count", + static_cast<int>(GetResponse()->additional_answer_count())); } - GetSocketNetLog().source().AddToEventParameters(&dict); + GetSocketNetLog().source().AddToEventParameters(dict); if (capture_mode == NetLogCaptureMode::kEverything) { - dict.SetKey("response_buffer", GetRawResponseBufferForLog()); + dict.Set("response_buffer", GetRawResponseBufferForLog()); } - return dict; + return base::Value(std::move(dict)); } // True if current attempt is pending (waiting for server response). @@ -203,7 +200,6 @@ class DnsUDPAttempt : public DnsAttempt { std::unique_ptr<DnsQuery> query, DnsUdpTracker* udp_tracker) : DnsAttempt(server_index), - next_state_(STATE_NONE), socket_(std::move(socket)), server_(server), query_(std::move(query)), @@ -352,7 +348,7 @@ class DnsUDPAttempt : public DnsAttempt { std::move(callback_).Run(rv); } - State next_state_; + State next_state_ = STATE_NONE; base::TimeTicks start_time_; std::unique_ptr<DatagramClientSocket> socket_; @@ -652,12 +648,10 @@ class DnsTCPAttempt : public DnsAttempt { std::unique_ptr<StreamSocket> socket, std::unique_ptr<DnsQuery> query) : DnsAttempt(server_index), - next_state_(STATE_NONE), socket_(std::move(socket)), query_(std::move(query)), length_buffer_( - base::MakeRefCounted<IOBufferWithSize>(sizeof(uint16_t))), - response_length_(0) {} + base::MakeRefCounted<IOBufferWithSize>(sizeof(uint16_t))) {} DnsTCPAttempt(const DnsTCPAttempt&) = delete; DnsTCPAttempt& operator=(const DnsTCPAttempt&) = delete; @@ -881,7 +875,7 @@ class DnsTCPAttempt : public DnsAttempt { base::BindOnce(&DnsTCPAttempt::OnIOComplete, base::Unretained(this))); } - State next_state_; + State next_state_ = STATE_NONE; base::TimeTicks start_time_; std::unique_ptr<StreamSocket> socket_; @@ -889,7 +883,7 @@ class DnsTCPAttempt : public DnsAttempt { scoped_refptr<IOBufferWithSize> length_buffer_; scoped_refptr<DrainableIOBuffer> buffer_; - uint16_t response_length_; + uint16_t response_length_ = 0; std::unique_ptr<DnsResponse> response_; CompletionOnceCallback callback_; @@ -897,8 +891,6 @@ class DnsTCPAttempt : public DnsAttempt { // ---------------------------------------------------------------------------- -const char kDoHProbeHostname[] = "www.gstatic.com"; - const net::BackoffEntry::Policy kProbeBackoffPolicy = { // Apply exponential backoff rules after the first error. 0, @@ -933,7 +925,7 @@ class DnsOverHttpsProbeRunner : public DnsProbeRunner { DCHECK(!session_->config().doh_config.servers().empty()); DCHECK(context_); - DNSDomainFromDot(kDoHProbeHostname, &formatted_probe_hostname_); + DNSDomainFromDot(kDohProbeHostname, &formatted_probe_hostname_); for (size_t i = 0; i < session_->config().doh_config.servers().size(); i++) { @@ -1053,7 +1045,7 @@ class DnsOverHttpsProbeRunner : public DnsProbeRunner { DnsResponseResultExtractor::ExtractionError extraction_error = extractor.ExtractDnsResults( DnsQueryType::A, - /*original_domain_name=*/kDoHProbeHostname, + /*original_domain_name=*/kDohProbeHostname, /*request_port=*/0, &results); if (extraction_error == @@ -1123,11 +1115,7 @@ class DnsTransactionImpl : public DnsTransaction, secure_dns_mode_(secure_dns_mode), fast_timeout_(fast_timeout), net_log_(net_log), - qnames_initial_size_(0), - attempts_count_(0), - had_tcp_retry_(false), - resolve_context_(resolve_context->AsSafeRef()), - request_priority_(DEFAULT_PRIORITY) { + resolve_context_(resolve_context->AsSafeRef()) { DCHECK(session_.get()); DCHECK(!hostname_.empty()); DCHECK(!IsIPLiteral(hostname_)); @@ -1270,14 +1258,7 @@ class DnsTransactionImpl : public DnsTransaction, net_log_.EndEventWithNetErrorCode(NetLogEventType::DNS_TRANSACTION, result.rv); - absl::optional<std::string> doh_provider_id; - if (secure_ && result.attempt) { - size_t server_index = result.attempt->server_index(); - doh_provider_id = GetDohProviderIdForHistogramFromServerConfig( - session_->config().doh_config.servers()[server_index]); - } - - std::move(callback_).Run(result.rv, response, doh_provider_id); + std::move(callback_).Run(result.rv, response); } void RecordAttemptUma(DnsAttemptType attempt_type) { @@ -1689,15 +1670,15 @@ class DnsTransactionImpl : public DnsTransaction, // Search list of fully-qualified DNS names to query next (in DNS format). base::circular_deque<std::string> qnames_; - size_t qnames_initial_size_; + size_t qnames_initial_size_ = 0; // List of attempts for the current name. std::vector<std::unique_ptr<DnsAttempt>> attempts_; // Count of attempts, not reset when |attempts_| vector is cleared. - int attempts_count_; + int attempts_count_ = 0; // Records when an attempt was retried via TCP due to a truncation error. - bool had_tcp_retry_; + bool had_tcp_retry_ = false; // Iterator to get the index of the DNS server for each search query. std::unique_ptr<DnsServerIterator> dns_server_iterator_; @@ -1706,7 +1687,7 @@ class DnsTransactionImpl : public DnsTransaction, std::unique_ptr<base::ElapsedTimer> time_from_start_; base::SafeRef<ResolveContext> resolve_context_; - RequestPriority request_priority_; + RequestPriority request_priority_ = DEFAULT_PRIORITY; THREAD_CHECKER(thread_checker_); }; diff --git a/chromium/net/dns/dns_transaction.h b/chromium/net/dns/dns_transaction.h index a8d199bf74f..1ed0792086f 100644 --- a/chromium/net/dns/dns_transaction.h +++ b/chromium/net/dns/dns_transaction.h @@ -12,6 +12,7 @@ #include "base/callback.h" #include "base/memory/weak_ptr.h" +#include "base/strings/string_piece.h" #include "base/time/time.h" #include "net/base/request_priority.h" #include "net/dns/public/secure_dns_mode.h" @@ -26,6 +27,9 @@ class DnsSession; class NetLogWithSource; class ResolveContext; +// The hostname probed by CreateDohProbeRunner(). +inline constexpr base::StringPiece kDohProbeHostname = "www.gstatic.com"; + // DnsTransaction implements a stub DNS resolver as defined in RFC 1034. // The DnsTransaction takes care of retransmissions, name server fallback (or // round-robin), suffix search, and simple response validation ("does it match @@ -38,14 +42,8 @@ class NET_EXPORT_PRIVATE DnsTransaction { // Note that the `GetDottedName()` of the response may be different than the // original `hostname` (passed to `DnsTransactionFactory::CreateTransaction()` // as a result of suffix search. - // - // The `doh_provider_id` contains the provider ID for histograms of the last - // DoH server attempted. If the name is unavailable, or this is not a DoH - // transaction, `doh_provider_id` is nullopt. using ResponseCallback = - base::OnceCallback<void(int neterror, - const DnsResponse* response, - absl::optional<std::string> doh_provider_id)>; + base::OnceCallback<void(int neterror, const DnsResponse* response)>; virtual ~DnsTransaction() = default; diff --git a/chromium/net/dns/dns_transaction_unittest.cc b/chromium/net/dns/dns_transaction_unittest.cc index 3d49a787b83..94d024b3255 100644 --- a/chromium/net/dns/dns_transaction_unittest.cc +++ b/chromium/net/dns/dns_transaction_unittest.cc @@ -362,9 +362,7 @@ class TransactionHelper { transaction_.reset(nullptr); } - void OnTransactionComplete(int rv, - const DnsResponse* response, - absl::optional<std::string> doh_provider_id) { + void OnTransactionComplete(int rv, const DnsResponse* response) { EXPECT_FALSE(completed_); completed_ = true; @@ -443,8 +441,6 @@ class URLRequestMockDohJob : public URLRequestJob, public AsyncSocket { ResponseModifierCallback response_modifier = ResponseModifierCallback(), UrlRequestStartedCallback on_start = UrlRequestStartedCallback()) : URLRequestJob(request), - content_length_(0), - leftover_data_len_(0), data_provider_(data_provider), response_modifier_(response_modifier), on_start_(on_start) { @@ -585,9 +581,9 @@ class URLRequestMockDohJob : public URLRequestJob, public AsyncSocket { return data_len; } - const int content_length_; + const int content_length_ = 0; const char* leftover_data_; - int leftover_data_len_; + int leftover_data_len_ = 0; raw_ptr<SocketDataProvider> data_provider_; const ResponseModifierCallback response_modifier_; const UrlRequestStartedCallback on_start_; @@ -636,7 +632,8 @@ class DnsTransactionTestBase : public testing::Test { base::StringPrintf("doh_test_%zu", i)) + (use_post ? "" : "{?dns}")); } - config_.doh_config = *DnsOverHttpsConfig::FromStrings(std::move(templates)); + config_.doh_config = + *DnsOverHttpsConfig::FromTemplatesForTesting(std::move(templates)); ConfigureFactory(); if (make_available) { @@ -2353,8 +2350,7 @@ void MakeResponseWithCookie(URLRequest* request, HttpResponseInfo* info) { class CookieCallback { public: - CookieCallback() - : result_(false), loop_to_quit_(std::make_unique<base::RunLoop>()) {} + CookieCallback() : loop_to_quit_(std::make_unique<base::RunLoop>()) {} void SetCookieCallback(CookieAccessResult result) { result_ = result.status.IsInclude(); @@ -2379,7 +2375,7 @@ class CookieCallback { private: net::CookieList list_; - bool result_; + bool result_ = false; std::unique_ptr<base::RunLoop> loop_to_quit_; }; @@ -2547,7 +2543,7 @@ TEST_F(DnsTransactionTest, CanLookupDohServerName) { class CountingObserver : public net::NetLog::ThreadSafeObserver { public: - CountingObserver() : count_(0), dict_count_(0) {} + CountingObserver() = default; ~CountingObserver() override { if (net_log()) @@ -2565,8 +2561,8 @@ class CountingObserver : public net::NetLog::ThreadSafeObserver { int dict_count() const { return dict_count_; } private: - int count_; - int dict_count_; + int count_ = 0; + int dict_count_ = 0; }; // Flaky on MSAN. https://crbug.com/1245953 diff --git a/chromium/net/dns/dns_util_unittest.cc b/chromium/net/dns/dns_util_unittest.cc index 9febaeadaa7..c31e29a56e4 100644 --- a/chromium/net/dns/dns_util_unittest.cc +++ b/chromium/net/dns/dns_util_unittest.cc @@ -501,7 +501,7 @@ TEST_F(DNSUtilTest, GetDohUpgradeServersFromNameservers) { EXPECT_EQ(0u, doh_servers.size()); doh_servers = GetDohUpgradeServersFromNameservers(nameservers); - auto expected_config = *DnsOverHttpsConfig::FromStrings( + auto expected_config = *DnsOverHttpsConfig::FromTemplatesForTesting( {"https://chrome.cloudflare-dns.com/dns-query", "https://doh.cleanbrowsing.org/doh/family-filter{?dns}", "https://doh.cleanbrowsing.org/doh/security-filter{?dns}"}); diff --git a/chromium/net/dns/host_cache.cc b/chromium/net/dns/host_cache.cc index 2f70a5ebd0b..4e8d6eea38b 100644 --- a/chromium/net/dns/host_cache.cc +++ b/chromium/net/dns/host_cache.cc @@ -77,9 +77,9 @@ const char kHostnameResultsKey[] = "hostname_results"; const char kHostPortsKey[] = "host_ports"; base::Value IpEndpointToValue(const IPEndPoint& endpoint) { - base::Value::DictStorage dictionary; - dictionary.emplace(kEndpointAddressKey, endpoint.ToStringWithoutPort()); - dictionary.emplace(kEndpointPortKey, int{endpoint.port()}); + base::Value::Dict dictionary; + dictionary.Set(kEndpointAddressKey, endpoint.ToStringWithoutPort()); + dictionary.Set(kEndpointPortKey, endpoint.port()); return base::Value(std::move(dictionary)); } @@ -87,8 +87,9 @@ absl::optional<IPEndPoint> IpEndpointFromValue(const base::Value& value) { if (!value.is_dict()) return absl::nullopt; - const std::string* ip_str = value.FindStringKey(kEndpointAddressKey); - absl::optional<int> port = value.FindIntKey(kEndpointPortKey); + const base::Value::Dict& dict = value.GetDict(); + const std::string* ip_str = dict.FindString(kEndpointAddressKey); + absl::optional<int> port = dict.FindInt(kEndpointPortKey); if (!ip_str || !port || !base::IsValueInRangeForNumericType<uint16_t>(port.value())) { @@ -104,9 +105,9 @@ absl::optional<IPEndPoint> IpEndpointFromValue(const base::Value& value) { base::Value EndpointMetadataPairToValue( const std::pair<HttpsRecordPriority, ConnectionEndpointMetadata>& pair) { - base::Value::DictStorage dictionary; - dictionary.emplace(kEndpointMetadataWeightKey, int{pair.first}); - dictionary.emplace(kEndpointMetadataValueKey, pair.second.ToValue()); + base::Value::Dict dictionary; + dictionary.Set(kEndpointMetadataWeightKey, pair.first); + dictionary.Set(kEndpointMetadataValueKey, pair.second.ToValue()); return base::Value(std::move(dictionary)); } @@ -115,8 +116,9 @@ EndpointMetadataPairFromValue(const base::Value& value) { if (!value.is_dict()) return absl::nullopt; - absl::optional<int> priority = value.FindIntKey(kEndpointMetadataWeightKey); - const base::Value* metadata_value = value.FindKey(kEndpointMetadataValueKey); + const base::Value::Dict& dict = value.GetDict(); + absl::optional<int> priority = dict.FindInt(kEndpointMetadataWeightKey); + const base::Value* metadata_value = dict.Find(kEndpointMetadataValueKey); if (!priority || !base::IsValueInRangeForNumericType<HttpsRecordPriority>( priority.value())) { @@ -135,7 +137,7 @@ EndpointMetadataPairFromValue(const base::Value& value) { std::move(metadata).value()); } -bool AddressListFromListValue(const base::Value* value, +bool AddressListFromListValue(const base::Value::List* value, absl::optional<AddressList>* out_list) { if (!value) { out_list->reset(); @@ -143,7 +145,7 @@ bool AddressListFromListValue(const base::Value* value, } out_list->emplace(); - for (const auto& it : value->GetListDeprecated()) { + for (const auto& it : *value) { IPAddress address; const std::string* addr_string = it.GetIfString(); if (!addr_string || !address.AssignFromIPLiteral(*addr_string)) { @@ -474,7 +476,7 @@ void HostCache::Entry::GetStaleness(base::TimeTicks now, } base::Value HostCache::Entry::NetLogParams() const { - return GetAsValue(false /* include_staleness */); + return base::Value(GetAsValue(false /* include_staleness */)); } void HostCache::Entry::MergeAddressesFrom(const HostCache::Entry& source) { @@ -537,89 +539,85 @@ void HostCache::Entry::MergeDnsAliasesFrom(const HostCache::Entry& source) { legacy_addresses_->AppendDnsAliases(std::move(deduplicated_source_aliases)); } -base::Value HostCache::Entry::GetAsValue(bool include_staleness) const { - base::Value entry_dict(base::Value::Type::DICTIONARY); +base::Value::Dict HostCache::Entry::GetAsValue(bool include_staleness) const { + base::Value::Dict entry_dict; if (include_staleness) { // The kExpirationKey value is using TimeTicks instead of Time used if // |include_staleness| is false, so it cannot be used to deserialize. // This is ok as it is used only for netlog. - entry_dict.SetStringKey(kExpirationKey, - NetLog::TickCountToString(expires())); - entry_dict.SetIntKey(kTtlKey, ttl().InMilliseconds()); - entry_dict.SetIntKey(kNetworkChangesKey, network_changes()); + entry_dict.Set(kExpirationKey, NetLog::TickCountToString(expires())); + entry_dict.Set(kTtlKey, base::saturated_cast<int>(ttl().InMilliseconds())); + entry_dict.Set(kNetworkChangesKey, network_changes()); // The "pinned" status is meaningful only if "network_changes" is also // preserved. if (pinning()) - entry_dict.SetBoolKey(kPinnedKey, *pinning()); + entry_dict.Set(kPinnedKey, *pinning()); } else { // Convert expiration time in TimeTicks to Time for serialization, using a // string because base::Value doesn't handle 64-bit integers. base::Time expiration_time = base::Time::Now() - (base::TimeTicks::Now() - expires()); - entry_dict.SetStringKey( - kExpirationKey, - base::NumberToString(expiration_time.ToInternalValue())); + entry_dict.Set(kExpirationKey, + base::NumberToString(expiration_time.ToInternalValue())); } if (error() != OK) { - entry_dict.SetIntKey(kNetErrorKey, error()); + entry_dict.Set(kNetErrorKey, error()); } else { if (ip_endpoints_) { - base::Value::ListStorage ip_endpoints_list; + base::Value::List ip_endpoints_list; for (const IPEndPoint& ip_endpoint : ip_endpoints_.value()) { - ip_endpoints_list.push_back(IpEndpointToValue(ip_endpoint)); + ip_endpoints_list.Append(IpEndpointToValue(ip_endpoint)); } - entry_dict.SetKey(kIpEndpointsKey, - base::Value(std::move(ip_endpoints_list))); + entry_dict.Set(kIpEndpointsKey, std::move(ip_endpoints_list)); } if (endpoint_metadatas_) { - base::Value::ListStorage endpoint_metadatas_list; + base::Value::List endpoint_metadatas_list; for (const auto& endpoint_metadata_pair : endpoint_metadatas_.value()) { - endpoint_metadatas_list.push_back( + endpoint_metadatas_list.Append( EndpointMetadataPairToValue(endpoint_metadata_pair)); } - entry_dict.SetKey(kEndpointMetadatasKey, - base::Value(std::move(endpoint_metadatas_list))); + entry_dict.Set(kEndpointMetadatasKey, std::move(endpoint_metadatas_list)); } if (aliases()) { - base::Value::ListStorage alias_list; + base::Value::List alias_list; for (const std::string& alias : *aliases()) { - alias_list.emplace_back(alias); + alias_list.Append(alias); } - entry_dict.SetKey(kAliasesKey, base::Value(std::move(alias_list))); + entry_dict.Set(kAliasesKey, std::move(alias_list)); } if (legacy_addresses()) { // Append all of the resolved addresses. - base::ListValue addresses_value; + base::Value::List addresses_value; for (const IPEndPoint& address : legacy_addresses().value()) { addresses_value.Append(address.ToStringWithoutPort()); } - entry_dict.SetKey(kAddressesKey, std::move(addresses_value)); + entry_dict.Set(kAddressesKey, std::move(addresses_value)); } if (text_records()) { // Append all resolved text records. - base::ListValue text_list_value; + base::Value::List text_list_value; for (const std::string& text_record : text_records().value()) { text_list_value.Append(text_record); } - entry_dict.SetKey(kTextRecordsKey, std::move(text_list_value)); + entry_dict.Set(kTextRecordsKey, std::move(text_list_value)); } if (hostnames()) { // Append all the resolved hostnames. - base::ListValue hostnames_value; - base::ListValue host_ports_value; + base::Value::List hostnames_value; + base::Value::List host_ports_value; for (const HostPortPair& hostname : hostnames().value()) { hostnames_value.Append(hostname.host()); host_ports_value.Append(hostname.port()); } - entry_dict.SetKey(kHostnameResultsKey, std::move(hostnames_value)); - entry_dict.SetKey(kHostPortsKey, std::move(host_ports_value)); + entry_dict.Set(kHostnameResultsKey, std::move(hostnames_value)); + entry_dict.Set(kHostPortsKey, std::move(host_ports_value)); } } @@ -632,9 +630,6 @@ const HostCache::EntryStaleness HostCache::kNotStale = {base::Seconds(-1), 0, HostCache::HostCache(size_t max_entries) : max_entries_(max_entries), - network_changes_(0), - restore_size_(0), - delegate_(nullptr), tick_clock_(base::DefaultTickClock::GetInstance()) {} HostCache::~HostCache() { @@ -854,12 +849,10 @@ void HostCache::ClearForHosts( delegate_->ScheduleWrite(); } -void HostCache::GetList(base::Value* entry_list, +void HostCache::GetList(base::Value::List& entry_list, bool include_staleness, SerializationType serialization_type) const { - DCHECK(entry_list); - DCHECK(entry_list->is_list()); - entry_list->ClearList(); + entry_list.clear(); for (const auto& pair : entries_) { const Key& key = pair.first; @@ -878,54 +871,54 @@ void HostCache::GetList(base::Value* entry_list, base::Value(key.network_isolation_key.ToDebugString()); } - auto entry_dict = - std::make_unique<base::Value>(entry.GetAsValue(include_staleness)); + base::Value::Dict entry_dict = entry.GetAsValue(include_staleness); const auto* host = absl::get_if<url::SchemeHostPort>(&key.host); if (host) { - entry_dict->SetStringKey(kSchemeKey, host->scheme()); - entry_dict->SetStringKey(kHostnameKey, host->host()); - entry_dict->SetIntKey(kPortKey, host->port()); + entry_dict.Set(kSchemeKey, host->scheme()); + entry_dict.Set(kHostnameKey, host->host()); + entry_dict.Set(kPortKey, host->port()); } else { - entry_dict->SetStringKey(kHostnameKey, absl::get<std::string>(key.host)); + entry_dict.Set(kHostnameKey, absl::get<std::string>(key.host)); } - entry_dict->SetIntKey(kDnsQueryTypeKey, - base::strict_cast<int>(key.dns_query_type)); - entry_dict->SetIntKey(kFlagsKey, key.host_resolver_flags); - entry_dict->SetIntKey(kHostResolverSourceKey, - static_cast<int>(key.host_resolver_source)); - entry_dict->SetKey(kNetworkIsolationKeyKey, - std::move(network_isolation_key_value)); - entry_dict->SetBoolKey(kSecureKey, static_cast<bool>(key.secure)); + entry_dict.Set(kDnsQueryTypeKey, + base::strict_cast<int>(key.dns_query_type)); + entry_dict.Set(kFlagsKey, key.host_resolver_flags); + entry_dict.Set(kHostResolverSourceKey, + base::strict_cast<int>(key.host_resolver_source)); + entry_dict.Set(kNetworkIsolationKeyKey, + std::move(network_isolation_key_value)); + entry_dict.Set(kSecureKey, key.secure); - entry_list->Append(std::move(*entry_dict)); + entry_list.Append(std::move(entry_dict)); } } -bool HostCache::RestoreFromListValue(const base::Value& old_cache) { +bool HostCache::RestoreFromListValue(const base::Value::List& old_cache) { // Reset the restore size to 0. restore_size_ = 0; - for (const auto& entry_dict : old_cache.GetListDeprecated()) { + for (const auto& entry : old_cache) { // If the cache is already full, don't bother prioritizing what to evict, // just stop restoring. if (size() == max_entries_) break; - if (!entry_dict.is_dict()) + if (!entry.is_dict()) return false; - const std::string* hostname_ptr = entry_dict.FindStringKey(kHostnameKey); + const base::Value::Dict& entry_dict = entry.GetDict(); + const std::string* hostname_ptr = entry_dict.FindString(kHostnameKey); if (!hostname_ptr || !IsValidHostname(*hostname_ptr)) { return false; } // Use presence of scheme to determine host type. - const std::string* scheme_ptr = entry_dict.FindStringKey(kSchemeKey); + const std::string* scheme_ptr = entry_dict.FindString(kSchemeKey); absl::variant<url::SchemeHostPort, std::string> host; if (scheme_ptr) { - absl::optional<int> port = entry_dict.FindIntKey(kPortKey); + absl::optional<int> port = entry_dict.FindInt(kPortKey); if (!port || !base::IsValueInRangeForNumericType<uint16_t>(port.value())) return false; @@ -938,16 +931,15 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { host = *hostname_ptr; } - const std::string* expiration_ptr = - entry_dict.FindStringKey(kExpirationKey); - absl::optional<int> maybe_flags = entry_dict.FindIntKey(kFlagsKey); + const std::string* expiration_ptr = entry_dict.FindString(kExpirationKey); + absl::optional<int> maybe_flags = entry_dict.FindInt(kFlagsKey); if (expiration_ptr == nullptr || !maybe_flags.has_value()) return false; std::string expiration(*expiration_ptr); HostResolverFlags flags = maybe_flags.value(); absl::optional<int> maybe_dns_query_type = - entry_dict.FindIntKey(kDnsQueryTypeKey); + entry_dict.FindInt(kDnsQueryTypeKey); if (!maybe_dns_query_type.has_value()) return false; DnsQueryType dns_query_type = @@ -955,11 +947,11 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { // HostResolverSource is optional. int host_resolver_source = - entry_dict.FindIntKey(kHostResolverSourceKey) - .value_or(static_cast<int>(HostResolverSource::ANY)); + entry_dict.FindInt(kHostResolverSourceKey) + .value_or(base::strict_cast<int>(HostResolverSource::ANY)); const base::Value* network_isolation_key_value = - entry_dict.FindKey(kNetworkIsolationKeyKey); + entry_dict.Find(kNetworkIsolationKeyKey); NetworkIsolationKey network_isolation_key; if (!network_isolation_key_value || network_isolation_key_value->type() == base::Value::Type::STRING || @@ -968,31 +960,31 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { return false; } - bool secure = entry_dict.FindBoolKey(kSecureKey).value_or(false); + bool secure = entry_dict.FindBool(kSecureKey).value_or(false); int error = OK; - const base::Value* ip_endpoints_value = nullptr; - const base::Value* endpoint_metadatas_value = nullptr; - const base::Value* aliases_value = nullptr; - const base::Value* legacy_addresses_value = nullptr; - const base::Value* text_records_value = nullptr; - const base::Value* hostname_records_value = nullptr; - const base::Value* host_ports_value = nullptr; - absl::optional<int> maybe_error = entry_dict.FindIntKey(kNetErrorKey); - absl::optional<bool> maybe_pinned = entry_dict.FindBoolKey(kPinnedKey); + const base::Value::List* ip_endpoints_list = nullptr; + const base::Value::List* endpoint_metadatas_list = nullptr; + const base::Value::List* aliases_list = nullptr; + const base::Value::List* legacy_addresses_list = nullptr; + const base::Value::List* text_records_list = nullptr; + const base::Value::List* hostname_records_list = nullptr; + const base::Value::List* host_ports_list = nullptr; + absl::optional<int> maybe_error = entry_dict.FindInt(kNetErrorKey); + absl::optional<bool> maybe_pinned = entry_dict.FindBool(kPinnedKey); if (maybe_error.has_value()) { error = maybe_error.value(); } else { - ip_endpoints_value = entry_dict.FindListKey(kIpEndpointsKey); - endpoint_metadatas_value = entry_dict.FindListKey(kEndpointMetadatasKey); - aliases_value = entry_dict.FindListKey(kAliasesKey); - legacy_addresses_value = entry_dict.FindListKey(kAddressesKey); - text_records_value = entry_dict.FindListKey(kTextRecordsKey); - hostname_records_value = entry_dict.FindListKey(kHostnameResultsKey); - host_ports_value = entry_dict.FindListKey(kHostPortsKey); - - if ((hostname_records_value == nullptr && host_ports_value != nullptr) || - (hostname_records_value != nullptr && host_ports_value == nullptr)) { + ip_endpoints_list = entry_dict.FindList(kIpEndpointsKey); + endpoint_metadatas_list = entry_dict.FindList(kEndpointMetadatasKey); + aliases_list = entry_dict.FindList(kAliasesKey); + legacy_addresses_list = entry_dict.FindList(kAddressesKey); + text_records_list = entry_dict.FindList(kTextRecordsKey); + hostname_records_list = entry_dict.FindList(kHostnameResultsKey); + host_ports_list = entry_dict.FindList(kHostPortsKey); + + if ((hostname_records_list == nullptr && host_ports_list != nullptr) || + (hostname_records_list != nullptr && host_ports_list == nullptr)) { return false; } } @@ -1006,10 +998,9 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { (base::Time::Now() - base::Time::FromInternalValue(time_internal)); absl::optional<std::vector<IPEndPoint>> ip_endpoints; - if (ip_endpoints_value) { + if (ip_endpoints_list) { ip_endpoints.emplace(); - for (const base::Value& ip_endpoint_value : - ip_endpoints_value->GetListDeprecated()) { + for (const base::Value& ip_endpoint_value : *ip_endpoints_list) { absl::optional<IPEndPoint> ip_endpoint = IpEndpointFromValue(ip_endpoint_value); if (!ip_endpoint) @@ -1021,10 +1012,10 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { absl::optional< std::multimap<HttpsRecordPriority, ConnectionEndpointMetadata>> endpoint_metadatas; - if (endpoint_metadatas_value) { + if (endpoint_metadatas_list) { endpoint_metadatas.emplace(); for (const base::Value& endpoint_metadata_value : - endpoint_metadatas_value->GetListDeprecated()) { + *endpoint_metadatas_list) { absl::optional< std::pair<HttpsRecordPriority, ConnectionEndpointMetadata>> pair = EndpointMetadataPairFromValue(endpoint_metadata_value); @@ -1035,26 +1026,25 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { } absl::optional<std::set<std::string>> aliases; - if (aliases_value) { + if (aliases_list) { aliases.emplace(); - for (const base::Value& alias_value : - aliases_value->GetListDeprecated()) { + for (const base::Value& alias_value : *aliases_list) { if (!alias_value.is_string()) return false; aliases->insert(alias_value.GetString()); } } - absl::optional<AddressList> legacy_address_list; - if (!AddressListFromListValue(legacy_addresses_value, - &legacy_address_list)) { + absl::optional<AddressList> legacy_address_value; + if (!AddressListFromListValue(legacy_addresses_list, + &legacy_address_value)) { return false; } absl::optional<std::vector<std::string>> text_records; - if (text_records_value) { + if (text_records_list) { text_records.emplace(); - for (const base::Value& value : text_records_value->GetListDeprecated()) { + for (const base::Value& value : *text_records_list) { if (!value.is_string()) return false; text_records.value().push_back(value.GetString()); @@ -1062,26 +1052,23 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { } absl::optional<std::vector<HostPortPair>> hostname_records; - if (hostname_records_value) { - DCHECK(host_ports_value); - if (hostname_records_value->GetListDeprecated().size() != - host_ports_value->GetListDeprecated().size()) { + if (hostname_records_list) { + DCHECK(host_ports_list); + if (hostname_records_list->size() != host_ports_list->size()) { return false; } hostname_records.emplace(); - for (size_t i = 0; i < hostname_records_value->GetListDeprecated().size(); - ++i) { - if (!hostname_records_value->GetListDeprecated()[i].is_string() || - !host_ports_value->GetListDeprecated()[i].is_int() || + for (size_t i = 0; i < hostname_records_list->size(); ++i) { + if (!(*hostname_records_list)[i].is_string() || + !(*host_ports_list)[i].is_int() || !base::IsValueInRangeForNumericType<uint16_t>( - host_ports_value->GetListDeprecated()[i].GetInt())) { + (*host_ports_list)[i].GetInt())) { return false; } hostname_records.value().push_back(HostPortPair( - hostname_records_value->GetListDeprecated()[i].GetString(), - base::checked_cast<uint16_t>( - host_ports_value->GetListDeprecated()[i].GetInt()))); + (*hostname_records_list)[i].GetString(), + base::checked_cast<uint16_t>((*host_ports_list)[i].GetInt()))); } } @@ -1090,8 +1077,8 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { // Assume an empty address list if we have an address type and no results. if (IsAddressType(dns_query_type) && !ip_endpoints && - !legacy_address_list && !text_records && !hostname_records) { - legacy_address_list.emplace(); + !legacy_address_value && !text_records && !hostname_records) { + legacy_address_value.emplace(); } Key key(std::move(host), dns_query_type, flags, @@ -1104,7 +1091,7 @@ bool HostCache::RestoreFromListValue(const base::Value& old_cache) { auto found = entries_.find(key); if (found == entries_.end()) { Entry entry(error, std::move(ip_endpoints), std::move(endpoint_metadatas), - std::move(aliases), legacy_address_list, + std::move(aliases), legacy_address_value, std::move(text_records), std::move(hostname_records), std::move(experimental_results), Entry::SOURCE_UNKNOWN, expiration_time, network_changes_ - 1); diff --git a/chromium/net/dns/host_cache.h b/chromium/net/dns/host_cache.h index 9924e432b52..bcbc165499f 100644 --- a/chromium/net/dns/host_cache.h +++ b/chromium/net/dns/host_cache.h @@ -327,7 +327,7 @@ class NET_EXPORT HostCache { // DNS aliases and deduplicates. void MergeDnsAliasesFrom(const HostCache::Entry& source); - base::Value GetAsValue(bool include_staleness) const; + base::Value::Dict GetAsValue(bool include_staleness) const; // The resolve results for this entry. int error_ = ERR_FAILED; @@ -462,13 +462,13 @@ class NET_EXPORT HostCache { // Fills the provided base::Value with the contents of the cache for // serialization. `entry_list` must be non-null list, and will be cleared // before adding the cache contents. - void GetList(base::Value* entry_list, + void GetList(base::Value::List& entry_list, bool include_staleness, SerializationType serialization_type) const; // Takes a base::Value list representing cache entries and stores them in the // cache, skipping any that already have entries. Returns true on success, // false on failure. - bool RestoreFromListValue(const base::Value& old_cache); + bool RestoreFromListValue(const base::Value::List& old_cache); // Returns the number of entries that were restored in the last call to // RestoreFromListValue(). size_t last_restore_size() const { return restore_size_; } @@ -526,12 +526,12 @@ class NET_EXPORT HostCache { // a resolved result entry. EntryMap entries_; size_t max_entries_; - int network_changes_; + int network_changes_ = 0; // Number of cache entries that were restored in the last call to // RestoreFromListValue(). Used in histograms. - size_t restore_size_; + size_t restore_size_ = 0; - raw_ptr<PersistenceDelegate> delegate_; + raw_ptr<PersistenceDelegate> delegate_ = nullptr; // Shared tick clock, overridden for testing. raw_ptr<const base::TickClock> tick_clock_; diff --git a/chromium/net/dns/host_cache_fuzzer.cc b/chromium/net/dns/host_cache_fuzzer.cc index c35adbc8e7d..8b302a29706 100644 --- a/chromium/net/dns/host_cache_fuzzer.cc +++ b/chromium/net/dns/host_cache_fuzzer.cc @@ -77,13 +77,13 @@ DEFINE_PROTO_FUZZER(const host_cache_fuzzer_proto::JsonOrBytes& input) { // Parse the HostCache. constexpr size_t kMaxEntries = 1000; HostCache host_cache(kMaxEntries); - if (!host_cache.RestoreFromListValue(*value)) + if (!host_cache.RestoreFromListValue(value->GetList())) return; // Serialize the HostCache. - base::Value serialized(base::Value::Type::LIST); + base::Value::List serialized; host_cache.GetList( - &serialized /* entry_list */, true /* include_staleness */, + serialized /* entry_list */, true /* include_staleness */, HostCache::SerializationType::kRestorable /* serialization_type */); CHECK_EQ(*value, serialized); diff --git a/chromium/net/dns/host_cache_unittest.cc b/chromium/net/dns/host_cache_unittest.cc index ee8c983e077..d53d089782f 100644 --- a/chromium/net/dns/host_cache_unittest.cc +++ b/chromium/net/dns/host_cache_unittest.cc @@ -1263,8 +1263,8 @@ TEST(HostCacheTest, SerializeAndDeserializeWithExpirations) { // Advance to t=12, and serialize/deserialize the cache. now += base::Seconds(7); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); @@ -1338,8 +1338,8 @@ TEST(HostCacheTest, SerializeAndDeserializeWithChanges) { EXPECT_EQ(2u, cache.size()); // Serialize the cache. - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); @@ -1440,8 +1440,8 @@ TEST(HostCacheTest, SerializeAndDeserializeLegacyAddresses) { // Advance to t=12, ansd serialize the cache. now += base::Seconds(7); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); @@ -1534,8 +1534,8 @@ TEST(HostCacheTest, SerializeAndDeserializeEntryWithoutScheme) { ASSERT_TRUE(cache.Lookup(key, now)); ASSERT_EQ(cache.size(), 1u); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, /*include_staleness=*/false, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, /*include_staleness=*/false, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); EXPECT_TRUE(restored_cache.RestoreFromListValue(serialized_cache)); @@ -1579,8 +1579,8 @@ TEST(HostCacheTest, SerializeAndDeserializeWithNetworkIsolationKey) { cache.Lookup(key2, now)->first.network_isolation_key); EXPECT_EQ(2u, cache.size()); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); EXPECT_TRUE(restored_cache.RestoreFromListValue(serialized_cache)); @@ -1615,18 +1615,18 @@ TEST(HostCacheTest, SerializeForDebugging) { cache.Lookup(key, now)->first.network_isolation_key); EXPECT_EQ(1u, cache.size()); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kDebug); HostCache restored_cache(kMaxCacheEntries); EXPECT_FALSE(restored_cache.RestoreFromListValue(serialized_cache)); - base::Value::ListView list = serialized_cache.GetListDeprecated(); - ASSERT_EQ(1u, list.size()); - ASSERT_TRUE(list[0].is_dict()); - base::Value* nik_value = list[0].FindPath("network_isolation_key"); - ASSERT_TRUE(nik_value); - ASSERT_EQ(base::Value(kNetworkIsolationKey.ToDebugString()), *nik_value); + ASSERT_EQ(1u, serialized_cache.size()); + ASSERT_TRUE(serialized_cache[0].is_dict()); + const std::string* nik_string = + serialized_cache[0].GetDict().FindString("network_isolation_key"); + ASSERT_TRUE(nik_string); + ASSERT_EQ(kNetworkIsolationKey.ToDebugString(), *nik_string); } TEST(HostCacheTest, SerializeAndDeserialize_Text) { @@ -1645,13 +1645,13 @@ TEST(HostCacheTest, SerializeAndDeserialize_Text) { cache.Set(key, entry, now, ttl); EXPECT_EQ(1u, cache.size()); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); restored_cache.RestoreFromListValue(serialized_cache); - ASSERT_EQ(1u, serialized_cache.GetListDeprecated().size()); + ASSERT_EQ(1u, serialized_cache.size()); ASSERT_EQ(1u, restored_cache.size()); HostCache::EntryStaleness stale; const std::pair<const HostCache::Key, HostCache::Entry>* result = @@ -1676,8 +1676,8 @@ TEST(HostCacheTest, SerializeAndDeserialize_Hostname) { cache.Set(key, entry, now, ttl); EXPECT_EQ(1u, cache.size()); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); restored_cache.RestoreFromListValue(serialized_cache); @@ -1726,8 +1726,8 @@ TEST(HostCacheTest, SerializeAndDeserializeEndpointResult) { cache.Set(key, merged_entry, now, ttl); EXPECT_EQ(1u, cache.size()); - base::Value serialized_cache(base::Value::Type::LIST); - cache.GetList(&serialized_cache, false /* include_staleness */, + base::Value::List serialized_cache; + cache.GetList(serialized_cache, false /* include_staleness */, HostCache::SerializationType::kRestorable); HostCache restored_cache(kMaxCacheEntries); restored_cache.RestoreFromListValue(serialized_cache); diff --git a/chromium/net/dns/host_resolver.cc b/chromium/net/dns/host_resolver.cc index b148c4c6af2..29ba042292c 100644 --- a/chromium/net/dns/host_resolver.cc +++ b/chromium/net/dns/host_resolver.cc @@ -19,7 +19,6 @@ #include "base/values.h" #include "net/base/address_list.h" #include "net/base/net_errors.h" -#include "net/base/network_change_notifier.h" #include "net/dns/context_host_resolver.h" #include "net/dns/dns_client.h" #include "net/dns/dns_util.h" @@ -173,6 +172,11 @@ const URLRequestContext* HostResolver::GetContextForTesting() const { return nullptr; } +NetworkChangeNotifier::NetworkHandle HostResolver::GetTargetNetworkForTesting() + const { + return NetworkChangeNotifier::kInvalidNetworkHandle; +} + // static std::unique_ptr<HostResolver> HostResolver::CreateResolver( HostResolverManager* manager, @@ -237,17 +241,51 @@ HostResolver::CreateStandaloneNetworkBoundResolver( base::StringPiece host_mapping_rules, bool enable_caching) { #if BUILDFLAG(IS_ANDROID) + // Note that the logic below uses Android APIs that don't work on a sandboxed + // process: This is not problematic because this function is used only by + // Cronet which doesn't enable sandboxing. + auto resolve_context = std::make_unique<ResolveContext>( nullptr /*url_request_context */, enable_caching); - - // Currently, only the system host resolver can perform lookups for a - // specific network. - // TODO(crbug.com/1309094): Remove this once the built-in resolver can also do - // this. auto manager_options = std::move(options).value_or(ManagerOptions()); - manager_options.insecure_dns_client_enabled = false; - manager_options.additional_types_via_insecure_dns_enabled = false; + // Support the use of the built-in resolver when possible. + bool is_builtin_resolver_supported = + manager_options.insecure_dns_client_enabled && + base::android::BuildInfo::GetInstance()->sdk_int() >= + base::android::SDK_VERSION_P; + if (is_builtin_resolver_supported) { + // Pre-existing DnsConfigOverrides is currently ignored, consider extending + // if a use case arises. + DCHECK(manager_options.dns_config_overrides == DnsConfigOverrides()); + + std::vector<IPEndPoint> dns_servers; + bool dns_over_tls_active; + std::string dns_over_tls_hostname; + std::vector<std::string> search_suffixes; + if (android::GetDnsServersForNetwork(&dns_servers, &dns_over_tls_active, + &dns_over_tls_hostname, + &search_suffixes, target_network)) { + DnsConfigOverrides dns_config_overrides = + DnsConfigOverrides::CreateOverridingEverythingWithDefaults(); + dns_config_overrides.nameservers = dns_servers; + // Android APIs don't specify whether to use DoT or DoH. So, leave the + // decision to `DnsConfig::allow_dns_over_https_upgrade` default value. + dns_config_overrides.dns_over_tls_active = dns_over_tls_active; + dns_config_overrides.dns_over_tls_hostname = dns_over_tls_hostname; + dns_config_overrides.search = search_suffixes; + + manager_options.dns_config_overrides = dns_config_overrides; + // Regardless of DoH vs DoT, the important contract to respect is not to + // perform insecure DNS lookups if `dns_over_tls_active` == true. + manager_options.additional_types_via_insecure_dns_enabled = + !dns_over_tls_active; + } else { + // Disable when android::GetDnsServersForNetwork fails. + is_builtin_resolver_supported = false; + } + } + manager_options.insecure_dns_client_enabled = is_builtin_resolver_supported; return std::make_unique<ContextHostResolver>( HostResolverManager::CreateNetworkBoundHostResolverManager( manager_options, target_network, net_log), diff --git a/chromium/net/dns/host_resolver.h b/chromium/net/dns/host_resolver.h index dbaec076482..bf866d7739b 100644 --- a/chromium/net/dns/host_resolver.h +++ b/chromium/net/dns/host_resolver.h @@ -56,6 +56,8 @@ class URLRequestContext; // See mock_host_resolver.h for test implementations. class NET_EXPORT HostResolver { public: + using Host = absl::variant<url::SchemeHostPort, HostPortPair>; + // Handler for an individual host resolution request. Created by // HostResolver::CreateRequest(). class ResolveHostRequest { @@ -393,6 +395,8 @@ class NET_EXPORT HostResolver { virtual HostResolverManager* GetManagerForTesting(); virtual const URLRequestContext* GetContextForTesting() const; + virtual NetworkChangeNotifier::NetworkHandle GetTargetNetworkForTesting() + const; // Creates a new HostResolver. |manager| must outlive the returned resolver. // @@ -424,6 +428,8 @@ class NET_EXPORT HostResolver { // performed exclusively for `target_network`, lookups will fail if // `target_network` disconnects. This can only be used by network-bound // URLRequestContexts. + // Due to the current implementation, if `options` is specified, its + // DnsConfigOverrides parameter must be empty. // Only implemented for Android starting from Marshmallow. static std::unique_ptr<HostResolver> CreateStandaloneNetworkBoundResolver( NetLog* net_log, diff --git a/chromium/net/dns/host_resolver_manager.cc b/chromium/net/dns/host_resolver_manager.cc index 207b769273f..713f7e6cad7 100644 --- a/chromium/net/dns/host_resolver_manager.cc +++ b/chromium/net/dns/host_resolver_manager.cc @@ -281,14 +281,14 @@ bool HaveOnlyLoopbackAddresses() { base::Value NetLogProcTaskFailedParams(uint32_t attempt_number, int net_error, int os_error) { - base::Value dict(base::Value::Type::DICTIONARY); + base::Value::Dict dict; if (attempt_number) - dict.SetIntKey("attempt_number", attempt_number); + dict.Set("attempt_number", base::saturated_cast<int>(attempt_number)); - dict.SetIntKey("net_error", net_error); + dict.Set("net_error", net_error); if (os_error) { - dict.SetIntKey("os_error", os_error); + dict.Set("os_error", os_error); #if BUILDFLAG(IS_WIN) // Map the error code to a human-readable string. LPWSTR error_string = nullptr; @@ -299,14 +299,14 @@ base::Value NetLogProcTaskFailedParams(uint32_t attempt_number, (LPWSTR)&error_string, 0, // Buffer size. nullptr); // Arguments (unused). - dict.SetStringKey("os_error_string", base::WideToUTF8(error_string)); + dict.Set("os_error_string", base::WideToUTF8(error_string)); LocalFree(error_string); #elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) - dict.SetStringKey("os_error_string", gai_strerror(os_error)); + dict.Set("os_error_string", gai_strerror(os_error)); #endif } - return dict; + return base::Value(std::move(dict)); } // Creates NetLog parameters when the DnsTask failed. @@ -315,44 +315,45 @@ base::Value NetLogDnsTaskFailedParams( absl::optional<DnsQueryType> failed_transaction_type, absl::optional<base::TimeDelta> ttl, const HostCache::Entry* saved_results) { - base::Value dict(base::Value::Type::DICTIONARY); + base::Value::Dict dict; if (failed_transaction_type) { - dict.SetIntKey("dns_query_type", - static_cast<int>(failed_transaction_type.value())); + dict.Set("dns_query_type", + base::strict_cast<int>(failed_transaction_type.value())); } if (ttl) - dict.SetIntKey("error_ttl_sec", ttl.value().InSeconds()); - dict.SetIntKey("net_error", net_error); + dict.Set("error_ttl_sec", + base::saturated_cast<int>(ttl.value().InSeconds())); + dict.Set("net_error", net_error); if (saved_results) - dict.SetKey("saved_results", saved_results->NetLogParams()); - return dict; + dict.Set("saved_results", saved_results->NetLogParams()); + return base::Value(std::move(dict)); } base::Value NetLogDnsTaskExtractionFailureParams( DnsResponseResultExtractor::ExtractionError extraction_error, DnsQueryType dns_query_type, const HostCache::Entry& results) { - base::Value dict(base::Value::Type::DICTIONARY); - dict.SetIntKey("extraction_error", static_cast<int>(extraction_error)); - dict.SetIntKey("dns_query_type", static_cast<int>(dns_query_type)); - dict.SetKey("results", results.NetLogParams()); - return dict; + base::Value::Dict dict; + dict.Set("extraction_error", base::strict_cast<int>(extraction_error)); + dict.Set("dns_query_type", base::strict_cast<int>(dns_query_type)); + dict.Set("results", results.NetLogParams()); + return base::Value(std::move(dict)); } // Creates NetLog parameters for HOST_RESOLVER_MANAGER_JOB_ATTACH/DETACH events. base::Value NetLogJobAttachParams(const NetLogSource& source, RequestPriority priority) { - base::Value dict(base::Value::Type::DICTIONARY); - source.AddToEventParameters(&dict); - dict.SetStringKey("priority", RequestPriorityToString(priority)); - return dict; + base::Value::Dict dict; + source.AddToEventParameters(dict); + dict.Set("priority", RequestPriorityToString(priority)); + return base::Value(std::move(dict)); } base::Value NetLogIPv6AvailableParams(bool ipv6_available, bool cached) { - base::Value dict(base::Value::Type::DICTIONARY); - dict.SetBoolKey("ipv6_available", ipv6_available); - dict.SetBoolKey("cached", cached); - return dict; + base::Value::Dict dict; + dict.Set("ipv6_available", ipv6_available); + dict.Set("cached", cached); + return base::Value(std::move(dict)); } // The logging routines are defined here because some requests are resolved @@ -462,13 +463,12 @@ class PriorityTracker { }; base::Value NetLogResults(const HostCache::Entry& results) { - base::Value dict(base::Value::Type::DICTIONARY); - dict.SetKey("results", results.NetLogParams()); - return dict; + base::Value::Dict dict; + dict.Set("results", results.NetLogParams()); + return base::Value(std::move(dict)); } -base::Value ToLogStringValue( - const absl::variant<url::SchemeHostPort, HostPortPair>& host) { +base::Value ToLogStringValue(const HostResolver::Host& host) { if (absl::holds_alternative<url::SchemeHostPort>(host)) return base::Value(absl::get<url::SchemeHostPort>(host).Serialize()); @@ -493,8 +493,7 @@ base::StringPiece GetScheme( return base::StringPiece(); } -base::StringPiece GetHostname( - const absl::variant<url::SchemeHostPort, HostPortPair>& host) { +base::StringPiece GetHostname(const HostResolver::Host& host) { if (absl::holds_alternative<url::SchemeHostPort>(host)) { base::StringPiece hostname = absl::get<url::SchemeHostPort>(host).host(); if (hostname.size() >= 2 && hostname.front() == '[' && @@ -521,7 +520,7 @@ base::StringPiece GetHostname( return absl::get<std::string>(host); } -uint16_t GetPort(const absl::variant<url::SchemeHostPort, HostPortPair>& host) { +uint16_t GetPort(const HostResolver::Host& host) { if (absl::holds_alternative<url::SchemeHostPort>(host)) { return absl::get<url::SchemeHostPort>(host).port(); } @@ -533,7 +532,7 @@ uint16_t GetPort(const absl::variant<url::SchemeHostPort, HostPortPair>& host) { // (or the query is explicitly for HTTPS). Otherwise DNS will not give different // results for the same hostname. absl::variant<url::SchemeHostPort, std::string> CreateHostForJobKey( - const absl::variant<url::SchemeHostPort, HostPortPair>& input, + const HostResolver::Host& input, DnsQueryType query_type) { if ((base::FeatureList::IsEnabled(features::kUseDnsHttpsSvcb) || query_type == DnsQueryType::HTTPS) && @@ -605,7 +604,7 @@ class HostResolverManager::RequestImpl public base::LinkNode<HostResolverManager::RequestImpl> { public: RequestImpl(NetLogWithSource source_net_log, - absl::variant<url::SchemeHostPort, HostPortPair> request_host, + HostResolver::Host request_host, NetworkIsolationKey network_isolation_key, absl::optional<ResolveHostParameters> optional_parameters, base::WeakPtr<ResolveContext> resolve_context, @@ -780,9 +779,7 @@ class HostResolverManager::RequestImpl // NetLog for the source, passed in HostResolver::Resolve. const NetLogWithSource& source_net_log() { return source_net_log_; } - const absl::variant<url::SchemeHostPort, HostPortPair>& request_host() const { - return request_host_; - } + const HostResolver::Host& request_host() const { return request_host_; } const NetworkIsolationKey& network_isolation_key() const { return network_isolation_key_; @@ -850,19 +847,19 @@ class HostResolverManager::RequestImpl source_net_log_.BeginEvent( NetLogEventType::HOST_RESOLVER_MANAGER_REQUEST, [this] { - base::Value dict(base::Value::Type::DICTIONARY); - dict.SetKey("host", ToLogStringValue(request_host_)); - dict.SetIntKey("dns_query_type", - base::strict_cast<int>(parameters_.dns_query_type)); - dict.SetBoolKey("allow_cached_response", - parameters_.cache_usage != - ResolveHostParameters::CacheUsage::DISALLOWED); - dict.SetBoolKey("is_speculative", parameters_.is_speculative); - dict.SetStringKey("network_isolation_key", - network_isolation_key_.ToDebugString()); - dict.SetIntKey("secure_dns_policy", - static_cast<int>(parameters_.secure_dns_policy)); - return dict; + base::Value::Dict dict; + dict.Set("host", ToLogStringValue(request_host_)); + dict.Set("dns_query_type", + base::strict_cast<int>(parameters_.dns_query_type)); + dict.Set("allow_cached_response", + parameters_.cache_usage != + ResolveHostParameters::CacheUsage::DISALLOWED); + dict.Set("is_speculative", parameters_.is_speculative); + dict.Set("network_isolation_key", + network_isolation_key_.ToDebugString()); + dict.Set("secure_dns_policy", + base::strict_cast<int>(parameters_.secure_dns_policy)); + return base::Value(std::move(dict)); }); } @@ -890,7 +887,7 @@ class HostResolverManager::RequestImpl const NetLogWithSource source_net_log_; - const absl::variant<url::SchemeHostPort, HostPortPair> request_host_; + const HostResolver::Host request_host_; const NetworkIsolationKey network_isolation_key_; ResolveHostParameters parameters_; base::WeakPtr<ResolveContext> resolve_context_; @@ -1275,10 +1272,9 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { DCHECK(client_); DCHECK(delegate_); - if (secure_) - DCHECK(client_->CanUseSecureDnsTransactions()); - else + if (!secure_) { DCHECK(client_->CanUseInsecureDnsTransactions()); + } PushTransactionsNeeded(MaybeDisableAdditionalQueries(query_types)); } @@ -1363,7 +1359,7 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { base::Value::List transactions_needed_value; for (const TransactionInfo& info : transactions_needed_) { base::Value::Dict transaction_dict; - transaction_dict.Set("dns_query_type", static_cast<int>(info.type)); + transaction_dict.Set("dns_query_type", base::strict_cast<int>(info.type)); transactions_needed_value.Append(std::move(transaction_dict)); } dict.Set("transactions_needed", std::move(transactions_needed_value)); @@ -1378,7 +1374,8 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { base::Value::List list; for (const TransactionInfo& info : transactions_in_progress_) { base::Value::Dict transaction_dict; - transaction_dict.Set("dns_query_type", static_cast<int>(info.type)); + transaction_dict.Set("dns_query_type", + base::strict_cast<int>(info.type)); list.Append(std::move(transaction_dict)); } dict.Set("started_transactions", std::move(list)); @@ -1388,7 +1385,8 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { base::Value::List list; for (const TransactionInfo& info : transactions_needed_) { base::Value::Dict transaction_dict; - transaction_dict.Set("dns_query_type", static_cast<int>(info.type)); + transaction_dict.Set("dns_query_type", + base::strict_cast<int>(info.type)); list.Append(std::move(transaction_dict)); } dict.Set("queued_transactions", std::move(list)); @@ -1411,7 +1409,7 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { types.Remove(DnsQueryType::HTTPS); } else { DCHECK(!httpssvc_metrics_); - httpssvc_metrics_.emplace(/*expect_intact=*/false); + httpssvc_metrics_.emplace(secure_, /*expect_intact=*/false); } } @@ -1425,6 +1423,7 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { DCHECK(!httpssvc_metrics_) << "Caller requested multiple experimental types"; httpssvc_metrics_.emplace( + secure_, /*expect_intact=*/httpssvc_domain_cache_.IsExperimental( GetHostname(host_))); } @@ -1513,13 +1512,9 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { switch (transaction.type) { case DnsQueryType::INTEGRITY: DCHECK(httpssvc_metrics_); - // Don't record provider ID for timeouts. It is not precisely known - // at this level which provider is actually to blame for the - // timeout, and breaking metrics out by provider is no longer - // important for current experimentation goals. - httpssvc_metrics_->SaveForIntegrity( - /*doh_provider_id=*/absl::nullopt, HttpssvcDnsRcode::kTimedOut, - {}, elapsed_time); + httpssvc_metrics_->SaveForIntegrity(HttpssvcDnsRcode::kTimedOut, + /*condensed_records=*/{}, + elapsed_time); break; case DnsQueryType::HTTPS: DCHECK(!secure_ || @@ -1531,9 +1526,9 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { // at this level which provider is actually to blame for the // timeout, and breaking metrics out by provider is no longer // important for current experimentation goals. - httpssvc_metrics_->SaveForHttps( - /*doh_provider_id=*/absl::nullopt, HttpssvcDnsRcode::kTimedOut, - /*condensed_records=*/{}, elapsed_time); + httpssvc_metrics_->SaveForHttps(HttpssvcDnsRcode::kTimedOut, + /*condensed_records=*/{}, + elapsed_time); } break; default: @@ -1557,8 +1552,7 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { std::set<TransactionInfo>::iterator transaction_info_it, uint16_t request_port, int net_error, - const DnsResponse* response, - absl::optional<std::string> doh_provider_id) { + const DnsResponse* response) { DCHECK(transaction_info_it != transactions_in_progress_.end()); DCHECK(transactions_in_progress_.find(*transaction_info_it) != transactions_in_progress_.end()); @@ -1655,18 +1649,17 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> { CHECK(experimental_results); // INTEGRITY queries can time out the normal way (here), or when the // experimental query timer runs out (OnExperimentalQueryTimeout). - httpssvc_metrics_->SaveForIntegrity(doh_provider_id, rcode_for_httpssvc, - *experimental_results, - elapsed_time); + httpssvc_metrics_->SaveForIntegrity( + rcode_for_httpssvc, *experimental_results, elapsed_time); } else if (transaction_info.type == DnsQueryType::HTTPS || transaction_info.type == DnsQueryType::HTTPS_EXPERIMENTAL) { const std::vector<bool>* record_compatibility = results.https_record_compatibility(); CHECK(record_compatibility); - httpssvc_metrics_->SaveForHttps(doh_provider_id, rcode_for_httpssvc, + httpssvc_metrics_->SaveForHttps(rcode_for_httpssvc, *record_compatibility, elapsed_time); } else { - httpssvc_metrics_->SaveForAddressQuery(doh_provider_id, elapsed_time, + httpssvc_metrics_->SaveForAddressQuery(elapsed_time, rcode_for_httpssvc); } } @@ -2498,17 +2491,17 @@ class HostResolverManager::Job : public PrioritizedDispatcher::Job, private: base::Value NetLogJobCreationParams(const NetLogSource& source) { - base::Value dict(base::Value::Type::DICTIONARY); - source.AddToEventParameters(&dict); - dict.SetKey("host", ToLogStringValue(key_.host)); + base::Value::Dict dict; + source.AddToEventParameters(dict); + dict.Set("host", ToLogStringValue(key_.host)); std::vector<base::Value> query_types_list; for (DnsQueryType query_type : key_.query_types) query_types_list.emplace_back(kDnsQueryTypes.at(query_type)); - dict.SetKey("dns_query_types", base::Value(std::move(query_types_list))); - dict.SetIntKey("secure_dns_mode", static_cast<int>(key_.secure_dns_mode)); - dict.SetStringKey("network_isolation_key", - key_.network_isolation_key.ToDebugString()); - return dict; + dict.Set("dns_query_types", base::Value(std::move(query_types_list))); + dict.Set("secure_dns_mode", base::strict_cast<int>(key_.secure_dns_mode)); + dict.Set("network_isolation_key", + key_.network_isolation_key.ToDebugString()); + return base::Value(std::move(dict)); } void Finish() { @@ -3122,17 +3115,12 @@ HostResolverManager::HostResolverManager( SystemDnsConfigChangeNotifier* system_dns_config_notifier, NetworkChangeNotifier::NetworkHandle target_network, NetLog* net_log) - : max_queued_jobs_(0), - proc_params_(nullptr, options.max_system_retry_attempts), + : proc_params_(nullptr, options.max_system_retry_attempts), net_log_(net_log), system_dns_config_notifier_(system_dns_config_notifier), target_network_(target_network), check_ipv6_on_wifi_(options.check_ipv6_on_wifi), - last_ipv6_probe_result_(true), - additional_resolver_flags_(0), - allow_fallback_to_proctask_(true), - tick_clock_(base::DefaultTickClock::GetInstance()), - invalidation_in_progress_(false) { + tick_clock_(base::DefaultTickClock::GetInstance()) { PrioritizedDispatcher::Limits job_limits = GetDispatcherLimits(options); dispatcher_ = std::make_unique<PrioritizedDispatcher>(job_limits); max_queued_jobs_ = job_limits.total_jobs * 100u; @@ -3216,7 +3204,7 @@ HostResolverManager::CreateNetworkBoundHostResolverManager( std::unique_ptr<HostResolver::ResolveHostRequest> HostResolverManager::CreateRequest( - absl::variant<url::SchemeHostPort, HostPortPair> host, + HostResolver::Host host, NetworkIsolationKey network_isolation_key, NetLogWithSource net_log, absl::optional<ResolveHostParameters> optional_parameters, @@ -3226,12 +3214,9 @@ HostResolverManager::CreateRequest( DCHECK(!invalidation_in_progress_); DCHECK_EQ(resolve_context->GetTargetNetwork(), target_network_); - - // If required, ResolveContexts must register (via RegisterResolveContext()) - // before use to ensure cached data is invalidated on network and - // configuration changes. - DCHECK(!resolve_context->MustRegisterForInvalidations() || - registered_contexts_.HasObserver(resolve_context)); + // ResolveContexts must register (via RegisterResolveContext()) before use to + // ensure cached data is invalidated on network and configuration changes. + DCHECK(registered_contexts_.HasObserver(resolve_context)); return std::make_unique<RequestImpl>( std::move(net_log), std::move(host), std::move(network_isolation_key), @@ -3330,14 +3315,6 @@ void HostResolverManager::SetDnsConfigOverrides(DnsConfigOverrides overrides) { } void HostResolverManager::RegisterResolveContext(ResolveContext* context) { - // MustRegisterForInvalidations() == false for ResolveContext which have been - // bound to a network. This is currently implemented only for Android, where - // we receive DNS config changes only after a network change. - // If network binding is implemented for another platform where this is not - // the case, this will need to be revised. - if (!context->MustRegisterForInvalidations()) - return; - registered_contexts_.AddObserver(context); context->InvalidateCachesAndPerSessionData( dns_client_ ? dns_client_->GetCurrentSession() : nullptr, @@ -3346,27 +3323,12 @@ void HostResolverManager::RegisterResolveContext(ResolveContext* context) { void HostResolverManager::DeregisterResolveContext( const ResolveContext* context) { - if (context->MustRegisterForInvalidations()) - registered_contexts_.RemoveObserver(context); + registered_contexts_.RemoveObserver(context); // Destroy Jobs when their context is closed. RemoveAllJobs(context); } -void HostResolverManager::RemoveResolveContextRegistrationIfNeeded( - const ResolveContext* context) { - // Whether ResolveContext should register for notifications or not depends on - // ResolveContext::MustRegisterForNotifications. Ideally that would be an - // invariant for the entire lifetime of `context`, unfortunately it is not - // due to the current ResolveContexts creation procedure (their - // URLRequestContext is initially set to nullptr and only later it is - // updated). - if (!context->MustRegisterForInvalidations() && - registered_contexts_.HasObserver(context)) { - registered_contexts_.RemoveObserver(context); - } -} - void HostResolverManager::SetTickClockForTesting( const base::TickClock* tick_clock) { tick_clock_ = tick_clock; @@ -3889,8 +3851,9 @@ void HostResolverManager::PushDnsTasks(bool proc_task_allowed, case SecureDnsMode::kSecure: DCHECK(!allow_cache || out_tasks->front() == TaskType::SECURE_CACHE_LOOKUP); - DCHECK(dns_client_->CanUseSecureDnsTransactions()); - if (dns_tasks_allowed) + // Policy misconfiguration can put us in secure DNS mode without any DoH + // servers to query. See https://crbug.com/1326526. + if (dns_tasks_allowed && dns_client_->CanUseSecureDnsTransactions()) out_tasks->push_back(TaskType::SECURE_DNS); break; case SecureDnsMode::kAutomatic: diff --git a/chromium/net/dns/host_resolver_manager.h b/chromium/net/dns/host_resolver_manager.h index 9d6afe9428d..602a18b241a 100644 --- a/chromium/net/dns/host_resolver_manager.h +++ b/chromium/net/dns/host_resolver_manager.h @@ -147,7 +147,7 @@ class NET_EXPORT HostResolverManager // TODO(crbug.com/1022059): Use the HostCache out of the ResolveContext // instead of passing it separately. std::unique_ptr<HostResolver::ResolveHostRequest> CreateRequest( - absl::variant<url::SchemeHostPort, HostPortPair> host, + HostResolver::Host host, NetworkIsolationKey network_isolation_key, NetLogWithSource net_log, absl::optional<ResolveHostParameters> optional_parameters, @@ -191,12 +191,6 @@ class NET_EXPORT HostResolverManager void RegisterResolveContext(ResolveContext* context); void DeregisterResolveContext(const ResolveContext* context); - // ContextHostResolvers should call this after associating a ResolveContext - // with a URLRequestContext. - // TODO(stefanoduo): Remove once MustRegisterForInvalidations becomes an - // invariant. - void RemoveResolveContextRegistrationIfNeeded(const ResolveContext* context); - void set_proc_params_for_test(const ProcTaskParams& proc_params) { proc_params_ = proc_params; } @@ -243,6 +237,10 @@ class NET_EXPORT HostResolverManager bool check_ipv6_on_wifi_for_testing() const { return check_ipv6_on_wifi_; } + NetworkChangeNotifier::NetworkHandle target_network_for_testing() const { + return target_network_; + } + // Public to be called from std::make_unique. Not to be called directly. HostResolverManager(base::PassKey<HostResolverManager>, const HostResolver::ManagerOptions& options, @@ -500,7 +498,7 @@ class NET_EXPORT HostResolverManager std::unique_ptr<PrioritizedDispatcher> dispatcher_; // Limit on the maximum number of jobs queued in |dispatcher_|. - size_t max_queued_jobs_; + size_t max_queued_jobs_ = 0; // Parameters for ProcTask. ProcTaskParams proc_params_; @@ -519,13 +517,13 @@ class NET_EXPORT HostResolverManager bool check_ipv6_on_wifi_; base::TimeTicks last_ipv6_probe_time_; - bool last_ipv6_probe_result_; + bool last_ipv6_probe_result_ = true; // Any resolver flags that should be added to a request by default. - HostResolverFlags additional_resolver_flags_; + HostResolverFlags additional_resolver_flags_ = 0; // Allow fallback to ProcTask if DnsTask fails. - bool allow_fallback_to_proctask_; + bool allow_fallback_to_proctask_ = true; // Task runner used for DNS lookups using the system resolver. Normally a // ThreadPool task runner, but can be overridden for tests. @@ -542,7 +540,7 @@ class NET_EXPORT HostResolverManager true /* check_empty */, false /* allow_reentrancy */> registered_contexts_; - bool invalidation_in_progress_; + bool invalidation_in_progress_ = false; // Helper for metrics associated with `features::kDnsHttpssvc`. HttpssvcExperimentDomainCache httpssvc_domain_cache_; diff --git a/chromium/net/dns/host_resolver_manager_unittest.cc b/chromium/net/dns/host_resolver_manager_unittest.cc index 95ec7970497..9b62694634e 100644 --- a/chromium/net/dns/host_resolver_manager_unittest.cc +++ b/chromium/net/dns/host_resolver_manager_unittest.cc @@ -144,8 +144,6 @@ class MockHostResolverProc : public HostResolverProc { MockHostResolverProc() : HostResolverProc(nullptr), - num_requests_waiting_(0), - num_slots_available_(0), requests_waiting_(&lock_), slots_available_(&lock_) {} @@ -273,8 +271,8 @@ class MockHostResolverProc : public HostResolverProc { mutable base::Lock lock_; std::map<ResolveKey, AddressList> rules_; CaptureList capture_list_; - unsigned num_requests_waiting_; - unsigned num_slots_available_; + unsigned num_requests_waiting_ = 0; + unsigned num_slots_available_ = 0; base::ConditionVariable requests_waiting_; base::ConditionVariable slots_available_; }; @@ -358,11 +356,7 @@ class LookupAttemptHostResolverProc : public HostResolverProc { int total_attempts) : HostResolverProc(previous), attempt_number_to_resolve_(attempt_number_to_resolve), - current_attempt_number_(0), total_attempts_(total_attempts), - total_attempts_resolved_(0), - resolved_attempt_number_(0), - num_attempts_waiting_(0), all_done_(&lock_), blocked_attempt_signal_(&lock_) {} @@ -458,11 +452,11 @@ class LookupAttemptHostResolverProc : public HostResolverProc { private: int attempt_number_to_resolve_; - int current_attempt_number_; // Incremented whenever Resolve is called. + int current_attempt_number_ = 0; // Incremented whenever Resolve is called. int total_attempts_; - int total_attempts_resolved_; - int resolved_attempt_number_; - int num_attempts_waiting_; + int total_attempts_resolved_ = 0; + int resolved_attempt_number_ = 0; + int num_attempts_waiting_ = 0; // All attempts wait for right attempt to be resolve. base::Lock lock_; @@ -4189,8 +4183,7 @@ class HostResolverManagerDnsTest : public HostResolverManagerTest { base::test::TaskEnvironment::TimeSource::MOCK_TIME) : HostResolverManagerTest(time_source), notifier_task_runner_( - base::MakeRefCounted<base::TestMockTimeTaskRunner>()), - dns_client_(nullptr) { + base::MakeRefCounted<base::TestMockTimeTaskRunner>()) { auto config_service = std::make_unique<TestDnsConfigService>(); config_service_ = config_service.get(); notifier_ = std::make_unique<SystemDnsConfigChangeNotifier>( @@ -4473,7 +4466,7 @@ class HostResolverManagerDnsTest : public HostResolverManagerTest { std::unique_ptr<SystemDnsConfigChangeNotifier> notifier_; // Owned by |resolver_|. - raw_ptr<MockDnsClient> dns_client_; + raw_ptr<MockDnsClient> dns_client_ = nullptr; }; TEST_F(HostResolverManagerDnsTest, FlushCacheOnDnsConfigChange) { @@ -8023,6 +8016,9 @@ TEST_F(HostResolverManagerDnsTest, SetDnsConfigOverrides) { const std::vector<IPEndPoint> nameservers = { CreateExpected("192.168.0.1", 92)}; overrides.nameservers = nameservers; + overrides.dns_over_tls_active = true; + const std::string dns_over_tls_hostname = "dns.example.com"; + overrides.dns_over_tls_hostname = dns_over_tls_hostname; const std::vector<std::string> search = {"str"}; overrides.search = search; overrides.append_to_multi_label_name = false; @@ -8053,6 +8049,8 @@ TEST_F(HostResolverManagerDnsTest, SetDnsConfigOverrides) { const DnsConfig* overridden_config = client_ptr->GetEffectiveConfig(); ASSERT_TRUE(overridden_config); EXPECT_EQ(nameservers, overridden_config->nameservers); + EXPECT_TRUE(overridden_config->dns_over_tls_active); + EXPECT_EQ(dns_over_tls_hostname, overridden_config->dns_over_tls_hostname); EXPECT_EQ(search, overridden_config->search); EXPECT_FALSE(overridden_config->append_to_multi_label_name); EXPECT_EQ(ndots, overridden_config->ndots); @@ -8303,7 +8301,7 @@ TEST_F(HostResolverManagerDnsTest, DohMapping) { const DnsConfig* fetched_config = client_ptr->GetEffectiveConfig(); EXPECT_EQ(original_config.nameservers, fetched_config->nameservers); - auto expected_doh_config = *DnsOverHttpsConfig::FromStrings( + auto expected_doh_config = *DnsOverHttpsConfig::FromTemplatesForTesting( {"https://chrome.cloudflare-dns.com/dns-query", "https://doh.cleanbrowsing.org/doh/family-filter{?dns}", "https://doh.cleanbrowsing.org/doh/security-filter{?dns}"}); @@ -8462,7 +8460,7 @@ TEST_F(HostResolverManagerDnsTest, DohMappingWithAutomaticDot) { const DnsConfig* fetched_config = client_ptr->GetEffectiveConfig(); EXPECT_EQ(original_config.nameservers, fetched_config->nameservers); - auto expected_doh_config = *DnsOverHttpsConfig::FromStrings( + auto expected_doh_config = *DnsOverHttpsConfig::FromTemplatesForTesting( {"https://chrome.cloudflare-dns.com/dns-query", "https://doh.cleanbrowsing.org/doh/family-filter{?dns}", "https://doh.cleanbrowsing.org/doh/security-filter{?dns}"}); diff --git a/chromium/net/dns/host_resolver_proc.cc b/chromium/net/dns/host_resolver_proc.cc index a205f6280c5..d43d82ef4e4 100644 --- a/chromium/net/dns/host_resolver_proc.cc +++ b/chromium/net/dns/host_resolver_proc.cc @@ -251,15 +251,11 @@ int SystemHostResolverProc::Resolve(const std::string& hostname, SystemHostResolverProc::~SystemHostResolverProc() = default; -const base::TimeDelta ProcTaskParams::kDnsDefaultUnresponsiveDelay = - base::Seconds(6); - ProcTaskParams::ProcTaskParams(HostResolverProc* resolver_proc, size_t in_max_retry_attempts) : resolver_proc(resolver_proc), max_retry_attempts(in_max_retry_attempts), - unresponsive_delay(kDnsDefaultUnresponsiveDelay), - retry_factor(2) { + unresponsive_delay(kDnsDefaultUnresponsiveDelay) { // Maximum of 4 retry attempts for host resolution. static const size_t kDefaultMaxRetryAttempts = 4u; if (max_retry_attempts == HostResolver::ManagerOptions::kDefaultRetryAttempts) diff --git a/chromium/net/dns/host_resolver_proc.h b/chromium/net/dns/host_resolver_proc.h index cff9f64baaf..9d33547a344 100644 --- a/chromium/net/dns/host_resolver_proc.h +++ b/chromium/net/dns/host_resolver_proc.h @@ -154,7 +154,8 @@ class NET_EXPORT_PRIVATE SystemHostResolverProc : public HostResolverProc { struct NET_EXPORT_PRIVATE ProcTaskParams { // Default delay between calls to the system resolver for the same hostname. // (Can be overridden by field trial.) - static const base::TimeDelta kDnsDefaultUnresponsiveDelay; + static constexpr base::TimeDelta kDnsDefaultUnresponsiveDelay = + base::Seconds(6); // Sets up defaults. ProcTaskParams(HostResolverProc* resolver_proc, size_t max_retry_attempts); @@ -174,10 +175,10 @@ struct NET_EXPORT_PRIVATE ProcTaskParams { // This is the limit after which we make another attempt to resolve the host // if the worker thread has not responded yet. - base::TimeDelta unresponsive_delay; + base::TimeDelta unresponsive_delay = kDnsDefaultUnresponsiveDelay; // Factor to grow |unresponsive_delay| when we re-re-try. - uint32_t retry_factor; + uint32_t retry_factor = 2; }; } // namespace net diff --git a/chromium/net/dns/httpssvc_metrics.cc b/chromium/net/dns/httpssvc_metrics.cc index c60b0925b25..c29e6a8b3c0 100644 --- a/chromium/net/dns/httpssvc_metrics.cc +++ b/chromium/net/dns/httpssvc_metrics.cc @@ -8,8 +8,8 @@ #include "base/metrics/histogram.h" #include "base/metrics/histogram_base.h" #include "base/metrics/histogram_functions.h" -#include "base/metrics/histogram_macros.h" #include "base/notreached.h" +#include "base/numerics/clamped_math.h" #include "base/strings/string_split.h" #include "base/strings/string_util.h" #include "net/base/features.h" @@ -68,19 +68,15 @@ bool HttpssvcExperimentDomainCache::IsControl(base::StringPiece domain) { control_list_); } -HttpssvcMetrics::HttpssvcMetrics(bool expect_intact) - : expect_intact_(expect_intact) {} +HttpssvcMetrics::HttpssvcMetrics(bool secure, bool expect_intact) + : secure_(secure), expect_intact_(expect_intact) {} HttpssvcMetrics::~HttpssvcMetrics() { RecordMetrics(); } -void HttpssvcMetrics::SaveForAddressQuery( - absl::optional<std::string> new_doh_provider_id, - base::TimeDelta resolve_time, - enum HttpssvcDnsRcode rcode) { - set_doh_provider_id(new_doh_provider_id); - +void HttpssvcMetrics::SaveForAddressQuery(base::TimeDelta resolve_time, + enum HttpssvcDnsRcode rcode) { address_resolve_times_.push_back(resolve_time); if (rcode != HttpssvcDnsRcode::kNoError) @@ -92,13 +88,10 @@ void HttpssvcMetrics::SaveAddressQueryFailure() { } void HttpssvcMetrics::SaveForIntegrity( - absl::optional<std::string> new_doh_provider_id, enum HttpssvcDnsRcode rcode_integrity, const std::vector<bool>& condensed_records, base::TimeDelta integrity_resolve_time) { DCHECK(!rcode_integrity_.has_value()); - set_doh_provider_id(new_doh_provider_id); - rcode_integrity_ = rcode_integrity; num_integrity_records_ = condensed_records.size(); @@ -117,13 +110,10 @@ void HttpssvcMetrics::SaveForIntegrity( integrity_resolve_time_ = integrity_resolve_time; } -void HttpssvcMetrics::SaveForHttps(absl::optional<std::string> doh_provider_id, - enum HttpssvcDnsRcode rcode, +void HttpssvcMetrics::SaveForHttps(enum HttpssvcDnsRcode rcode, const std::vector<bool>& condensed_records, base::TimeDelta https_resolve_time) { DCHECK(!rcode_https_.has_value()); - set_doh_provider_id(doh_provider_id); - rcode_https_ = rcode; num_https_records_ = condensed_records.size(); @@ -142,44 +132,21 @@ void HttpssvcMetrics::SaveForHttps(absl::optional<std::string> doh_provider_id, https_resolve_time_ = https_resolve_time; } -void HttpssvcMetrics::set_doh_provider_id( - absl::optional<std::string> new_doh_provider_id) { - // "Other" never gets updated. - if (doh_provider_id_.has_value() && *doh_provider_id_ == "Other") - return; - - // If provider IDs mismatch, downgrade the new provider ID to "Other". - if ((doh_provider_id_.has_value() && !new_doh_provider_id.has_value()) || - (doh_provider_id_.has_value() && new_doh_provider_id.has_value() && - *doh_provider_id_ != *new_doh_provider_id)) { - new_doh_provider_id = "Other"; - } - - doh_provider_id_ = new_doh_provider_id; -} - std::string HttpssvcMetrics::BuildMetricName( RecordType type, base::StringPiece leaf_name) const { // Build shared pieces of the metric names. - base::StringPiece type_str; - switch (type) { - case RecordType::kIntegrity: - type_str = "RecordIntegrity"; - break; - case RecordType::kHttps: - type_str = "RecordHttps"; - break; - } - const base::StringPiece expectation = + CHECK(type == RecordType::kHttps || type == RecordType::kIntegrity); + base::StringPiece type_str = + type == RecordType::kHttps ? "RecordHttps" : "RecordIntegrity"; + base::StringPiece secure = secure_ ? "Secure" : "Insecure"; + base::StringPiece expectation = expect_intact_ ? "ExpectIntact" : "ExpectNoerror"; - const std::string provider_id = doh_provider_id_.value_or("Other"); // Example INTEGRITY metric name: - // Net.DNS.HTTPSSVC.RecordIntegrity.CleanBrowsingAdult.ExpectIntact.DnsRcode - return base::JoinString({"Net.DNS.HTTPSSVC", type_str, provider_id.c_str(), - expectation, leaf_name}, - "."); + // Net.DNS.HTTPSSVC.RecordIntegrity.Secure.ExpectIntact.DnsRcode + return base::JoinString( + {"Net.DNS.HTTPSSVC", type_str, secure, expectation, leaf_name}, "."); } void HttpssvcMetrics::RecordMetrics() { @@ -221,21 +188,21 @@ void HttpssvcMetrics::RecordCommonMetrics() { https_resolve_time_.has_value()); if (integrity_resolve_time_.has_value()) { base::UmaHistogramMediumTimes( - BuildMetricName(RecordType::kIntegrity, "ResolveTimeIntegrityRecord"), + BuildMetricName(RecordType::kIntegrity, "ResolveTimeExperimental"), *integrity_resolve_time_); } if (https_resolve_time_.has_value()) { base::UmaHistogramMediumTimes( - BuildMetricName(RecordType::kHttps, "ResolveTimeHttpsRecord"), + BuildMetricName(RecordType::kHttps, "ResolveTimeExperimental"), *https_resolve_time_); } DCHECK(!address_resolve_times_.empty()); // Not specific to INTEGRITY or HTTPS, but for the sake of picking one for the // metric name and only recording the time once, always record the address - // resolve times under `kIntegrity`. + // resolve times under `kHttps`. const std::string kMetricResolveTimeAddressRecord = - BuildMetricName(RecordType::kIntegrity, "ResolveTimeNonIntegrityRecord"); + BuildMetricName(RecordType::kHttps, "ResolveTimeAddress"); for (base::TimeDelta resolve_time_other : address_resolve_times_) { base::UmaHistogramMediumTimes(kMetricResolveTimeAddressRecord, resolve_time_other); @@ -282,6 +249,19 @@ void HttpssvcMetrics::RecordCommonMetrics() { BuildMetricName(RecordType::kHttps, "ResolveTimeRatio"), resolve_time_percent / kPercentScale, kMaxRatio); } + + if (num_https_records_ > 0) { + DCHECK(rcode_https_.has_value()); + if (*rcode_https_ == HttpssvcDnsRcode::kNoError) { + base::UmaHistogramBoolean(BuildMetricName(RecordType::kHttps, "Parsable"), + is_https_parsable_.value_or(false)); + } else { + // Record boolean indicating whether we received an HTTPS record and + // an error simultaneously. + base::UmaHistogramBoolean( + BuildMetricName(RecordType::kHttps, "RecordWithError"), true); + } + } } void HttpssvcMetrics::RecordExpectIntactMetrics() { @@ -313,18 +293,6 @@ void HttpssvcMetrics::RecordExpectIntactMetrics() { BuildMetricName(RecordType::kIntegrity, "RecordWithError"), true); } } - if (num_https_records_ > 0) { - DCHECK(rcode_https_.has_value()); - if (*rcode_https_ == HttpssvcDnsRcode::kNoError) { - base::UmaHistogramBoolean(BuildMetricName(RecordType::kHttps, "Parsable"), - is_https_parsable_.value_or(false)); - } else { - // Record boolean indicating whether we received an HTTPS record and - // an error simultaneously. - base::UmaHistogramBoolean( - BuildMetricName(RecordType::kHttps, "RecordWithError"), true); - } - } } void HttpssvcMetrics::RecordExpectNoerrorMetrics() { @@ -343,24 +311,6 @@ void HttpssvcMetrics::RecordExpectNoerrorMetrics() { base::UmaHistogramBoolean( BuildMetricName(RecordType::kIntegrity, "RecordReceived"), true); } - - // HTTPS records received for expect-noerror domains are actual in-the-wild - // records not specific to Chrome experiments. Record some extra metrics on - // seen records, but not broken out by DNS provider. - if (num_https_records_ > 0) { - if (*rcode_https_ == HttpssvcDnsRcode::kNoError) { - UMA_HISTOGRAM_BOOLEAN( - "Net.DNS.HTTPSSVC.RecordHttps.AnyProvider.ExpectNoerror.Parsable", - is_https_parsable_.value_or(false)); - } else { - // Record boolean indicating whether we received an HTTPS record and - // an error simultaneously. - UMA_HISTOGRAM_BOOLEAN( - "Net.DNS.HTTPSSVC.RecordHttps.AnyProvider.ExpectNoerror." - "RecordWithError", - true); - } - } } } // namespace net diff --git a/chromium/net/dns/httpssvc_metrics.h b/chromium/net/dns/httpssvc_metrics.h index cebab812c20..97416c9ff4e 100644 --- a/chromium/net/dns/httpssvc_metrics.h +++ b/chromium/net/dns/httpssvc_metrics.h @@ -62,14 +62,13 @@ enum HttpssvcDnsRcode TranslateDnsRcodeForHttpssvcExperiment(uint8_t rcode); // the Save* methods. Records metrics to UMA on destruction. class NET_EXPORT_PRIVATE HttpssvcMetrics { public: - explicit HttpssvcMetrics(bool expect_intact); + HttpssvcMetrics(bool secure, bool expect_intact); ~HttpssvcMetrics(); HttpssvcMetrics(HttpssvcMetrics&) = delete; HttpssvcMetrics(HttpssvcMetrics&&) = delete; // May be called many times. - void SaveForAddressQuery(absl::optional<std::string> doh_provider_id, - base::TimeDelta resolve_time, + void SaveForAddressQuery(base::TimeDelta resolve_time, enum HttpssvcDnsRcode rcode); // Save the fact that the non-integrity queries failed. Prevents metrics from @@ -77,12 +76,10 @@ class NET_EXPORT_PRIVATE HttpssvcMetrics { void SaveAddressQueryFailure(); // Must only be called once. - void SaveForIntegrity(absl::optional<std::string> doh_provider_id, - enum HttpssvcDnsRcode rcode, + void SaveForIntegrity(enum HttpssvcDnsRcode rcode, const std::vector<bool>& condensed_records, base::TimeDelta integrity_resolve_time); - void SaveForHttps(absl::optional<std::string> doh_provider_id, - enum HttpssvcDnsRcode rcode, + void SaveForHttps(enum HttpssvcDnsRcode rcode, const std::vector<bool>& condensed_records, base::TimeDelta https_resolve_time); @@ -98,13 +95,11 @@ class NET_EXPORT_PRIVATE HttpssvcMetrics { void RecordExpectIntactMetrics(); void RecordExpectNoerrorMetrics(); - void set_doh_provider_id(absl::optional<std::string> doh_provider_id); - + const bool secure_; const bool expect_intact_; // RecordIntegrityMetrics() will do nothing when |disqualified_| is true. bool disqualified_ = false; bool already_recorded_ = false; - absl::optional<std::string> doh_provider_id_; absl::optional<enum HttpssvcDnsRcode> rcode_integrity_; absl::optional<enum HttpssvcDnsRcode> rcode_https_; size_t num_integrity_records_ = 0; diff --git a/chromium/net/dns/httpssvc_metrics_unittest.cc b/chromium/net/dns/httpssvc_metrics_unittest.cc index 425758199b7..724d4bad5bd 100644 --- a/chromium/net/dns/httpssvc_metrics_unittest.cc +++ b/chromium/net/dns/httpssvc_metrics_unittest.cc @@ -16,6 +16,7 @@ #include "base/test/scoped_feature_list.h" #include "net/base/features.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" namespace net { @@ -164,10 +165,11 @@ INSTANTIATE_TEST_SUITE_P( testing::Values(false) /* DnsHttpssvcControlDomainWildcard */))); // Base for testing the metrics collection code in |HttpssvcMetrics|. -class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { +class HttpssvcMetricsTest + : public ::testing::TestWithParam<std::tuple<bool, bool>> { public: void SetUp() override { - querying_experimental_ = GetParam(); + std::tie(secure_, querying_experimental_) = GetParam(); config_ = HttpssvcFeatureConfig( {true /* enabled */, true /* use_integrity */, true /* use_httpssvc */, false /* control_domain_wildcard */}, @@ -178,7 +180,7 @@ class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { std::string BuildMetricNamePrefix(base::StringPiece record_type_str, base::StringPiece expect_str) const { return base::StrCat({"Net.DNS.HTTPSSVC.", record_type_str, ".", - doh_provider_, ".", expect_str, "."}); + secure_ ? "Secure." : "Insecure.", expect_str, "."}); } template <typename T> @@ -201,11 +203,11 @@ class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { absl::optional<base::TimeDelta> expect_intact_time = absl::nullopt, absl::optional<base::TimeDelta> expect_noerror_time = absl::nullopt) { const std::string kExpectIntact = - base::StrCat({BuildMetricNamePrefix("RecordIntegrity", "ExpectIntact"), - "ResolveTimeNonIntegrityRecord"}); + base::StrCat({BuildMetricNamePrefix("RecordHttps", "ExpectIntact"), + "ResolveTimeAddress"}); const std::string kExpectNoerror = - base::StrCat({BuildMetricNamePrefix("RecordIntegrity", "ExpectNoerror"), - "ResolveTimeNonIntegrityRecord"}); + base::StrCat({BuildMetricNamePrefix("RecordHttps", "ExpectNoerror"), + "ResolveTimeAddress"}); ExpectSample(kExpectIntact, expect_intact_time); ExpectSample(kExpectNoerror, expect_noerror_time); @@ -223,15 +225,15 @@ class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { const std::string kMetricIntegrity = base::StrCat({kPrefix, "Integrity"}); const std::string kMetricRecordWithError = base::StrCat({kPrefix, "RecordWithError"}); - const std::string kMetricResolveTimeIntegrity = - base::StrCat({kPrefix, "ResolveTimeIntegrityRecord"}); + const std::string kMetricResolveTimeExperimental = + base::StrCat({kPrefix, "ResolveTimeExperimental"}); const std::string kMetricResolveTimeRatio = base::StrCat({kPrefix, "ResolveTimeRatio"}); ExpectSample(kMetricDnsRcode, rcode); ExpectSample(kMetricIntegrity, integrity); ExpectSample(kMetricRecordWithError, record_with_error); - ExpectSample(kMetricResolveTimeIntegrity, resolve_time_integrity); + ExpectSample(kMetricResolveTimeExperimental, resolve_time_integrity); ExpectSample(kMetricResolveTimeRatio, resolve_time_ratio); } @@ -247,15 +249,15 @@ class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { const std::string kMetricParsable = base::StrCat({kPrefix, "Parsable"}); const std::string kMetricRecordWithError = base::StrCat({kPrefix, "RecordWithError"}); - const std::string kMetricResolveTimeHttps = - base::StrCat({kPrefix, "ResolveTimeHttpsRecord"}); + const std::string kMetricResolveTimeExperimental = + base::StrCat({kPrefix, "ResolveTimeExperimental"}); const std::string kMetricResolveTimeRatio = base::StrCat({kPrefix, "ResolveTimeRatio"}); ExpectSample(kMetricDnsRcode, rcode); ExpectSample(kMetricParsable, parsable); ExpectSample(kMetricRecordWithError, record_with_error); - ExpectSample(kMetricResolveTimeHttps, resolve_time_https); + ExpectSample(kMetricResolveTimeExperimental, resolve_time_https); ExpectSample(kMetricResolveTimeRatio, resolve_time_ratio); } @@ -269,14 +271,14 @@ class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { const std::string kMetricDnsRcode = base::StrCat({kPrefix, "DnsRcode"}); const std::string kMetricRecordReceived = base::StrCat({kPrefix, "RecordReceived"}); - const std::string kMetricResolveTimeIntegrity = - base::StrCat({kPrefix, "ResolveTimeIntegrityRecord"}); + const std::string kMetricResolveTimeExperimental = + base::StrCat({kPrefix, "ResolveTimeExperimental"}); const std::string kMetricResolveTimeRatio = base::StrCat({kPrefix, "ResolveTimeRatio"}); ExpectSample(kMetricDnsRcode, rcode); ExpectSample(kMetricRecordReceived, record_received); - ExpectSample(kMetricResolveTimeIntegrity, resolve_time_integrity); + ExpectSample(kMetricResolveTimeExperimental, resolve_time_integrity); ExpectSample(kMetricResolveTimeRatio, resolve_time_ratio); } @@ -289,20 +291,18 @@ class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { const std::string kPrefix = BuildMetricNamePrefix("RecordHttps", "ExpectNoerror"); const std::string kMetricDnsRcode = base::StrCat({kPrefix, "DnsRcode"}); - const std::string kMetricParsable = - "Net.DNS.HTTPSSVC.RecordHttps.AnyProvider.ExpectNoerror.Parsable"; + const std::string kMetricParsable = base::StrCat({kPrefix, "Parsable"}); const std::string kMetricRecordWithError = - "Net.DNS.HTTPSSVC.RecordHttps.AnyProvider.ExpectNoerror." - "RecordWithError"; - const std::string kMetricResolveTimeHttps = - base::StrCat({kPrefix, "ResolveTimeHttpsRecord"}); + base::StrCat({kPrefix, "RecordWithError"}); + const std::string kMetricResolveTimeExperimental = + base::StrCat({kPrefix, "ResolveTimeExperimental"}); const std::string kMetricResolveTimeRatio = base::StrCat({kPrefix, "ResolveTimeRatio"}); ExpectSample(kMetricDnsRcode, rcode); ExpectSample(kMetricParsable, parsable); ExpectSample(kMetricRecordWithError, record_with_error); - ExpectSample(kMetricResolveTimeHttps, resolve_time_https); + ExpectSample(kMetricResolveTimeExperimental, resolve_time_https); ExpectSample(kMetricResolveTimeRatio, resolve_time_ratio); } @@ -321,22 +321,25 @@ class HttpssvcMetricsTest : public ::testing::TestWithParam<bool> { const HttpssvcFeatureConfig& config() const { return config_; } protected: + bool secure_; bool querying_experimental_; private: HttpssvcFeatureConfig config_; base::test::ScopedFeatureList scoped_feature_list_; base::HistogramTester histogram_; - std::string doh_provider_ = "Other"; }; // This instantiation focuses on whether the correct metrics are recorded. The // domain list parser is already tested against encoding quirks in // |HttpssvcMetricsTestDomainParsing|, so we fix the quirks at false. -INSTANTIATE_TEST_SUITE_P(HttpssvcMetricsTestSimple, - HttpssvcMetricsTest, - // Whether we are querying an experimental domain. - testing::Bool()); +INSTANTIATE_TEST_SUITE_P( + HttpssvcMetricsTestSimple, + HttpssvcMetricsTest, + testing::Combine( + testing::Bool(), // Querying over DoH or Do53. + testing::Bool() // Whether we are querying an experimental domain. + )); TEST_P(HttpssvcDomainParsingTest, ParseFeatureParamIntegrityDomains) { HttpssvcExperimentDomainCache domain_cache; @@ -378,9 +381,9 @@ TEST_P(HttpssvcDomainParsingTest, ParseFeatureParamIntegrityDomains) { // Only record metrics for a non-integrity query. TEST_P(HttpssvcMetricsTest, AddressAndExperimentalMissing) { const base::TimeDelta kResolveTime = base::Milliseconds(10); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyAddressResolveTimeMetric(); @@ -393,11 +396,11 @@ TEST_P(HttpssvcMetricsTest, AddressAndExperimentalMissing) { TEST_P(HttpssvcMetricsTest, AddressAndIntegrityIntact) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeIntegrity = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForIntegrity(absl::nullopt, HttpssvcDnsRcode::kNoError, {true}, + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForIntegrity(HttpssvcDnsRcode::kNoError, {true}, kResolveTimeIntegrity); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyHttpsMetricsForExpectIntact(); @@ -428,11 +431,10 @@ TEST_P(HttpssvcMetricsTest, AddressAndIntegrityIntact) { TEST_P(HttpssvcMetricsTest, AddressAndHttpsParsable) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeHttps = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForHttps(absl::nullopt, HttpssvcDnsRcode::kNoError, {true}, - kResolveTimeHttps); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForHttps(HttpssvcDnsRcode::kNoError, {true}, kResolveTimeHttps); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyIntegrityMetricsForExpectIntact(); @@ -465,13 +467,12 @@ TEST_P(HttpssvcMetricsTest, AddressAndIntegrityIntactAndHttpsParsable) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeIntegrity = base::Milliseconds(15); const base::TimeDelta kResolveTimeHttps = base::Milliseconds(20); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForIntegrity(absl::nullopt, HttpssvcDnsRcode::kNoError, {true}, + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForIntegrity(HttpssvcDnsRcode::kNoError, {true}, kResolveTimeIntegrity); - metrics->SaveForHttps(absl::nullopt, HttpssvcDnsRcode::kNoError, {true}, - kResolveTimeHttps); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + metrics->SaveForHttps(HttpssvcDnsRcode::kNoError, {true}, kResolveTimeHttps); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. if (querying_experimental_) { @@ -514,11 +515,11 @@ TEST_P(HttpssvcMetricsTest, AddressAndIntegrityMissingWithRcode) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeIntegrity = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForIntegrity(absl::nullopt, HttpssvcDnsRcode::kNxDomain, {}, + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForIntegrity(HttpssvcDnsRcode::kNxDomain, {}, kResolveTimeIntegrity); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyHttpsMetricsForExpectIntact(); @@ -553,11 +554,10 @@ TEST_P(HttpssvcMetricsTest, AddressAndHttpsMissingWithRcode) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeHttps = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForHttps(absl::nullopt, HttpssvcDnsRcode::kNxDomain, {}, - kResolveTimeHttps); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForHttps(HttpssvcDnsRcode::kNxDomain, {}, kResolveTimeHttps); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyIntegrityMetricsForExpectIntact(); @@ -592,11 +592,11 @@ TEST_P(HttpssvcMetricsTest, AddressAndIntegrityIntactWithRcode) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeIntegrity = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForIntegrity(absl::nullopt, HttpssvcDnsRcode::kNxDomain, {true}, + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForIntegrity(HttpssvcDnsRcode::kNxDomain, {true}, kResolveTimeIntegrity); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyHttpsMetricsForExpectIntact(); @@ -632,11 +632,10 @@ TEST_P(HttpssvcMetricsTest, AddressAndHttpsParsableWithRcode) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeHttps = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForHttps(absl::nullopt, HttpssvcDnsRcode::kNxDomain, {true}, - kResolveTimeHttps); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForHttps(HttpssvcDnsRcode::kNxDomain, {true}, kResolveTimeHttps); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyIntegrityMetricsForExpectIntact(); @@ -673,11 +672,11 @@ TEST_P(HttpssvcMetricsTest, AddressAndHttpsParsableWithRcode) { TEST_P(HttpssvcMetricsTest, AddressAndIntegrityMangledWithRcode) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeIntegrity = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForIntegrity(absl::nullopt, HttpssvcDnsRcode::kNxDomain, {false}, + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForIntegrity(HttpssvcDnsRcode::kNxDomain, {false}, kResolveTimeIntegrity); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyHttpsMetricsForExpectIntact(); @@ -712,11 +711,11 @@ TEST_P(HttpssvcMetricsTest, AddressAndIntegrityMangledWithRcode) { TEST_P(HttpssvcMetricsTest, AddressAndHttpsMangledWithRcode) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeHttps = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForHttps(absl::nullopt, HttpssvcDnsRcode::kNxDomain, {false}, + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForHttps(HttpssvcDnsRcode::kNxDomain, {false}, kResolveTimeHttps); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyIntegrityMetricsForExpectIntact(); @@ -753,11 +752,11 @@ TEST_P(HttpssvcMetricsTest, AddressAndHttpsMangledWithRcode) { TEST_P(HttpssvcMetricsTest, AddressAndIntegrityTimedOut) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeIntegrity = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForIntegrity(absl::nullopt, HttpssvcDnsRcode::kTimedOut, {}, + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForIntegrity(HttpssvcDnsRcode::kTimedOut, {}, kResolveTimeIntegrity); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyHttpsMetricsForExpectIntact(); @@ -792,11 +791,10 @@ TEST_P(HttpssvcMetricsTest, AddressAndIntegrityTimedOut) { TEST_P(HttpssvcMetricsTest, AddressAndHttpsTimedOut) { const base::TimeDelta kResolveTime = base::Milliseconds(10); const base::TimeDelta kResolveTimeHttps = base::Milliseconds(15); - absl::optional<HttpssvcMetrics> metrics(querying_experimental_); - metrics->SaveForHttps(absl::nullopt, HttpssvcDnsRcode::kTimedOut, {}, - kResolveTimeHttps); - metrics->SaveForAddressQuery(absl::nullopt, kResolveTime, - HttpssvcDnsRcode::kNoError); + auto metrics = + absl::make_optional<HttpssvcMetrics>(secure_, querying_experimental_); + metrics->SaveForHttps(HttpssvcDnsRcode::kTimedOut, {}, kResolveTimeHttps); + metrics->SaveForAddressQuery(kResolveTime, HttpssvcDnsRcode::kNoError); metrics.reset(); // Record the metrics to UMA. VerifyIntegrityMetricsForExpectIntact(); diff --git a/chromium/net/dns/mdns_client_impl.cc b/chromium/net/dns/mdns_client_impl.cc index 624912d67cb..2bd3b825b33 100644 --- a/chromium/net/dns/mdns_client_impl.cc +++ b/chromium/net/dns/mdns_client_impl.cc @@ -62,8 +62,7 @@ MDnsConnection::SocketHandler::SocketHandler( MDnsConnection* connection) : socket_(std::move(socket)), connection_(connection), - response_(dns_protocol::kMaxMulticastSize), - send_in_progress_(false) {} + response_(dns_protocol::kMaxMulticastSize) {} MDnsConnection::SocketHandler::~SocketHandler() = default; @@ -496,9 +495,7 @@ MDnsListenerImpl::MDnsListenerImpl(uint16_t rrtype, name_(name), clock_(clock), client_(client), - delegate_(delegate), - started_(false), - active_refresh_(false) {} + delegate_(delegate) {} MDnsListenerImpl::~MDnsListenerImpl() { if (started_) { @@ -628,7 +625,6 @@ MDnsTransactionImpl::MDnsTransactionImpl( name_(name), callback_(callback), client_(client), - started_(false), flags_(flags) { DCHECK((flags_ & MDnsTransaction::FLAG_MASK) == flags_); DCHECK(flags_ & MDnsTransaction::QUERY_CACHE || diff --git a/chromium/net/dns/mdns_client_impl.h b/chromium/net/dns/mdns_client_impl.h index 2bca834d4a0..d21c1dbb762 100644 --- a/chromium/net/dns/mdns_client_impl.h +++ b/chromium/net/dns/mdns_client_impl.h @@ -103,7 +103,7 @@ class NET_EXPORT_PRIVATE MDnsConnection { IPEndPoint recv_addr_; DnsResponse response_; IPEndPoint multicast_addr_; - bool send_in_progress_; + bool send_in_progress_ = false; base::queue<std::pair<scoped_refptr<IOBuffer>, unsigned>> send_queue_; }; @@ -293,8 +293,8 @@ class MDnsListenerImpl : public MDnsListener, base::Time last_update_; uint32_t ttl_; - bool started_; - bool active_refresh_; + bool started_ = false; + bool active_refresh_ = false; base::CancelableRepeatingClosure next_refresh_; }; @@ -360,7 +360,7 @@ class MDnsTransactionImpl : public base::SupportsWeakPtr<MDnsTransactionImpl>, raw_ptr<MDnsClientImpl> client_; - bool started_; + bool started_ = false; int flags_; }; diff --git a/chromium/net/dns/mock_host_resolver.cc b/chromium/net/dns/mock_host_resolver.cc index 75b7e7e4431..03500cf172b 100644 --- a/chromium/net/dns/mock_host_resolver.cc +++ b/chromium/net/dns/mock_host_resolver.cc @@ -72,16 +72,14 @@ const unsigned kMaxCacheEntries = 100; // TTL for the successful resolutions. Failures are not cached. const unsigned kCacheEntryTTLSeconds = 60; -base::StringPiece GetScheme( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint) { +base::StringPiece GetScheme(const HostResolver::Host& endpoint) { DCHECK(absl::holds_alternative<url::SchemeHostPort>(endpoint)); return absl::get<url::SchemeHostPort>(endpoint).scheme(); } // In HostPortPair format (no brackets around IPv6 literals) purely for // compatibility with IPAddress::AssignFromIPLiteral(). -base::StringPiece GetHostname( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint) { +base::StringPiece GetHostname(const HostResolver::Host& endpoint) { if (absl::holds_alternative<url::SchemeHostPort>(endpoint)) { base::StringPiece hostname = absl::get<url::SchemeHostPort>(endpoint).host(); @@ -96,8 +94,7 @@ base::StringPiece GetHostname( return absl::get<HostPortPair>(endpoint).host(); } -uint16_t GetPort( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint) { +uint16_t GetPort(const HostResolver::Host& endpoint) { if (absl::holds_alternative<url::SchemeHostPort>(endpoint)) return absl::get<url::SchemeHostPort>(endpoint).port(); @@ -106,7 +103,7 @@ uint16_t GetPort( } absl::variant<url::SchemeHostPort, std::string> GetCacheHost( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint) { + const HostResolver::Host& endpoint) { if (absl::holds_alternative<url::SchemeHostPort>(endpoint)) return absl::get<url::SchemeHostPort>(endpoint); @@ -136,7 +133,7 @@ int ParseAddressList(base::StringPiece host_list, class MockHostResolverBase::RequestImpl : public HostResolver::ResolveHostRequest { public: - RequestImpl(absl::variant<url::SchemeHostPort, HostPortPair> request_endpoint, + RequestImpl(Host request_endpoint, const NetworkIsolationKey& network_isolation_key, const absl::optional<ResolveHostParameters>& optional_parameters, base::WeakPtr<MockHostResolverBase> resolver) @@ -147,9 +144,7 @@ class MockHostResolverBase::RequestImpl priority_(parameters_.initial_priority), host_resolver_flags_(ParametersToHostResolverFlags(parameters_)), resolve_error_info_(ResolveErrorInfo(ERR_IO_PENDING)), - id_(0), - resolver_(resolver), - complete_(false) {} + resolver_(resolver) {} RequestImpl(const RequestImpl&) = delete; RequestImpl& operator=(const RequestImpl&) = delete; @@ -314,10 +309,7 @@ class MockHostResolverBase::RequestImpl std::move(callback_).Run(error); } - const absl::variant<url::SchemeHostPort, HostPortPair>& request_endpoint() - const { - return request_endpoint_; - } + const Host& request_endpoint() const { return request_endpoint_; } const NetworkIsolationKey& network_isolation_key() const { return network_isolation_key_; @@ -375,7 +367,7 @@ class MockHostResolverBase::RequestImpl return corrected; } - const absl::variant<url::SchemeHostPort, HostPortPair> request_endpoint_; + const Host request_endpoint_; const NetworkIsolationKey network_isolation_key_; const ResolveHostParameters parameters_; RequestPriority priority_; @@ -388,13 +380,13 @@ class MockHostResolverBase::RequestImpl ResolveErrorInfo resolve_error_info_; // Used while stored with the resolver for async resolution. Otherwise 0. - size_t id_; + size_t id_ = 0; CompletionOnceCallback callback_; // Use a WeakPtr as the resolver may be destroyed while there are still // outstanding request objects. base::WeakPtr<MockHostResolverBase> resolver_; - bool complete_; + bool complete_ = false; }; class MockHostResolverBase::ProbeRequestImpl @@ -429,10 +421,7 @@ class MockHostResolverBase::MdnsListenerImpl MdnsListenerImpl(const HostPortPair& host, DnsQueryType query_type, base::WeakPtr<MockHostResolverBase> resolver) - : host_(host), - query_type_(query_type), - delegate_(nullptr), - resolver_(resolver) { + : host_(host), query_type_(query_type), resolver_(resolver) { DCHECK_NE(DnsQueryType::UNSPECIFIED, query_type_); DCHECK(resolver_); } @@ -479,7 +468,7 @@ class MockHostResolverBase::MdnsListenerImpl const HostPortPair host_; const DnsQueryType query_type_; - raw_ptr<Delegate> delegate_; + raw_ptr<Delegate> delegate_ = nullptr; // Use a WeakPtr as the resolver may be destroyed while there are still // outstanding listener objects. @@ -519,7 +508,7 @@ MockHostResolverBase::RuleResolver::operator=(RuleResolver&&) = default; const MockHostResolverBase::RuleResolver::RuleResult& MockHostResolverBase::RuleResolver::Resolve( - const absl::variant<url::SchemeHostPort, HostPortPair>& request_endpoint, + const Host& request_endpoint, DnsQueryTypeSet request_types, HostResolverSource request_source) const { for (const auto& rule : rules_) { @@ -749,7 +738,7 @@ HostCache* MockHostResolverBase::GetHostCache() { } int MockHostResolverBase::LoadIntoCache( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint, + const Host& endpoint, const NetworkIsolationKey& network_isolation_key, const absl::optional<ResolveHostParameters>& optional_parameters) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -888,11 +877,7 @@ MockHostResolverBase::RequestImpl* MockHostResolverBase::request(size_t id) { MockHostResolverBase::MockHostResolverBase(bool use_caching, int cache_invalidation_num, RuleResolver rule_resolver) - : last_request_priority_(DEFAULT_PRIORITY), - last_secure_dns_policy_(SecureDnsPolicy::kAllow), - synchronous_mode_(false), - ondemand_mode_(false), - rule_resolver_(std::move(rule_resolver)), + : rule_resolver_(std::move(rule_resolver)), initial_cache_invalidation_num_(cache_invalidation_num), tick_clock_(base::DefaultTickClock::GetInstance()), state_(base::MakeRefCounted<State>()) { @@ -954,7 +939,7 @@ int MockHostResolverBase::Resolve(RequestImpl* request) { } int MockHostResolverBase::ResolveFromIPLiteralOrCache( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint, + const Host& endpoint, const NetworkIsolationKey& network_isolation_key, DnsQueryType dns_query_type, HostResolverFlags flags, @@ -1153,8 +1138,7 @@ RuleBasedHostResolverProc::Rule::~Rule() = default; RuleBasedHostResolverProc::RuleBasedHostResolverProc(HostResolverProc* previous, bool allow_fallback) - : HostResolverProc(previous, allow_fallback), - modifications_allowed_(true) {} + : HostResolverProc(previous, allow_fallback) {} void RuleBasedHostResolverProc::AddRule(const std::string& host_pattern, const std::string& replacement) { diff --git a/chromium/net/dns/mock_host_resolver.h b/chromium/net/dns/mock_host_resolver.h index 7b260261149..06f57b1897f 100644 --- a/chromium/net/dns/mock_host_resolver.h +++ b/chromium/net/dns/mock_host_resolver.h @@ -154,11 +154,9 @@ class MockHostResolverBase RuleResolver(RuleResolver&&); RuleResolver& operator=(RuleResolver&&); - const RuleResult& Resolve( - const absl::variant<url::SchemeHostPort, HostPortPair>& - request_endpoint, - DnsQueryTypeSet request_types, - HostResolverSource request_source) const; + const RuleResult& Resolve(const Host& request_endpoint, + DnsQueryTypeSet request_types, + HostResolverSource request_source) const; void ClearRules(); @@ -286,7 +284,7 @@ class MockHostResolverBase // Preloads the cache with what would currently be the result of a request // with the given parameters. Returns the net error of the cached result. int LoadIntoCache( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint, + const Host& endpoint, const NetworkIsolationKey& network_isolation_key, const absl::optional<ResolveHostParameters>& optional_parameters); @@ -402,7 +400,7 @@ class MockHostResolverBase // Resolve as IP or from |cache_| return cached error or // DNS_CACHE_MISS if failed. int ResolveFromIPLiteralOrCache( - const absl::variant<url::SchemeHostPort, HostPortPair>& endpoint, + const Host& endpoint, const NetworkIsolationKey& network_isolation_key, DnsQueryType dns_query_type, HostResolverFlags flags, @@ -415,11 +413,11 @@ class MockHostResolverBase void AddListener(MdnsListenerImpl* listener); void RemoveCancelledListener(MdnsListenerImpl* listener); - RequestPriority last_request_priority_; + RequestPriority last_request_priority_ = DEFAULT_PRIORITY; absl::optional<NetworkIsolationKey> last_request_network_isolation_key_; - SecureDnsPolicy last_secure_dns_policy_; - bool synchronous_mode_; - bool ondemand_mode_; + SecureDnsPolicy last_secure_dns_policy_ = SecureDnsPolicy::kAllow; + bool synchronous_mode_ = false; + bool ondemand_mode_ = false; RuleResolver rule_resolver_; std::unique_ptr<HostCache> cache_; @@ -628,7 +626,7 @@ class RuleBasedHostResolverProc : public HostResolverProc { base::Lock rule_lock_; // Whether changes are allowed. - bool modifications_allowed_; + bool modifications_allowed_ = true; }; // Create rules that map all requests to localhost. diff --git a/chromium/net/dns/notify_watcher_mac.cc b/chromium/net/dns/notify_watcher_mac.cc index b7bab37bfd0..c1ba558d0d7 100644 --- a/chromium/net/dns/notify_watcher_mac.cc +++ b/chromium/net/dns/notify_watcher_mac.cc @@ -13,31 +13,7 @@ namespace net { -namespace { - -// Registers a dummy file descriptor to workaround a bug in libnotify -// in macOS 10.12 -// See https://bugs.chromium.org/p/chromium/issues/detail?id=783148. -class NotifyFileDescriptorsGlobalsHolder { - public: - NotifyFileDescriptorsGlobalsHolder() { - int notify_fd = -1; - int notify_token = -1; - notify_register_file_descriptor("notify_file_descriptor_holder", ¬ify_fd, - 0, ¬ify_token); - } -}; - -void HoldNotifyFileDescriptorsGlobals() { - if (base::mac::IsAtMostOS10_12()) { - static NotifyFileDescriptorsGlobalsHolder holder; - } -} -} // namespace - -NotifyWatcherMac::NotifyWatcherMac() : notify_fd_(-1), notify_token_(-1) { - HoldNotifyFileDescriptorsGlobals(); -} +NotifyWatcherMac::NotifyWatcherMac() : notify_fd_(-1), notify_token_(-1) {} NotifyWatcherMac::~NotifyWatcherMac() { Cancel(); diff --git a/chromium/net/dns/public/dns_config_overrides.cc b/chromium/net/dns/public/dns_config_overrides.cc index 1957c667989..242badeb203 100644 --- a/chromium/net/dns/public/dns_config_overrides.cc +++ b/chromium/net/dns/public/dns_config_overrides.cc @@ -24,7 +24,10 @@ DnsConfigOverrides& DnsConfigOverrides::operator=(DnsConfigOverrides&& other) = default; bool DnsConfigOverrides::operator==(const DnsConfigOverrides& other) const { - return nameservers == other.nameservers && search == other.search && + return nameservers == other.nameservers && + dns_over_tls_active == other.dns_over_tls_active && + dns_over_tls_hostname == other.dns_over_tls_hostname && + search == other.search && append_to_multi_label_name == other.append_to_multi_label_name && ndots == other.ndots && fallback_period == other.fallback_period && attempts == other.attempts && doh_attempts == other.doh_attempts && @@ -46,6 +49,8 @@ DnsConfigOverrides::CreateOverridingEverythingWithDefaults() { DnsConfigOverrides overrides; overrides.nameservers = defaults.nameservers; + overrides.dns_over_tls_active = defaults.dns_over_tls_active; + overrides.dns_over_tls_hostname = defaults.dns_over_tls_hostname; overrides.search = defaults.search; overrides.append_to_multi_label_name = defaults.append_to_multi_label_name; overrides.ndots = defaults.ndots; @@ -64,9 +69,10 @@ DnsConfigOverrides::CreateOverridingEverythingWithDefaults() { } bool DnsConfigOverrides::OverridesEverything() const { - return nameservers && search && append_to_multi_label_name && ndots && - fallback_period && attempts && doh_attempts && rotate && - use_local_ipv6 && dns_over_https_config && secure_dns_mode && + return nameservers && dns_over_tls_active && dns_over_tls_hostname && + search && append_to_multi_label_name && ndots && fallback_period && + attempts && doh_attempts && rotate && use_local_ipv6 && + dns_over_https_config && secure_dns_mode && allow_dns_over_https_upgrade && clear_hosts; } @@ -78,6 +84,10 @@ DnsConfig DnsConfigOverrides::ApplyOverrides(const DnsConfig& config) const { if (nameservers) overridden.nameservers = nameservers.value(); + if (dns_over_tls_active) + overridden.dns_over_tls_active = dns_over_tls_active.value(); + if (dns_over_tls_hostname) + overridden.dns_over_tls_hostname = dns_over_tls_hostname.value(); if (search) overridden.search = search.value(); if (append_to_multi_label_name) diff --git a/chromium/net/dns/public/dns_config_overrides.h b/chromium/net/dns/public/dns_config_overrides.h index 2770a8dab49..023eff26270 100644 --- a/chromium/net/dns/public/dns_config_overrides.h +++ b/chromium/net/dns/public/dns_config_overrides.h @@ -48,6 +48,8 @@ struct NET_EXPORT DnsConfigOverrides { // Overriding values. See same-named fields in DnsConfig for explanations. absl::optional<std::vector<IPEndPoint>> nameservers; + absl::optional<bool> dns_over_tls_active; + absl::optional<std::string> dns_over_tls_hostname; absl::optional<std::vector<std::string>> search; absl::optional<bool> append_to_multi_label_name; absl::optional<int> ndots; diff --git a/chromium/net/dns/public/dns_over_https_config.cc b/chromium/net/dns/public/dns_over_https_config.cc index 08a2a7f5730..9eee96c580b 100644 --- a/chromium/net/dns/public/dns_over_https_config.cc +++ b/chromium/net/dns/public/dns_over_https_config.cc @@ -8,6 +8,8 @@ #include <string> #include <vector> +#include "base/json/json_reader.h" +#include "base/json/json_writer.h" #include "base/ranges/algorithm.h" #include "base/strings/string_split.h" #include "base/strings/string_util.h" @@ -26,16 +28,43 @@ std::vector<std::string> SplitGroup(base::StringPiece group) { base::SPLIT_WANT_NONEMPTY); } -std::vector<absl::optional<DnsOverHttpsServerConfig>> ParseServers( - std::vector<std::string> servers) { +std::vector<absl::optional<DnsOverHttpsServerConfig>> ParseTemplates( + std::vector<std::string> templates) { std::vector<absl::optional<DnsOverHttpsServerConfig>> parsed; - parsed.reserve(servers.size()); - base::ranges::transform(servers, std::back_inserter(parsed), [](auto& s) { + parsed.reserve(templates.size()); + base::ranges::transform(templates, std::back_inserter(parsed), [](auto& s) { return DnsOverHttpsServerConfig::FromString(std::move(s)); }); return parsed; } +constexpr base::StringPiece kJsonKeyServers("servers"); + +absl::optional<DnsOverHttpsConfig> FromValue(base::Value::Dict value) { + base::Value::List* servers_value = value.FindList(kJsonKeyServers); + if (!servers_value) + return absl::nullopt; + std::vector<DnsOverHttpsServerConfig> servers; + servers.reserve(servers_value->size()); + for (base::Value& elt : *servers_value) { + base::Value::Dict* dict = elt.GetIfDict(); + if (!dict) + return absl::nullopt; + auto parsed = DnsOverHttpsServerConfig::FromValue(std::move(*dict)); + if (!parsed.has_value()) + return absl::nullopt; + servers.push_back(std::move(*parsed)); + } + return DnsOverHttpsConfig(servers); +} + +absl::optional<DnsOverHttpsConfig> FromJson(base::StringPiece json) { + absl::optional<base::Value> value = base::JSONReader::Read(json); + if (!value || !value->is_dict()) + return absl::nullopt; + return FromValue(std::move(value->GetDict())); +} + } // namespace DnsOverHttpsConfig::DnsOverHttpsConfig() = default; @@ -53,11 +82,11 @@ DnsOverHttpsConfig::DnsOverHttpsConfig( : servers_(std::move(servers)) {} // static -absl::optional<DnsOverHttpsConfig> DnsOverHttpsConfig::FromStrings( - std::vector<std::string> server_strings) { +absl::optional<DnsOverHttpsConfig> DnsOverHttpsConfig::FromTemplates( + std::vector<std::string> server_templates) { // All templates must be valid for the group to be considered valid. std::vector<DnsOverHttpsServerConfig> servers; - for (auto& server_config : ParseServers(server_strings)) { + for (auto& server_config : ParseTemplates(server_templates)) { if (!server_config) return absl::nullopt; servers.push_back(std::move(*server_config)); @@ -66,19 +95,29 @@ absl::optional<DnsOverHttpsConfig> DnsOverHttpsConfig::FromStrings( } // static +absl::optional<DnsOverHttpsConfig> DnsOverHttpsConfig::FromTemplatesForTesting( + std::vector<std::string> server_templates) { + return FromTemplates(std::move(server_templates)); +} + +// static absl::optional<DnsOverHttpsConfig> DnsOverHttpsConfig::FromString( base::StringPiece doh_config) { - // TODO(crbug.com/1200908): Also accept JSON-formatted input. - std::vector<std::string> server_strings = SplitGroup(doh_config); - if (server_strings.empty()) + absl::optional<DnsOverHttpsConfig> parsed = FromJson(doh_config); + if (parsed && !parsed->servers().empty()) + return parsed; + std::vector<std::string> server_templates = SplitGroup(doh_config); + if (server_templates.empty()) return absl::nullopt; // `doh_config` must contain at least one server. - return FromStrings(std::move(server_strings)); + return FromTemplates(std::move(server_templates)); } // static DnsOverHttpsConfig DnsOverHttpsConfig::FromStringLax( base::StringPiece doh_config) { - auto parsed = ParseServers(SplitGroup(doh_config)); + if (absl::optional<DnsOverHttpsConfig> parsed = FromJson(doh_config)) + return *parsed; + auto parsed = ParseTemplates(SplitGroup(doh_config)); std::vector<DnsOverHttpsServerConfig> servers; for (auto& server_config : parsed) { if (server_config) @@ -91,25 +130,32 @@ bool DnsOverHttpsConfig::operator==(const DnsOverHttpsConfig& other) const { return servers() == other.servers(); } -std::vector<base::StringPiece> DnsOverHttpsConfig::ToStrings() const { - std::vector<base::StringPiece> strings; - strings.reserve(servers().size()); - base::ranges::transform(servers(), std::back_inserter(strings), - &DnsOverHttpsServerConfig::server_template_piece); - return strings; -} - std::string DnsOverHttpsConfig::ToString() const { - // TODO(crbug.com/1200908): Return JSON for complex configurations. - return base::JoinString(ToStrings(), "\n"); + if (base::ranges::all_of(servers(), &DnsOverHttpsServerConfig::IsSimple)) { + // Return the templates on separate lines. + std::vector<base::StringPiece> strings; + strings.reserve(servers().size()); + base::ranges::transform(servers(), std::back_inserter(strings), + &DnsOverHttpsServerConfig::server_template_piece); + return base::JoinString(std::move(strings), "\n"); + } + std::string json; + CHECK(base::JSONWriter::WriteWithOptions( + ToValue(), base::JSONWriter::OPTIONS_PRETTY_PRINT, &json)); + // Remove the trailing newline from pretty-print output. + base::TrimWhitespaceASCII(json, base::TRIM_TRAILING, &json); + return json; } -base::Value DnsOverHttpsConfig::ToValue() const { - base::Value::ListStorage list; +base::Value::Dict DnsOverHttpsConfig::ToValue() const { + base::Value::List list; list.reserve(servers().size()); - base::ranges::transform(servers(), std::back_inserter(list), - &DnsOverHttpsServerConfig::ToValue); - return base::Value(std::move(list)); + for (const auto& server : servers()) { + list.Append(server.ToValue()); + } + base::Value::Dict dict; + dict.Set(kJsonKeyServers, std::move(list)); + return dict; } } // namespace net diff --git a/chromium/net/dns/public/dns_over_https_config.h b/chromium/net/dns/public/dns_over_https_config.h index fb030d6cadc..dbc13f5d824 100644 --- a/chromium/net/dns/public/dns_over_https_config.h +++ b/chromium/net/dns/public/dns_over_https_config.h @@ -17,7 +17,8 @@ namespace net { // Represents a collection of DnsOverHttpsServerConfig. The string -// representation is a whitespace-separated list of DoH URI templates. +// representation is either a JSON object or a whitespace-separated +// list of DoH URI templates. // The Value representation is a list of dictionaries. class NET_EXPORT DnsOverHttpsConfig { public: @@ -30,9 +31,9 @@ class NET_EXPORT DnsOverHttpsConfig { explicit DnsOverHttpsConfig(std::vector<DnsOverHttpsServerConfig> servers); - // Constructs a Config from textual representations of zero or more servers. + // Constructs a Config from URI templates of zero or more servers. // Returns `nullopt` if any string is invalid. - static absl::optional<DnsOverHttpsConfig> FromStrings( + static absl::optional<DnsOverHttpsConfig> FromTemplatesForTesting( std::vector<std::string> servers); // Constructs a Config from its text form if valid. Returns `nullopt` if the @@ -41,7 +42,7 @@ class NET_EXPORT DnsOverHttpsConfig { base::StringPiece doh_config); // Constructs a DnsOverHttpsConfig from its text form, skipping any invalid - // templates. The result may be empty. + // templates in the whitespace-separated form. The result may be empty. static DnsOverHttpsConfig FromStringLax(base::StringPiece doh_config); bool operator==(const DnsOverHttpsConfig& other) const; @@ -51,17 +52,18 @@ class NET_EXPORT DnsOverHttpsConfig { return servers_; } - // Returns string representations of the individual DnsOverHttpsServerConfigs. - // The return value will be invalidated if this object is destroyed or moved. - std::vector<base::StringPiece> ToStrings() const; - - // Inverse of FromString(). + // Inverse of FromString(). Uses the JSON representation if necessary. std::string ToString() const; - // Encodes the config as a Value. Currently only used for NetLog. - base::Value ToValue() const; + // Encodes the config as a Value. Used to produce the JSON representation. + base::Value::Dict ToValue() const; private: + // Constructs a Config from URI templates of zero or more servers. + // Returns `nullopt` if any string is invalid. + static absl::optional<DnsOverHttpsConfig> FromTemplates( + std::vector<std::string> servers); + std::vector<DnsOverHttpsServerConfig> servers_; }; diff --git a/chromium/net/dns/public/dns_over_https_config_unittest.cc b/chromium/net/dns/public/dns_over_https_config_unittest.cc index e4baa667853..6830cf53f17 100644 --- a/chromium/net/dns/public/dns_over_https_config_unittest.cc +++ b/chromium/net/dns/public/dns_over_https_config_unittest.cc @@ -6,7 +6,7 @@ #include "base/values.h" #include "net/dns/public/dns_over_https_server_config.h" -#include "testing/gmock/include/gmock/gmock-matchers.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { @@ -21,8 +21,10 @@ TEST(DnsOverHttpsConfigTest, SingleValue) { DnsOverHttpsConfig config({kServerConfig1}); EXPECT_THAT(config.servers(), testing::ElementsAre(kServerConfig1)); - base::Value expected_value(base::Value::Type::LIST); - expected_value.Append(kServerConfig1.ToValue()); + base::Value::List expected_servers; + expected_servers.Append(kServerConfig1.ToValue()); + base::Value::Dict expected_value; + expected_value.Set("servers", std::move(expected_servers)); EXPECT_EQ(expected_value, config.ToValue()); EXPECT_EQ(config, config); @@ -33,16 +35,15 @@ TEST(DnsOverHttpsConfigTest, MultiValue) { DnsOverHttpsConfig config(servers); EXPECT_EQ(servers, config.servers()); - EXPECT_THAT(config.ToStrings(), - testing::ElementsAre(kServerConfig1.server_template(), - kServerConfig2.server_template())); EXPECT_EQ(kServerConfig1.server_template() + "\n" + kServerConfig2.server_template(), config.ToString()); - base::Value expected_value(base::Value::Type::LIST); - expected_value.Append(kServerConfig1.ToValue()); - expected_value.Append(kServerConfig2.ToValue()); + base::Value::List expected_servers; + expected_servers.Append(kServerConfig1.ToValue()); + expected_servers.Append(kServerConfig2.ToValue()); + base::Value::Dict expected_value; + expected_value.Set("servers", std::move(expected_servers)); EXPECT_EQ(expected_value, config.ToValue()); EXPECT_EQ(config, config); @@ -85,18 +86,6 @@ TEST(DnsOverHttpsConfigTest, FromStringMultiValue) { testing::Optional(DnsOverHttpsConfig({kServerConfig1, kServerConfig2}))); } -TEST(DnsOverHttpsConfigTest, FromStrings) { - EXPECT_THAT(DnsOverHttpsConfig::FromStrings({}), - testing::Optional(DnsOverHttpsConfig())); - EXPECT_THAT( - DnsOverHttpsConfig::FromStrings({kServerConfig1.server_template()}), - testing::Optional(DnsOverHttpsConfig({kServerConfig1}))); - EXPECT_THAT( - DnsOverHttpsConfig::FromStrings( - {kServerConfig1.server_template(), kServerConfig2.server_template()}), - testing::Optional(DnsOverHttpsConfig({kServerConfig1, kServerConfig2}))); -} - TEST(DnsOverHttpsConfigTest, FromStringExtraWhitespace) { auto config = DnsOverHttpsConfig::FromString( " \t" + kServerConfig1.server_template() + " " + @@ -118,18 +107,13 @@ TEST(DnsOverHttpsConfigTest, FromStringEmpty) { TEST(DnsOverHttpsConfigTest, FromStringAllInvalid) { EXPECT_FALSE(DnsOverHttpsConfig::FromString("foo")); - EXPECT_FALSE(DnsOverHttpsConfig::FromStrings({"foo"})); EXPECT_EQ(DnsOverHttpsConfig(), DnsOverHttpsConfig::FromStringLax("foo")); EXPECT_FALSE(DnsOverHttpsConfig::FromString("foo bar")); - EXPECT_FALSE(DnsOverHttpsConfig::FromStrings({"foo", "bar"})); EXPECT_EQ(DnsOverHttpsConfig(), DnsOverHttpsConfig::FromStringLax("foo bar")); } TEST(DnsOverHttpsConfigTest, FromStringSomeInvalid) { - EXPECT_FALSE(DnsOverHttpsConfig::FromStrings( - {"foo", kServerConfig1.server_template(), "bar"})); - std::string some_invalid = "foo " + kServerConfig1.server_template() + " bar " + kServerConfig2.server_template() + " baz"; @@ -138,5 +122,99 @@ TEST(DnsOverHttpsConfigTest, FromStringSomeInvalid) { DnsOverHttpsConfig::FromStringLax(some_invalid)); } +TEST(DnsOverHttpsConfigTest, Json) { + auto parsed = DnsOverHttpsConfig::FromString(R"( + { + "servers": [{ + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": [{ + "ips": ["192.0.2.1", "2001:db8::1"] + }, { + "ips": ["192.0.2.2", "2001:db8::2"] + }] + }] + } + )"); + + ASSERT_TRUE(parsed); + EXPECT_EQ(1u, parsed->servers().size()); + + auto parsed2 = DnsOverHttpsConfig::FromString(parsed->ToString()); + EXPECT_EQ(parsed, parsed2); +} + +TEST(DnsOverHttpsConfigTest, JsonWithUnknownKey) { + auto parsed = DnsOverHttpsConfig::FromString(R"( + { + "servers": [{ + "template": "https://dnsserver.example.net/dns-query{?dns}" + }], + "unknown key": "value is ignored" + } + )"); + + ASSERT_TRUE(parsed); + EXPECT_EQ(1u, parsed->servers().size()); + + auto parsed2 = DnsOverHttpsConfig::FromString(parsed->ToString()); + EXPECT_EQ(parsed, parsed2); +} + +TEST(DnsOverHttpsConfigTest, BadJson) { + // Not JSON + EXPECT_FALSE(DnsOverHttpsConfig::FromString("{")); + + // No servers + EXPECT_FALSE(DnsOverHttpsConfig::FromString("{}")); + + // Not a Dict + EXPECT_FALSE(DnsOverHttpsConfig::FromString("[]")); + + // Wrong type for "servers" + EXPECT_FALSE(DnsOverHttpsConfig::FromString("{\"servers\": 12345}")); + + // One bad server + EXPECT_FALSE(DnsOverHttpsConfig::FromString(R"( + { + "servers": [{ + "template": "https://dnsserver.example.net/dns-query{?dns}", + }, { + "template": "not a valid template" + }] + } + )")); +} + +TEST(DnsOverHttpsConfigTest, JsonLax) { + // Valid JSON is allowed + auto parsed = *DnsOverHttpsConfig::FromString(R"( + { + "servers": [{ + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": [{ + "ips": ["192.0.2.1", "2001:db8::1"] + }, { + "ips": ["192.0.2.2", "2001:db8::2"] + }] + }] + } + )"); + DnsOverHttpsConfig reparsed = + DnsOverHttpsConfig::FromStringLax(parsed.ToString()); + EXPECT_EQ(parsed, reparsed); + + // Lax parsing does not accept bad servers in JSON. + DnsOverHttpsConfig from_bad = DnsOverHttpsConfig::FromStringLax(R"( + { + "servers": [{ + "template": "https://dnsserver.example.net/dns-query{?dns}", + }, { + "template": "not a valid template" + }] + } + )"); + EXPECT_THAT(from_bad.servers(), testing::IsEmpty()); +} + } // namespace } // namespace net diff --git a/chromium/net/dns/public/dns_over_https_server_config.cc b/chromium/net/dns/public/dns_over_https_server_config.cc index 51aa35eaa92..f8c3334f118 100644 --- a/chromium/net/dns/public/dns_over_https_server_config.cc +++ b/chromium/net/dns/public/dns_over_https_server_config.cc @@ -8,6 +8,8 @@ #include <string> #include <unordered_map> +#include "base/json/json_reader.h" +#include "base/json/json_writer.h" #include "base/strings/string_piece.h" #include "base/values.h" #include "net/third_party/uri_template/uri_template.h" @@ -68,27 +70,54 @@ bool IsValidDohTemplate(const std::string& server_template, bool* use_post) { return true; } +constexpr base::StringPiece kJsonKeyTemplate("template"); +constexpr base::StringPiece kJsonKeyEndpoints("endpoints"); +constexpr base::StringPiece kJsonKeyIps("ips"); + } // namespace namespace net { +DnsOverHttpsServerConfig::DnsOverHttpsServerConfig(std::string server_template, + bool use_post, + Endpoints endpoints) + : server_template_(std::move(server_template)), + use_post_(use_post), + endpoints_(std::move(endpoints)) {} + +DnsOverHttpsServerConfig::DnsOverHttpsServerConfig() = default; +DnsOverHttpsServerConfig::DnsOverHttpsServerConfig( + const DnsOverHttpsServerConfig& other) = default; +DnsOverHttpsServerConfig& DnsOverHttpsServerConfig::operator=( + const DnsOverHttpsServerConfig& other) = default; +DnsOverHttpsServerConfig::DnsOverHttpsServerConfig( + DnsOverHttpsServerConfig&& other) = default; +DnsOverHttpsServerConfig& DnsOverHttpsServerConfig::operator=( + DnsOverHttpsServerConfig&& other) = default; + +DnsOverHttpsServerConfig::~DnsOverHttpsServerConfig() = default; + absl::optional<DnsOverHttpsServerConfig> DnsOverHttpsServerConfig::FromString( - std::string doh_template) { + std::string doh_template, + Endpoints bindings) { bool use_post; if (!IsValidDohTemplate(doh_template, &use_post)) return absl::nullopt; - return DnsOverHttpsServerConfig(std::move(doh_template), use_post); + return DnsOverHttpsServerConfig(std::move(doh_template), use_post, + std::move(bindings)); } bool DnsOverHttpsServerConfig::operator==( const DnsOverHttpsServerConfig& other) const { // use_post_ is derived from server_template_, so we don't need to compare it. - return server_template_ == other.server_template_; + return server_template_ == other.server_template_ && + endpoints_ == other.endpoints_; } bool DnsOverHttpsServerConfig::operator<( const DnsOverHttpsServerConfig& other) const { - return server_template_ < other.server_template_; + return std::tie(server_template_, endpoints_) < + std::tie(other.server_template_, other.endpoints_); } const std::string& DnsOverHttpsServerConfig::server_template() const { @@ -103,10 +132,78 @@ bool DnsOverHttpsServerConfig::use_post() const { return use_post_; } -base::Value DnsOverHttpsServerConfig::ToValue() const { - base::Value value(base::Value::Type::DICTIONARY); - value.SetStringKey("server_template", server_template()); +const DnsOverHttpsServerConfig::Endpoints& DnsOverHttpsServerConfig::endpoints() + const { + return endpoints_; +} + +bool DnsOverHttpsServerConfig::IsSimple() const { + return endpoints_.empty(); +} + +base::Value::Dict DnsOverHttpsServerConfig::ToValue() const { + base::Value::Dict value; + value.Set(kJsonKeyTemplate, server_template()); + if (!endpoints_.empty()) { + base::Value::List bindings; + bindings.reserve(endpoints_.size()); + for (const IPAddressList& ip_list : endpoints_) { + base::Value::Dict binding; + base::Value::List ips; + ips.reserve(ip_list.size()); + for (const IPAddress& ip : ip_list) { + ips.Append(ip.ToString()); + } + binding.Set(kJsonKeyIps, std::move(ips)); + bindings.Append(std::move(binding)); + } + value.Set(kJsonKeyEndpoints, std::move(bindings)); + } return value; } +// static +absl::optional<DnsOverHttpsServerConfig> DnsOverHttpsServerConfig::FromValue( + base::Value::Dict value) { + std::string* server_template = value.FindString(kJsonKeyTemplate); + if (!server_template) + return absl::nullopt; + bool use_post; + if (!IsValidDohTemplate(*server_template, &use_post)) + return absl::nullopt; + Endpoints endpoints; + const base::Value* endpoints_json = value.Find(kJsonKeyEndpoints); + if (endpoints_json) { + if (!endpoints_json->is_list()) + return absl::nullopt; + const base::Value::List& json_list = endpoints_json->GetList(); + endpoints.reserve(json_list.size()); + for (const base::Value& endpoint : json_list) { + const base::Value::Dict* dict = endpoint.GetIfDict(); + if (!dict) + return absl::nullopt; + IPAddressList parsed_ips; + const base::Value* ips = dict->Find(kJsonKeyIps); + if (ips) { + const base::Value::List* ip_list = ips->GetIfList(); + if (!ip_list) + return absl::nullopt; + parsed_ips.reserve(ip_list->size()); + for (const base::Value& ip : *ip_list) { + const std::string* ip_str = ip.GetIfString(); + if (!ip_str) + return absl::nullopt; + IPAddress parsed; + if (!parsed.AssignFromIPLiteral(*ip_str)) + return absl::nullopt; + parsed_ips.push_back(std::move(parsed)); + } + } + endpoints.push_back(std::move(parsed_ips)); + } + } + return DnsOverHttpsServerConfig(std::move(*server_template), use_post, + std::move(endpoints)); +} + } // namespace net diff --git a/chromium/net/dns/public/dns_over_https_server_config.h b/chromium/net/dns/public/dns_over_https_server_config.h index 4ca53f9c8f2..312ce74e6a5 100644 --- a/chromium/net/dns/public/dns_over_https_server_config.h +++ b/chromium/net/dns/public/dns_over_https_server_config.h @@ -9,6 +9,7 @@ #include "base/strings/string_piece.h" #include "base/values.h" +#include "net/base/ip_address.h" #include "net/base/net_export.h" #include "third_party/abseil-cpp/absl/types/optional.h" @@ -17,9 +18,25 @@ namespace net { // Simple representation of a DoH server for use in configurations. class NET_EXPORT DnsOverHttpsServerConfig { public: + // TODO(crbug.com/1200908): Generalize endpoints to enable other capabilities + // of HTTPS records, such as extended metadata and aliases. + using Endpoints = std::vector<IPAddressList>; + + // A default constructor is required by Mojo. + DnsOverHttpsServerConfig(); + DnsOverHttpsServerConfig(const DnsOverHttpsServerConfig& other); + DnsOverHttpsServerConfig& operator=(const DnsOverHttpsServerConfig& other); + DnsOverHttpsServerConfig(DnsOverHttpsServerConfig&& other); + DnsOverHttpsServerConfig& operator=(DnsOverHttpsServerConfig&& other); + ~DnsOverHttpsServerConfig(); + // Returns nullopt if |doh_template| is invalid. static absl::optional<DnsOverHttpsServerConfig> FromString( - std::string doh_template); + std::string doh_template, + Endpoints endpoints = {}); + + static absl::optional<DnsOverHttpsServerConfig> FromValue( + base::Value::Dict value); bool operator==(const DnsOverHttpsServerConfig& other) const; bool operator<(const DnsOverHttpsServerConfig& other) const; @@ -27,15 +44,21 @@ class NET_EXPORT DnsOverHttpsServerConfig { const std::string& server_template() const; base::StringPiece server_template_piece() const; bool use_post() const; + const Endpoints& endpoints() const; + + // Returns true if this server config can be represented as just a template. + bool IsSimple() const; - base::Value ToValue() const; + base::Value::Dict ToValue() const; private: - DnsOverHttpsServerConfig(std::string server_template, bool use_post) - : server_template_(std::move(server_template)), use_post_(use_post) {} + DnsOverHttpsServerConfig(std::string server_template, + bool use_post, + Endpoints endpoints); std::string server_template_; bool use_post_; + Endpoints endpoints_; }; } // namespace net diff --git a/chromium/net/dns/public/dns_over_https_server_config_unittest.cc b/chromium/net/dns/public/dns_over_https_server_config_unittest.cc index 39f8e478e06..8c262e60703 100644 --- a/chromium/net/dns/public/dns_over_https_server_config_unittest.cc +++ b/chromium/net/dns/public/dns_over_https_server_config_unittest.cc @@ -6,6 +6,8 @@ #include <string> +#include "base/json/json_reader.h" +#include "net/base/ip_address.h" #include "net/dns/public/dns_over_https_config.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -13,6 +15,12 @@ namespace net { namespace { +const IPAddress ip1(192, 0, 2, 1); +const IPAddress ip2(0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1); +const IPAddress ip3(192, 0, 2, 2); +const IPAddress ip4(0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2); +const DnsOverHttpsServerConfig::Endpoints endpoints{{ip1, ip2}, {ip3, ip4}}; + TEST(DnsOverHttpsServerConfigTest, ValidWithGet) { auto parsed = DnsOverHttpsServerConfig::FromString( "https://dnsserver.example.net/dns-query{?dns}"); @@ -67,5 +75,162 @@ TEST(DnsOverHttpsServerConfigTest, Empty) { EXPECT_FALSE(net::DnsOverHttpsServerConfig::FromString("")); } +TEST(DnsOverHttpsServerConfigTest, Simple) { + auto parsed = DnsOverHttpsServerConfig::FromString( + "https://dnsserver.example.net/dns-query{?dns}"); + EXPECT_THAT(parsed, testing::Optional(testing::Property( + &DnsOverHttpsServerConfig::IsSimple, true))); +} + +TEST(DnsOverHttpsServerConfigTest, ToValueSimple) { + auto parsed = DnsOverHttpsServerConfig::FromString( + "https://dnsserver.example.net/dns-query{?dns}"); + ASSERT_TRUE(parsed); + + base::Value expected = *base::JSONReader::Read(R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}" + } + )"); + EXPECT_EQ(expected.GetDict(), parsed->ToValue()); +} + +TEST(DnsOverHttpsServerConfigTest, ToValueWithEndpoints) { + auto parsed = DnsOverHttpsServerConfig::FromString( + "https://dnsserver.example.net/dns-query{?dns}", endpoints); + ASSERT_TRUE(parsed); + + EXPECT_THAT(parsed, testing::Optional(testing::Property( + &DnsOverHttpsServerConfig::IsSimple, false))); + EXPECT_THAT(parsed, testing::Optional(testing::Property( + &DnsOverHttpsServerConfig::endpoints, endpoints))); + + base::Value expected = *base::JSONReader::Read( + R"({ + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": [{ + "ips": ["192.0.2.1", "2001:db8::1"] + }, { + "ips": ["192.0.2.2", "2001:db8::2"] + }] + })"); + EXPECT_EQ(expected.GetDict(), parsed->ToValue()); +} + +TEST(DnsOverHttpsServerConfigTest, FromValueSimple) { + base::Value input = *base::JSONReader::Read(R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}" + } + )"); + + auto parsed = DnsOverHttpsServerConfig::FromValue(std::move(input.GetDict())); + + auto expected = DnsOverHttpsServerConfig::FromString( + "https://dnsserver.example.net/dns-query{?dns}"); + EXPECT_EQ(expected, parsed); +} + +TEST(DnsOverHttpsServerConfigTest, FromValueWithEndpoints) { + base::Value input = *base::JSONReader::Read(R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": [{ + "ips": ["192.0.2.1", "2001:db8::1"] + }, { + "ips": ["192.0.2.2", "2001:db8::2"] + }] + } + )"); + + auto parsed = DnsOverHttpsServerConfig::FromValue(std::move(input.GetDict())); + + auto expected = DnsOverHttpsServerConfig::FromString( + "https://dnsserver.example.net/dns-query{?dns}", endpoints); + EXPECT_EQ(expected, parsed); +} + +TEST(DnsOverHttpsServerConfigTest, FromValueWithUnknownKey) { + base::Value input = *base::JSONReader::Read(R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}", + "unknown key": "value is ignored" + } + )"); + + auto parsed = DnsOverHttpsServerConfig::FromValue(std::move(input.GetDict())); + + auto expected = DnsOverHttpsServerConfig::FromString( + "https://dnsserver.example.net/dns-query{?dns}"); + EXPECT_EQ(expected, parsed); +} + +TEST(DnsOverHttpsServerConfigTest, FromValueInvalid) { + // Empty dict + EXPECT_FALSE(DnsOverHttpsServerConfig::FromValue(base::Value::Dict())); + + // Wrong scheme + base::StringPiece input = R"( + { + "template": "http://dnsserver.example.net/dns-query{?dns}" + } + )"; + EXPECT_FALSE(DnsOverHttpsServerConfig::FromValue( + std::move(base::JSONReader::Read(input)->GetDict()))); + + // Wrong template type + input = R"({"template": 12345})"; + EXPECT_FALSE(DnsOverHttpsServerConfig::FromValue( + std::move(base::JSONReader::Read(input)->GetDict()))); + + // Wrong endpoints type + input = R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": { + "ips": ["192.0.2.1", "2001:db8::1"] + } + } + )"; + EXPECT_FALSE(DnsOverHttpsServerConfig::FromValue( + std::move(base::JSONReader::Read(input)->GetDict()))); + + // Wrong "ips" type + input = R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": [{ + "ips": "192.0.2.1" + }] + } + )"; + EXPECT_FALSE(DnsOverHttpsServerConfig::FromValue( + std::move(base::JSONReader::Read(input)->GetDict()))); + + // Wrong IP type + input = R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": [{ + "ips": ["2001:db8::1", 192.021] + }] + } + )"; + EXPECT_FALSE(DnsOverHttpsServerConfig::FromValue( + std::move(base::JSONReader::Read(input)->GetDict()))); + + // Bad IP address + input = R"( + { + "template": "https://dnsserver.example.net/dns-query{?dns}", + "endpoints": [{ + "ips": ["2001:db8::1", "256.257.258.259"] + }] + } + )"; + EXPECT_FALSE(DnsOverHttpsServerConfig::FromValue( + std::move(base::JSONReader::Read(input)->GetDict()))); +} + } // namespace } // namespace net diff --git a/chromium/net/dns/public/util.cc b/chromium/net/dns/public/util.cc index 873099614a4..0423fcd95d4 100644 --- a/chromium/net/dns/public/util.cc +++ b/chromium/net/dns/public/util.cc @@ -50,7 +50,7 @@ IPEndPoint GetMdnsReceiveEndPoint(AddressFamily address_family) { // CrOS as described in crbug.com/931916, and the following is a temporary // mitigation to reconcile the two issues. Remove this after closing // crbug.com/899310. -#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_FUCHSIA) || BUILDFLAG(IS_APPLE) +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_APPLE) // With Windows, binding to a mulitcast group address is not allowed. // Multicast messages will be received appropriate to the multicast groups the // socket has joined. Sockets intending to receive multicast messages should @@ -66,12 +66,14 @@ IPEndPoint GetMdnsReceiveEndPoint(AddressFamily address_family) { NOTREACHED(); return IPEndPoint(); } -#else // !(BUILDFLAG(IS_WIN) || BUILDFLAG(IS_FUCHSIA)) || BUILDFLAG(IS_APPLE) - // With POSIX, any socket can receive messages for multicast groups joined by - // any socket on the system. Sockets intending to receive messages for a - // specific multicast group should bind to that group address. +#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) + // With POSIX/Fuchsia, any socket can receive messages for multicast groups + // joined by any socket on the system. Sockets intending to receive messages + // for a specific multicast group should bind to that group address. return GetMdnsGroupEndPoint(address_family); -#endif // !(BUILDFLAG(IS_WIN) || BUILDFLAG(IS_FUCHSIA)) || BUILDFLAG(IS_APPLE) +#else +#error Platform not supported. +#endif } std::string GetNameForHttpsQuery(const url::SchemeHostPort& scheme_host_port, diff --git a/chromium/net/dns/record_parsed.h b/chromium/net/dns/record_parsed.h index b8aceb32774..5bbf84e9d4f 100644 --- a/chromium/net/dns/record_parsed.h +++ b/chromium/net/dns/record_parsed.h @@ -10,6 +10,7 @@ #include <memory> #include <string> +#include "base/check.h" #include "base/time/time.h" #include "net/base/net_export.h" diff --git a/chromium/net/dns/record_rdata.cc b/chromium/net/dns/record_rdata.cc index 26b46cb3623..8e969a16d70 100644 --- a/chromium/net/dns/record_rdata.cc +++ b/chromium/net/dns/record_rdata.cc @@ -54,8 +54,7 @@ bool RecordRdata::HasValidSize(const base::StringPiece& data, uint16_t type) { } } -SrvRecordRdata::SrvRecordRdata() : priority_(0), weight_(0), port_(0) { -} +SrvRecordRdata::SrvRecordRdata() = default; SrvRecordRdata::~SrvRecordRdata() = default; diff --git a/chromium/net/dns/record_rdata.h b/chromium/net/dns/record_rdata.h index 8e4b4e6fcd5..6097b96bb66 100644 --- a/chromium/net/dns/record_rdata.h +++ b/chromium/net/dns/record_rdata.h @@ -68,9 +68,9 @@ class NET_EXPORT_PRIVATE SrvRecordRdata : public RecordRdata { private: SrvRecordRdata(); - uint16_t priority_; - uint16_t weight_; - uint16_t port_; + uint16_t priority_ = 0; + uint16_t weight_ = 0; + uint16_t port_ = 0; std::string target_; }; diff --git a/chromium/net/dns/resolve_context.cc b/chromium/net/dns/resolve_context.cc index b4a3101440f..52ffae34190 100644 --- a/chromium/net/dns/resolve_context.cc +++ b/chromium/net/dns/resolve_context.cc @@ -119,7 +119,7 @@ static std::unique_ptr<base::SampleVector> GetRttHistogram( ResolveContext::ServerStats::ServerStats( std::unique_ptr<base::SampleVector> buckets) - : last_failure_count(0), rtt_histogram(std::move(buckets)) {} + : rtt_histogram(std::move(buckets)) {} ResolveContext::ServerStats::ServerStats(ServerStats&&) = default; @@ -333,7 +333,10 @@ void ResolveContext::UnregisterDohStatusObserver( void ResolveContext::InvalidateCachesAndPerSessionData( const DnsSession* new_session, bool network_change) { - DCHECK(MustRegisterForInvalidations()); + // Network-bound ResolveContexts should never receive a cache invalidation due + // to a network change. + DCHECK(GetTargetNetwork() == NetworkChangeNotifier::kInvalidNetworkHandle || + !network_change); if (host_cache_) host_cache_->Invalidate(); @@ -386,12 +389,6 @@ NetworkChangeNotifier::NetworkHandle ResolveContext::GetTargetNetwork() const { return url_request_context()->bound_network(); } -bool ResolveContext::MustRegisterForInvalidations() const { - // Resolve contexts targeting a network shouldn't have their cache invalidated - // when an network (or DNS) change occurs. More context at crbug.com/1292548. - return GetTargetNetwork() == NetworkChangeNotifier::kInvalidNetworkHandle; -} - size_t ResolveContext::FirstServerIndex(bool doh_server, const DnsSession* session) { if (!IsCurrentSession(session)) diff --git a/chromium/net/dns/resolve_context.h b/chromium/net/dns/resolve_context.h index d9063ae8eb1..f9153321cac 100644 --- a/chromium/net/dns/resolve_context.h +++ b/chromium/net/dns/resolve_context.h @@ -186,11 +186,6 @@ class NET_EXPORT_PRIVATE ResolveContext : public base::CheckedObserver { // Virtual for testing. virtual NetworkChangeNotifier::NetworkHandle GetTargetNetwork() const; - // Helper method to know whether this ResolveContext must be registered to - // receive cache and per-session data invalidations (i.e., receive - // InvalidateCachesAndPerSessionData type of calls) to function properly. - bool MustRegisterForInvalidations() const; - base::SafeRef<ResolveContext> AsSafeRef() { return weak_ptr_factory_.GetSafeRef(); } @@ -211,7 +206,7 @@ class NET_EXPORT_PRIVATE ResolveContext : public base::CheckedObserver { ~ServerStats(); // Count of consecutive failures after last success. - int last_failure_count; + int last_failure_count = 0; // True if any success has ever been recorded for this server for the // current connection. diff --git a/chromium/net/dns/resolve_context_unittest.cc b/chromium/net/dns/resolve_context_unittest.cc index 4ca342d0bf4..4543399ff67 100644 --- a/chromium/net/dns/resolve_context_unittest.cc +++ b/chromium/net/dns/resolve_context_unittest.cc @@ -74,7 +74,8 @@ DnsConfig CreateDnsConfig(int num_servers, int num_doh_servers) { templates.push_back( base::StringPrintf("https://mock.http/doh_test_%d{?dns}", i)); } - config.doh_config = *DnsOverHttpsConfig::FromStrings(std::move(templates)); + config.doh_config = + *DnsOverHttpsConfig::FromTemplatesForTesting(std::move(templates)); return config; } diff --git a/chromium/net/dns/serial_worker.cc b/chromium/net/dns/serial_worker.cc index 5a3444b7f96..ee0a6c36734 100644 --- a/chromium/net/dns/serial_worker.cc +++ b/chromium/net/dns/serial_worker.cc @@ -49,8 +49,7 @@ void SerialWorker::WorkItem::FollowupWork(base::OnceClosure closure) { SerialWorker::SerialWorker(int max_number_of_retries, const net::BackoffEntry::Policy* backoff_policy) - : state_(State::kIdle), - max_number_of_retries_(max_number_of_retries), + : max_number_of_retries_(max_number_of_retries), backoff_entry_(backoff_policy ? backoff_policy : &kDefaultBackoffPolicy) { } diff --git a/chromium/net/dns/serial_worker.h b/chromium/net/dns/serial_worker.h index b7076be9f8e..c226fe6d010 100644 --- a/chromium/net/dns/serial_worker.h +++ b/chromium/net/dns/serial_worker.h @@ -117,7 +117,7 @@ class NET_EXPORT_PRIVATE SerialWorker { void RerunWork(std::unique_ptr<WorkItem> work_item); - State state_; + State state_ = State::kIdle; // Max retries and backoff entry to control timing. const int max_number_of_retries_; diff --git a/chromium/net/dns/serial_worker_unittest.cc b/chromium/net/dns/serial_worker_unittest.cc index e04b0f1428f..d5e7e7fe7f3 100644 --- a/chromium/net/dns/serial_worker_unittest.cc +++ b/chromium/net/dns/serial_worker_unittest.cc @@ -168,8 +168,7 @@ class SerialWorkerTest : public TestWithTaskEnvironment { work_allowed_(base::WaitableEvent::ResetPolicy::AUTOMATIC, base::WaitableEvent::InitialState::NOT_SIGNALED), work_called_(base::WaitableEvent::ResetPolicy::AUTOMATIC, - base::WaitableEvent::InitialState::NOT_SIGNALED), - work_running_(false) {} + base::WaitableEvent::InitialState::NOT_SIGNALED) {} // Helpers for tests. @@ -212,7 +211,7 @@ class SerialWorkerTest : public TestWithTaskEnvironment { base::WaitableEvent work_called_; // Protected by read_lock_. Used to verify that read calls are serialized. - bool work_running_; + bool work_running_ = false; base::Lock work_lock_; int work_finished_calls_ = 0; |