Skip to content

Commit

Permalink
Added sanitizer step for api helper (#12685)
Browse files Browse the repository at this point in the history
Added sanitizer step for api helper
  • Loading branch information
spylogsster authored Mar 29, 2022
1 parent c67ebcb commit 837a78c
Show file tree
Hide file tree
Showing 19 changed files with 286 additions and 40 deletions.
2 changes: 2 additions & 0 deletions browser/brave_wallet/brave_wallet_provider_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_web_contents_factory.h"
#include "content/test/test_web_contents.h"
#include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -761,6 +762,7 @@ class BraveWalletProviderImplUnitTest : public testing::Test {
std::unique_ptr<content::TestWebContents> web_contents_;
std::unique_ptr<BraveWalletProviderImpl> provider_;
network::TestURLLoaderFactory url_loader_factory_;
data_decoder::test::InProcessDataDecoder in_process_data_decoder_;
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
base::ScopedTempDir temp_dir_;
TestingProfile profile_;
Expand Down
2 changes: 2 additions & 0 deletions browser/brave_wallet/eth_nonce_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "components/prefs/pref_service.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "content/public/test/browser_task_environment.h"
#include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -81,6 +82,7 @@ class EthNonceTrackerUnitTest : public testing::Test {
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
content::BrowserTaskEnvironment task_environment_;
std::unique_ptr<TestingProfile> profile_;
data_decoder::test::InProcessDataDecoder in_process_data_decoder_;
};

TEST_F(EthNonceTrackerUnitTest, GetNonce) {
Expand Down
2 changes: 2 additions & 0 deletions browser/brave_wallet/eth_pending_tx_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "components/prefs/pref_service.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "content/public/test/browser_task_environment.h"
#include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -65,6 +66,7 @@ class EthPendingTxTrackerUnitTest : public testing::Test {
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
content::BrowserTaskEnvironment task_environment_;
std::unique_ptr<TestingProfile> profile_;
data_decoder::test::InProcessDataDecoder in_process_data_decoder_;
};

TEST_F(EthPendingTxTrackerUnitTest, IsNonceTaken) {
Expand Down
2 changes: 2 additions & 0 deletions browser/ui/webui/settings/brave_wallet_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_web_ui.h"
#include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -130,6 +131,7 @@ class TestBraveWalletHandler : public BraveWalletHandler {
std::unique_ptr<content::WebContents> web_contents_;
network::TestURLLoaderFactory url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
data_decoder::test::InProcessDataDecoder in_process_data_decoder_;
content::TestWebUI test_web_ui_;
};

Expand Down
12 changes: 12 additions & 0 deletions chromium_src/services/data_decoder/public/cpp/data_decoder.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/* Copyright (c) 2022 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "base/json/json_reader.h"

#include "src/services/data_decoder/public/cpp/data_decoder.h"

#define JSON_PARSE_RFC JSON_PARSE_RFC | base::JSON_ALLOW_TRAILING_COMMAS
#include "src/services/data_decoder/public/cpp/data_decoder.cc"
#undef JSON_PARSE_RFC
21 changes: 21 additions & 0 deletions components/api_request_helper/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.

import("//brave/build/config.gni")
import("//testing/test.gni")

static_library("api_request_helper") {
sources = [
"api_request_helper.cc",
Expand All @@ -12,7 +15,25 @@ static_library("api_request_helper") {
deps = [
"//base",
"//net",
"//services/data_decoder/public/cpp",
"//services/network/public/cpp",
"//url",
]
}

source_set("api_request_helper_unit_tests") {
testonly = true
sources =
[ "//brave/components/api_request_helper/api_request_helper_unittest.cc" ]
deps = [
":api_request_helper",
"//base/test:test_support",
"//net/traffic_annotation:test_support",
"//net/traffic_annotation:traffic_annotation",
"//services/data_decoder/public/cpp",
"//services/data_decoder/public/cpp:test_support",
"//services/network:test_support",
"//services/network/public/cpp",
"//testing/gtest:gtest",
]
}
2 changes: 2 additions & 0 deletions components/api_request_helper/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
include_rules = [
"+net",
"+services/data_decoder/public",
"+services/network/public/cpp",
"+services/network/public/mojom",
"+third_party/abseil-cpp/absl"
]
60 changes: 52 additions & 8 deletions components/api_request_helper/api_request_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,38 @@
#include <utility>

#include "net/base/load_flags.h"
#include "services/data_decoder/public/cpp/json_sanitizer.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "services/network/public/mojom/url_response_head.mojom.h"

namespace api_request_helper {

namespace {

void OnSanitize(const int http_code,
const base::flat_map<std::string, std::string>& headers,
APIRequestHelper::ResultCallback result_callback,
data_decoder::JsonSanitizer::Result result) {
std::string response_body;
if (result.error) {
VLOG(1) << "Response validation error:" << *result.error;
std::move(result_callback).Run(http_code, "", headers);
return;
}

if (result.value.has_value()) {
response_body = result.value.value();
}

std::move(result_callback).Run(http_code, response_body, headers);
}

const unsigned int kRetriesCountOnNetworkChange = 1;

} // namespace

APIRequestHelper::APIRequestHelper(
net::NetworkTrafficAnnotationTag annotation_tag,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
Expand All @@ -32,8 +55,9 @@ void APIRequestHelper::Request(
const std::string& payload_content_type,
bool auto_retry_on_network_change,
ResultCallback callback,
const base::flat_map<std::string, std::string>& headers /* ={} */,
size_t max_body_size /* =-1 */) {
const base::flat_map<std::string, std::string>& headers,
size_t max_body_size /* = -1u */,
ResponseConversionCallback conversion_callback) {
auto request = std::make_unique<network::ResourceRequest>();
request->url = url;
request->load_flags = net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
Expand Down Expand Up @@ -61,20 +85,23 @@ void APIRequestHelper::Request(
if (max_body_size == -1u) {
iter->get()->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory_.get(),
base::BindOnce(&APIRequestHelper::OnResponse, base::Unretained(this),
iter, std::move(callback)));
base::BindOnce(&APIRequestHelper::OnResponse,
weak_ptr_factory_.GetWeakPtr(), iter,
std::move(callback), std::move(conversion_callback)));
} else {
iter->get()->DownloadToString(
url_loader_factory_.get(),
base::BindOnce(&APIRequestHelper::OnResponse, base::Unretained(this),
iter, std::move(callback)),
base::BindOnce(&APIRequestHelper::OnResponse,
weak_ptr_factory_.GetWeakPtr(), iter,
std::move(callback), std::move(conversion_callback)),
max_body_size);
}
}

void APIRequestHelper::OnResponse(
SimpleURLLoaderList::iterator iter,
ResultCallback callback,
ResponseConversionCallback conversion_callback,
const std::unique_ptr<std::string> response_body) {
auto* loader = iter->get();
auto response_code = -1;
Expand All @@ -92,9 +119,26 @@ void APIRequestHelper::OnResponse(
}
}
}

url_loaders_.erase(iter);
std::move(callback).Run(response_code, response_body ? *response_body : "",
headers);
if (!response_body) {
std::move(callback).Run(response_code, "", headers);
return;
}
auto& raw_body = *response_body;
if (conversion_callback) {
auto converted_body = std::move(conversion_callback).Run(raw_body);
if (!converted_body) {
std::move(callback).Run(422, raw_body, headers);
return;
}
raw_body = converted_body.value();
}

data_decoder::JsonSanitizer::Sanitize(
std::move(raw_body),
base::BindOnce(&OnSanitize, response_code, std::move(headers),
std::move(callback)));
}

} // namespace api_request_helper
32 changes: 24 additions & 8 deletions components/api_request_helper/api_request_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
#include <string>

#include "base/callback.h"
#include "base/callback_helpers.h"
#include "base/containers/flat_map.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"

namespace network {
Expand All @@ -34,14 +36,26 @@ class APIRequestHelper {
base::OnceCallback<void(const int,
const std::string&,
const base::flat_map<std::string, std::string>&)>;
void Request(const std::string& method,
const GURL& url,
const std::string& payload,
const std::string& payload_content_type,
bool auto_retry_on_network_change,
ResultCallback callback,
const base::flat_map<std::string, std::string>& headers = {},
size_t max_body_size = -1u);
using ResponseConversionCallback =
base::OnceCallback<absl::optional<std::string>(
const std::string& raw_response)>;

// Each response is expected in json format and will be validated through
// JsonSanitizer. In cases where json contains values that are not supported
// by the standard base/json parser it is necessary to convert such values
// into string before validating the response. For these purposes
// conversion_callback is added which receives raw response and can perform
// necessary conversions.
void Request(
const std::string& method,
const GURL& url,
const std::string& payload,
const std::string& payload_content_type,
bool auto_retry_on_network_change,
ResultCallback callback,
const base::flat_map<std::string, std::string>& headers = {},
size_t max_body_size = -1u,
ResponseConversionCallback conversion_callback = base::NullCallback());

private:
APIRequestHelper(const APIRequestHelper&) = delete;
Expand All @@ -50,11 +64,13 @@ class APIRequestHelper {
std::list<std::unique_ptr<network::SimpleURLLoader>>;
void OnResponse(SimpleURLLoaderList::iterator iter,
ResultCallback callback,
ResponseConversionCallback conversion_callback,
const std::unique_ptr<std::string> response_body);

net::NetworkTrafficAnnotationTag annotation_tag_;
SimpleURLLoaderList url_loaders_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
base::WeakPtrFactory<APIRequestHelper> weak_ptr_factory_{this};
};

} // namespace api_request_helper
Expand Down
Loading

0 comments on commit 837a78c

Please sign in to comment.