From 8be3d7f33f9f74d75a5dbbc9aa2e56bbd2f985f1 Mon Sep 17 00:00:00 2001 From: Cepera Date: Wed, 12 Jul 2023 17:41:00 +0300 Subject: [PATCH] Fix IPFSHostResolver::Resolve crash (#19247) --- browser/ipfs/ipfs_host_resolver.cc | 28 ++++++++--- browser/ipfs/ipfs_host_resolver.h | 18 ++++++-- browser/ipfs/ipfs_host_resolver_unittest.cc | 9 ++-- browser/ipfs/ipfs_tab_helper.cc | 6 +-- browser/ipfs/ipfs_tab_helper_browsertest.cc | 51 ++++++++++----------- browser/ipfs/ipfs_tab_helper_unittest.cc | 8 ++-- 6 files changed, 72 insertions(+), 48 deletions(-) diff --git a/browser/ipfs/ipfs_host_resolver.cc b/browser/ipfs/ipfs_host_resolver.cc index da826ad42e7..bd2282602ea 100644 --- a/browser/ipfs/ipfs_host_resolver.cc +++ b/browser/ipfs/ipfs_host_resolver.cc @@ -10,7 +10,8 @@ #include "base/strings/string_split.h" #include "brave/browser/ipfs/ipfs_host_resolver.h" #include "chrome/browser/net/secure_dns_config.h" -#include "chrome/browser/net/system_network_context_manager.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/storage_partition.h" #include "mojo/public/cpp/bindings/receiver.h" #include "net/base/host_port_pair.h" #include "net/dns/public/dns_protocol.h" @@ -42,10 +43,9 @@ absl::optional GetDNSRecordValue( namespace ipfs { -IPFSHostResolver::IPFSHostResolver( - network::mojom::NetworkContext& network_context, - const std::string& prefix) - : prefix_(prefix), network_context_(network_context) {} +IPFSHostResolver::IPFSHostResolver(content::BrowserContext* browser_context, + const std::string& prefix) + : prefix_(prefix), browser_context_(browser_context) {} IPFSHostResolver::~IPFSHostResolver() = default; void IPFSHostResolver::Resolve( @@ -72,13 +72,27 @@ void IPFSHostResolver::Resolve( dnslink_ = absl::nullopt; resolving_host_ = host.host(); net::HostPortPair local_host_port(prefix_ + resolving_host_, host.port()); - - network_context_->ResolveHost( + auto* network_context = GetNetworkContext(); + if (!network_context) { + return; + } + network_context->ResolveHost( network::mojom::HostResolverHost::NewHostPortPair(local_host_port), anonymization_key, std::move(parameters), receiver_.BindNewPipeAndPassRemote()); } +network::mojom::NetworkContext* IPFSHostResolver::GetNetworkContext() { + if (network_context_for_testing_.has_value()) { + return network_context_for_testing_.value(); + } + auto* storage_partition = browser_context_->GetDefaultStoragePartition(); + if (!storage_partition) { + return nullptr; + } + return storage_partition->GetNetworkContext(); +} + void IPFSHostResolver::OnComplete( int result, const net::ResolveErrorInfo& error_info, diff --git a/browser/ipfs/ipfs_host_resolver.h b/browser/ipfs/ipfs_host_resolver.h index cc96a59497c..a35218d33f6 100644 --- a/browser/ipfs/ipfs_host_resolver.h +++ b/browser/ipfs/ipfs_host_resolver.h @@ -11,13 +11,18 @@ #include #include "base/functional/callback_forward.h" +#include "base/memory/raw_ptr.h" +#include "chrome/browser/net/system_network_context_manager.h" #include "mojo/public/cpp/bindings/receiver.h" #include "net/base/host_port_pair.h" #include "net/base/network_anonymization_key.h" #include "net/dns/public/dns_query_type.h" #include "services/network/public/cpp/resolve_host_client_base.h" #include "services/network/public/mojom/host_resolver.mojom.h" -#include "services/network/public/mojom/network_context.mojom.h" + +namespace content { +class BrowserContext; +} // namespace content namespace ipfs { @@ -25,7 +30,7 @@ namespace ipfs { // automatically adds it to the host. class IPFSHostResolver : public network::ResolveHostClientBase { public: - explicit IPFSHostResolver(network::mojom::NetworkContext& network_context, + explicit IPFSHostResolver(content::BrowserContext* browser_context, const std::string& prefix = std::string()); ~IPFSHostResolver() override; @@ -40,6 +45,10 @@ class IPFSHostResolver : public network::ResolveHostClientBase { std::string host() const { return resolving_host_; } absl::optional dnslink() const { return dnslink_; } + void SetNetworkContextForTesting( + network::mojom::NetworkContext* network_context) { + network_context_for_testing_ = network_context; + } private: // network::mojom::ResolveHostClient implementation: @@ -49,12 +58,15 @@ class IPFSHostResolver : public network::ResolveHostClientBase { const absl::optional& endpoint_results_with_metadata) override; void OnTextResults(const std::vector& text_results) override; + network::mojom::NetworkContext* GetNetworkContext(); std::string resolving_host_; std::string prefix_; absl::optional dnslink_; + absl::optional network_context_for_testing_ = + nullptr; - raw_ref network_context_; + raw_ptr browser_context_; HostTextResultsCallback resolved_callback_; mojo::Receiver receiver_{this}; diff --git a/browser/ipfs/ipfs_host_resolver_unittest.cc b/browser/ipfs/ipfs_host_resolver_unittest.cc index 8ac91f962c1..6b4b17fb030 100644 --- a/browser/ipfs/ipfs_host_resolver_unittest.cc +++ b/browser/ipfs/ipfs_host_resolver_unittest.cc @@ -174,7 +174,8 @@ TEST_F(IPFSHostResolverTest, PrefixRunSuccess) { auto* fake_host_resolver_raw = fake_host_resolver.get(); network_context->SetHostResolver(std::move(fake_host_resolver)); base::RunLoop run_loop; - ipfs::IPFSHostResolver ipfs_resolver(*network_context, prefix); + ipfs::IPFSHostResolver ipfs_resolver(nullptr, prefix); + ipfs_resolver.SetNetworkContextForTesting(network_context); SetResolvedCallbackCalled(false); ipfs_resolver.Resolve( @@ -202,7 +203,8 @@ TEST_F(IPFSHostResolverTest, SuccessOnReuse) { auto* network_context = GetNetworkContext(); auto* fake_host_resolver_raw = fake_host_resolver.get(); network_context->SetHostResolver(std::move(fake_host_resolver)); - ipfs::IPFSHostResolver ipfs_resolver(*network_context, prefix); + ipfs::IPFSHostResolver ipfs_resolver(nullptr, prefix); + ipfs_resolver.SetNetworkContextForTesting(network_context); SetResolvedCallbackCalled(false); { @@ -247,7 +249,8 @@ TEST_F(IPFSHostResolverTest, ResolutionFailed) { auto* network_context = GetNetworkContext(); auto* fake_host_resolver_raw = fake_host_resolver.get(); network_context->SetHostResolver(std::move(fake_host_resolver)); - ipfs::IPFSHostResolver ipfs_resolver(*network_context); + ipfs::IPFSHostResolver ipfs_resolver(nullptr); + ipfs_resolver.SetNetworkContextForTesting(network_context); base::RunLoop run_loop; ipfs_resolver.Resolve( net::HostPortPair(host, 11), net::NetworkAnonymizationKey(), diff --git a/browser/ipfs/ipfs_tab_helper.cc b/browser/ipfs/ipfs_tab_helper.cc index 70f46a869b5..a637d60e173 100644 --- a/browser/ipfs/ipfs_tab_helper.cc +++ b/browser/ipfs/ipfs_tab_helper.cc @@ -23,7 +23,6 @@ #include "components/user_prefs/user_prefs.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/navigation_handle.h" -#include "content/public/browser/storage_partition.h" #include "content/public/browser/web_contents_delegate.h" #include "net/base/url_util.h" #include "net/http/http_status_code.h" @@ -71,11 +70,8 @@ IPFSTabHelper::IPFSTabHelper(content::WebContents* web_contents) content::WebContentsUserData(*web_contents), pref_service_( user_prefs::UserPrefs::Get(web_contents->GetBrowserContext())) { - auto* storage_partition = - web_contents->GetBrowserContext()->GetDefaultStoragePartition(); - resolver_ = std::make_unique( - *storage_partition->GetNetworkContext(), kDnsDomainPrefix); + web_contents->GetBrowserContext(), kDnsDomainPrefix); pref_change_registrar_.Init(pref_service_); pref_change_registrar_.Add( kIPFSResolveMethod, diff --git a/browser/ipfs/ipfs_tab_helper_browsertest.cc b/browser/ipfs/ipfs_tab_helper_browsertest.cc index 9d7890f212b..26ecf276eef 100644 --- a/browser/ipfs/ipfs_tab_helper_browsertest.cc +++ b/browser/ipfs/ipfs_tab_helper_browsertest.cc @@ -81,8 +81,7 @@ class IpfsTabHelperBrowserTest : public InProcessBrowserTest { class FakeIPFSHostResolver : public ipfs::IPFSHostResolver { public: - explicit FakeIPFSHostResolver(network::mojom::NetworkContext& context) - : ipfs::IPFSHostResolver(context) {} + FakeIPFSHostResolver() : ipfs::IPFSHostResolver(nullptr) {} ~FakeIPFSHostResolver() override = default; void Resolve(const net::HostPortPair& host, const net::NetworkAnonymizationKey& anonymization_key, @@ -114,9 +113,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ResolvedIPFSLinkLocal) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = user_prefs::UserPrefs::Get(active_contents()->GetBrowserContext()); @@ -192,9 +191,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ResolvedIPFSLinkGateway) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = user_prefs::UserPrefs::Get(active_contents()->GetBrowserContext()); @@ -221,9 +220,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, NoResolveIPFSLinkCalledMode) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = user_prefs::UserPrefs::Get(active_contents()->GetBrowserContext()); @@ -259,9 +258,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = user_prefs::UserPrefs::Get(active_contents()->GetBrowserContext()); @@ -287,9 +286,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = user_prefs::UserPrefs::Get(active_contents()->GetBrowserContext()); @@ -324,9 +323,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, GatewayRedirectToIPFS) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); resolver_raw->SetDNSLinkToRespond("/ipfs/QmXoypiz"); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = @@ -371,9 +370,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); resolver_raw->SetDNSLinkToRespond("/ipfs/QmXoypiz"); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = @@ -421,9 +420,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, GatewayRedirectToIPNS) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); resolver_raw->SetDNSLinkToRespond("/ipns/QmXoypiz"); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = @@ -469,9 +468,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ResolveIPFSLinkCalled5xx) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); resolver_raw->SetDNSLinkToRespond("/ipfs/QmXoypiz"); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = @@ -501,9 +500,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ResolveNotCalled5xx) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); SetHttpStatusCode(net::HTTP_INTERNAL_SERVER_ERROR); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = @@ -531,10 +530,10 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ResolvedIPFSLinkBad) { ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); helper->SetResolverForTesting(std::move(resolver)); auto* prefs = @@ -572,9 +571,9 @@ IN_PROC_BROWSER_TEST_F(IpfsTabHelperBrowserTest, ->GetDefaultStoragePartition() ->GetNetworkContext(); ASSERT_TRUE(network_context); - std::unique_ptr resolver( - new FakeIPFSHostResolver(*network_context)); + std::unique_ptr resolver(new FakeIPFSHostResolver()); FakeIPFSHostResolver* resolver_raw = resolver.get(); + resolver_raw->SetNetworkContextForTesting(network_context); helper->SetResolverForTesting(std::move(resolver)); std::string ipfs_path = "/ipfs/bafybeiemx/"; SetXIpfsPathHeader(ipfs_path); diff --git a/browser/ipfs/ipfs_tab_helper_unittest.cc b/browser/ipfs/ipfs_tab_helper_unittest.cc index 646fb4e0a5e..29fe67e71b2 100644 --- a/browser/ipfs/ipfs_tab_helper_unittest.cc +++ b/browser/ipfs/ipfs_tab_helper_unittest.cc @@ -28,8 +28,7 @@ namespace ipfs { class FakeIPFSHostResolver : public ipfs::IPFSHostResolver { public: - explicit FakeIPFSHostResolver(network::mojom::NetworkContext& context) - : ipfs::IPFSHostResolver(context) {} + FakeIPFSHostResolver() : ipfs::IPFSHostResolver(nullptr) {} ~FakeIPFSHostResolver() override = default; void Resolve(const net::HostPortPair& host, const net::NetworkAnonymizationKey& anonymization_key, @@ -60,9 +59,10 @@ class IpfsTabHelperUnitTest : public testing::Test { test_network_context_ = std::make_unique(); profile_ = profile_manager_.CreateTestingProfile("TestProfile"); web_contents_ = content::TestWebContents::Create(profile(), nullptr); - auto ipfs_host_resolver = - std::make_unique(*test_network_context_); + auto ipfs_host_resolver = std::make_unique(); ipfs_host_resolver_ = ipfs_host_resolver.get(); + ipfs_host_resolver_->SetNetworkContextForTesting( + test_network_context_.get()); ASSERT_TRUE(web_contents_.get()); ASSERT_TRUE( ipfs::IPFSTabHelper::MaybeCreateForWebContents(web_contents_.get()));