Loading DnsTlsDispatcher.cpp +1 −2 Original line number Diff line number Diff line Loading @@ -242,8 +242,7 @@ DnsTlsTransport::Result DnsTlsDispatcher::queryInternal(Transport& xport, const auto status = res.wait_for(xport.timeout()); if (status == std::future_status::timeout) { // TODO: notify the Transport to remove the query because no queries will await this future. // b/186613628. // TODO(b/186613628): notify the Transport to remove this query. LOG(WARNING) << "DoT query timed out after " << xport.timeout().count() << " ms"; return DnsTlsTransport::Result{ .code = DnsTlsTransport::Response::network_error, Loading PrivateDnsConfiguration.cpp +11 −13 Original line number Diff line number Diff line Loading @@ -280,14 +280,14 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const DnsTlsServer& ser auto netPair = mPrivateDnsTransports.find(netId); if (netPair == mPrivateDnsTransports.end()) { LOG(WARNING) << "netId " << netId << " was erased during private DNS validation"; notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return DONT_REEVALUATE; } const auto mode = mPrivateDnsModes.find(netId); if (mode == mPrivateDnsModes.end()) { LOG(WARNING) << "netId " << netId << " has no private DNS validation mode"; notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return DONT_REEVALUATE; } Loading @@ -308,8 +308,6 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const DnsTlsServer& ser success = false; reevaluationStatus = DONT_REEVALUATE; } else if (!(serverPair->second == server)) { // TODO: It doesn't seem correct to overwrite the tracker entry for // |server| down below in this circumstance... Fix this. LOG(WARNING) << "Server " << server.toIpString() << " was changed during private DNS validation"; success = false; Loading Loading @@ -342,18 +340,18 @@ void PrivateDnsConfiguration::updateServerState(const ServerIdentity& identity, uint32_t netId) { auto netPair = mPrivateDnsTransports.find(netId); if (netPair == mPrivateDnsTransports.end()) { notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return; } auto& tracker = netPair->second; if (tracker.find(identity) == tracker.end()) { notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return; } tracker[identity].setValidationState(state); notifyValidationStateUpdate(identity.ip.toString(), state, netId); notifyValidationStateUpdate(identity.sockaddr, state, netId); RecordEntry record(netId, identity, state); mPrivateDnsLog.push(std::move(record)); Loading @@ -380,11 +378,11 @@ void PrivateDnsConfiguration::setObserver(PrivateDnsValidationObserver* observer mObserver = observer; } void PrivateDnsConfiguration::notifyValidationStateUpdate(const std::string& serverIp, void PrivateDnsConfiguration::notifyValidationStateUpdate(const netdutils::IPSockAddr& sockaddr, Validation validation, uint32_t netId) const { if (mObserver) { mObserver->onValidationStateUpdate(serverIp, validation, netId); mObserver->onValidationStateUpdate(sockaddr.ip().toString(), validation, netId); } } Loading @@ -393,10 +391,10 @@ void PrivateDnsConfiguration::dump(netdutils::DumpWriter& dw) const { netdutils::ScopedIndent indentStats(dw); for (const auto& record : mPrivateDnsLog.copy()) { dw.println(fmt::format("{} - netId={} PrivateDns={{{}/{}}} state={}", timestampToString(record.timestamp), record.netId, record.serverIdentity.ip.toString(), record.serverIdentity.name, validationStatusToString(record.state))); dw.println(fmt::format( "{} - netId={} PrivateDns={{{}/{}}} state={}", timestampToString(record.timestamp), record.netId, record.serverIdentity.sockaddr.toString(), record.serverIdentity.provider, validationStatusToString(record.state))); } dw.blankline(); } Loading PrivateDnsConfiguration.h +6 −10 Original line number Diff line number Diff line Loading @@ -72,20 +72,17 @@ class PrivateDnsConfiguration { EXCLUDES(mPrivateDnsLock); struct ServerIdentity { const netdutils::IPAddress ip; const std::string name; const int protocol; const netdutils::IPSockAddr sockaddr; const std::string provider; explicit ServerIdentity(const DnsTlsServer& server) : ip(netdutils::IPSockAddr::toIPSockAddr(server.ss).ip()), name(server.name), protocol(server.protocol) {} : sockaddr(netdutils::IPSockAddr::toIPSockAddr(server.ss)), provider(server.name) {} bool operator<(const ServerIdentity& other) const { return std::tie(ip, name, protocol) < std::tie(other.ip, other.name, other.protocol); return std::tie(sockaddr, provider) < std::tie(other.sockaddr, other.provider); } bool operator==(const ServerIdentity& other) const { return std::tie(ip, name, protocol) == std::tie(other.ip, other.name, other.protocol); return std::tie(sockaddr, provider) == std::tie(other.sockaddr, other.provider); } }; Loading @@ -95,7 +92,6 @@ class PrivateDnsConfiguration { private: typedef std::map<ServerIdentity, DnsTlsServer> PrivateDnsTracker; typedef std::set<DnsTlsServer, AddressComparator> ThreadTracker; PrivateDnsConfiguration() = default; Loading Loading @@ -127,7 +123,7 @@ class PrivateDnsConfiguration { // Any pending validation threads will continue running because we have no way to cancel them. std::map<unsigned, PrivateDnsTracker> mPrivateDnsTransports GUARDED_BY(mPrivateDnsLock); void notifyValidationStateUpdate(const std::string& serverIp, Validation validation, void notifyValidationStateUpdate(const netdutils::IPSockAddr& sockaddr, Validation validation, uint32_t netId) const REQUIRES(mPrivateDnsLock); // TODO: fix the reentrancy problem. Loading PrivateDnsConfigurationTest.cpp +2 −9 Original line number Diff line number Diff line Loading @@ -234,13 +234,12 @@ TEST_F(PrivateDnsConfigurationTest, ServerIdentity_Comparison) { DnsTlsServer server(netdutils::IPSockAddr::toIPSockAddr("127.0.0.1", 853)); server.name = "dns.example.com"; server.protocol = 1; // Different IP address (port is ignored). // Different socket address. DnsTlsServer other = server; EXPECT_EQ(ServerIdentity(server), ServerIdentity(other)); other.ss = netdutils::IPSockAddr::toIPSockAddr("127.0.0.1", 5353); EXPECT_EQ(ServerIdentity(server), ServerIdentity(other)); EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); other.ss = netdutils::IPSockAddr::toIPSockAddr("127.0.0.2", 853); EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); Loading @@ -251,12 +250,6 @@ TEST_F(PrivateDnsConfigurationTest, ServerIdentity_Comparison) { EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); other.name = ""; EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); // Different protocol. other = server; EXPECT_EQ(ServerIdentity(server), ServerIdentity(other)); other.protocol++; EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); } TEST_F(PrivateDnsConfigurationTest, RequestValidation) { Loading Loading
DnsTlsDispatcher.cpp +1 −2 Original line number Diff line number Diff line Loading @@ -242,8 +242,7 @@ DnsTlsTransport::Result DnsTlsDispatcher::queryInternal(Transport& xport, const auto status = res.wait_for(xport.timeout()); if (status == std::future_status::timeout) { // TODO: notify the Transport to remove the query because no queries will await this future. // b/186613628. // TODO(b/186613628): notify the Transport to remove this query. LOG(WARNING) << "DoT query timed out after " << xport.timeout().count() << " ms"; return DnsTlsTransport::Result{ .code = DnsTlsTransport::Response::network_error, Loading
PrivateDnsConfiguration.cpp +11 −13 Original line number Diff line number Diff line Loading @@ -280,14 +280,14 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const DnsTlsServer& ser auto netPair = mPrivateDnsTransports.find(netId); if (netPair == mPrivateDnsTransports.end()) { LOG(WARNING) << "netId " << netId << " was erased during private DNS validation"; notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return DONT_REEVALUATE; } const auto mode = mPrivateDnsModes.find(netId); if (mode == mPrivateDnsModes.end()) { LOG(WARNING) << "netId " << netId << " has no private DNS validation mode"; notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return DONT_REEVALUATE; } Loading @@ -308,8 +308,6 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const DnsTlsServer& ser success = false; reevaluationStatus = DONT_REEVALUATE; } else if (!(serverPair->second == server)) { // TODO: It doesn't seem correct to overwrite the tracker entry for // |server| down below in this circumstance... Fix this. LOG(WARNING) << "Server " << server.toIpString() << " was changed during private DNS validation"; success = false; Loading Loading @@ -342,18 +340,18 @@ void PrivateDnsConfiguration::updateServerState(const ServerIdentity& identity, uint32_t netId) { auto netPair = mPrivateDnsTransports.find(netId); if (netPair == mPrivateDnsTransports.end()) { notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return; } auto& tracker = netPair->second; if (tracker.find(identity) == tracker.end()) { notifyValidationStateUpdate(identity.ip.toString(), Validation::fail, netId); notifyValidationStateUpdate(identity.sockaddr, Validation::fail, netId); return; } tracker[identity].setValidationState(state); notifyValidationStateUpdate(identity.ip.toString(), state, netId); notifyValidationStateUpdate(identity.sockaddr, state, netId); RecordEntry record(netId, identity, state); mPrivateDnsLog.push(std::move(record)); Loading @@ -380,11 +378,11 @@ void PrivateDnsConfiguration::setObserver(PrivateDnsValidationObserver* observer mObserver = observer; } void PrivateDnsConfiguration::notifyValidationStateUpdate(const std::string& serverIp, void PrivateDnsConfiguration::notifyValidationStateUpdate(const netdutils::IPSockAddr& sockaddr, Validation validation, uint32_t netId) const { if (mObserver) { mObserver->onValidationStateUpdate(serverIp, validation, netId); mObserver->onValidationStateUpdate(sockaddr.ip().toString(), validation, netId); } } Loading @@ -393,10 +391,10 @@ void PrivateDnsConfiguration::dump(netdutils::DumpWriter& dw) const { netdutils::ScopedIndent indentStats(dw); for (const auto& record : mPrivateDnsLog.copy()) { dw.println(fmt::format("{} - netId={} PrivateDns={{{}/{}}} state={}", timestampToString(record.timestamp), record.netId, record.serverIdentity.ip.toString(), record.serverIdentity.name, validationStatusToString(record.state))); dw.println(fmt::format( "{} - netId={} PrivateDns={{{}/{}}} state={}", timestampToString(record.timestamp), record.netId, record.serverIdentity.sockaddr.toString(), record.serverIdentity.provider, validationStatusToString(record.state))); } dw.blankline(); } Loading
PrivateDnsConfiguration.h +6 −10 Original line number Diff line number Diff line Loading @@ -72,20 +72,17 @@ class PrivateDnsConfiguration { EXCLUDES(mPrivateDnsLock); struct ServerIdentity { const netdutils::IPAddress ip; const std::string name; const int protocol; const netdutils::IPSockAddr sockaddr; const std::string provider; explicit ServerIdentity(const DnsTlsServer& server) : ip(netdutils::IPSockAddr::toIPSockAddr(server.ss).ip()), name(server.name), protocol(server.protocol) {} : sockaddr(netdutils::IPSockAddr::toIPSockAddr(server.ss)), provider(server.name) {} bool operator<(const ServerIdentity& other) const { return std::tie(ip, name, protocol) < std::tie(other.ip, other.name, other.protocol); return std::tie(sockaddr, provider) < std::tie(other.sockaddr, other.provider); } bool operator==(const ServerIdentity& other) const { return std::tie(ip, name, protocol) == std::tie(other.ip, other.name, other.protocol); return std::tie(sockaddr, provider) == std::tie(other.sockaddr, other.provider); } }; Loading @@ -95,7 +92,6 @@ class PrivateDnsConfiguration { private: typedef std::map<ServerIdentity, DnsTlsServer> PrivateDnsTracker; typedef std::set<DnsTlsServer, AddressComparator> ThreadTracker; PrivateDnsConfiguration() = default; Loading Loading @@ -127,7 +123,7 @@ class PrivateDnsConfiguration { // Any pending validation threads will continue running because we have no way to cancel them. std::map<unsigned, PrivateDnsTracker> mPrivateDnsTransports GUARDED_BY(mPrivateDnsLock); void notifyValidationStateUpdate(const std::string& serverIp, Validation validation, void notifyValidationStateUpdate(const netdutils::IPSockAddr& sockaddr, Validation validation, uint32_t netId) const REQUIRES(mPrivateDnsLock); // TODO: fix the reentrancy problem. Loading
PrivateDnsConfigurationTest.cpp +2 −9 Original line number Diff line number Diff line Loading @@ -234,13 +234,12 @@ TEST_F(PrivateDnsConfigurationTest, ServerIdentity_Comparison) { DnsTlsServer server(netdutils::IPSockAddr::toIPSockAddr("127.0.0.1", 853)); server.name = "dns.example.com"; server.protocol = 1; // Different IP address (port is ignored). // Different socket address. DnsTlsServer other = server; EXPECT_EQ(ServerIdentity(server), ServerIdentity(other)); other.ss = netdutils::IPSockAddr::toIPSockAddr("127.0.0.1", 5353); EXPECT_EQ(ServerIdentity(server), ServerIdentity(other)); EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); other.ss = netdutils::IPSockAddr::toIPSockAddr("127.0.0.2", 853); EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); Loading @@ -251,12 +250,6 @@ TEST_F(PrivateDnsConfigurationTest, ServerIdentity_Comparison) { EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); other.name = ""; EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); // Different protocol. other = server; EXPECT_EQ(ServerIdentity(server), ServerIdentity(other)); other.protocol++; EXPECT_NE(ServerIdentity(server), ServerIdentity(other)); } TEST_F(PrivateDnsConfigurationTest, RequestValidation) { Loading