Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added sanitizer step for api helper #12685

Merged
merged 7 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
diracdeltas marked this conversation as resolved.
Show resolved Hide resolved
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