Commit 48026448 authored by Brandon Wylie's avatar Brandon Wylie Committed by Commit Bot

Add helper method to sanitize DeletionInfo of invalid/empty urls

Bug: 1136486
Change-Id: Id15a99875e7529985df3d795994dbb162e5c0460
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462491
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818041}
parent fbe0aaf8
...@@ -24,6 +24,17 @@ static jlong JNI_HistoryDeletionBridge_Init(JNIEnv* env, ...@@ -24,6 +24,17 @@ static jlong JNI_HistoryDeletionBridge_Init(JNIEnv* env,
return reinterpret_cast<intptr_t>(new HistoryDeletionBridge(jobj)); return reinterpret_cast<intptr_t>(new HistoryDeletionBridge(jobj));
} }
// static
history::DeletionInfo HistoryDeletionBridge::SanitizeDeletionInfo(
const history::DeletionInfo& deletion_info) {
std::vector<history::URLRow> sanitized_rows;
for (auto row : deletion_info.deleted_rows()) {
if (!row.url().is_empty() && row.url().is_valid())
sanitized_rows.push_back(row);
}
return history::DeletionInfo::ForUrls(sanitized_rows, {});
}
HistoryDeletionBridge::HistoryDeletionBridge(const JavaRef<jobject>& jobj) HistoryDeletionBridge::HistoryDeletionBridge(const JavaRef<jobject>& jobj)
: jobj_(ScopedJavaGlobalRef<jobject>(jobj)), : jobj_(ScopedJavaGlobalRef<jobject>(jobj)),
profile_(ProfileManager::GetLastUsedProfile()->GetOriginalProfile()) { profile_(ProfileManager::GetLastUsedProfile()->GetOriginalProfile()) {
...@@ -47,6 +58,7 @@ void HistoryDeletionBridge::OnURLsDeleted( ...@@ -47,6 +58,7 @@ void HistoryDeletionBridge::OnURLsDeleted(
const history::DeletionInfo& deletion_info) { const history::DeletionInfo& deletion_info) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
history::DeletionInfo sanitized_info = SanitizeDeletionInfo(deletion_info);
Java_HistoryDeletionBridge_onURLsDeleted( Java_HistoryDeletionBridge_onURLsDeleted(
env, jobj_, CreateHistoryDeletionInfo(env, &deletion_info)); env, jobj_, CreateHistoryDeletionInfo(env, &sanitized_info));
} }
...@@ -25,6 +25,12 @@ class HistoryDeletionBridge : public history::HistoryServiceObserver { ...@@ -25,6 +25,12 @@ class HistoryDeletionBridge : public history::HistoryServiceObserver {
void OnURLsDeleted(history::HistoryService* history_service, void OnURLsDeleted(history::HistoryService* history_service,
const history::DeletionInfo& deletion_info) override; const history::DeletionInfo& deletion_info) override;
// Sanitize the DeletionInfo of empty/invalid urls before passing to java.
// Fix for empty java strings being passed to the content capture service
// (crbug.com/1136486).
static history::DeletionInfo SanitizeDeletionInfo(
const history::DeletionInfo& deletion_info);
private: private:
~HistoryDeletionBridge() override; ~HistoryDeletionBridge() override;
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/android/history/history_deletion_bridge.h"
#include "base/time/time.h"
#include "components/history/core/browser/history_types.h"
#include "components/history/core/browser/url_row.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
TEST(HistoryDeletionBridge, TestSanitizeDeletionInfo) {
history::DeletionInfo info = history::DeletionInfo::ForUrls(
{history::URLResult(GURL("https://google.com/"), base::Time()),
history::URLResult(GURL("https://google.com/foo"), base::Time()),
history::URLResult(GURL("htt\\invalido\\gle.com"), base::Time()),
history::URLResult(GURL(""), base::Time())},
{});
std::vector<GURL> expected = {GURL("https://google.com/"),
GURL("https://google.com/foo")};
std::vector<history::URLRow> actual =
HistoryDeletionBridge::SanitizeDeletionInfo(info).deleted_rows();
EXPECT_EQ(expected.size(), actual.size());
for (auto row : actual)
EXPECT_NE(expected.end(),
std::find(expected.begin(), expected.end(), row.url()));
}
...@@ -3790,6 +3790,7 @@ test("unit_tests") { ...@@ -3790,6 +3790,7 @@ test("unit_tests") {
"../browser/android/explore_sites/ntp_json_fetcher_unittest.cc", "../browser/android/explore_sites/ntp_json_fetcher_unittest.cc",
"../browser/android/explore_sites/record_site_click_task_unittest.cc", "../browser/android/explore_sites/record_site_click_task_unittest.cc",
"../browser/android/favicon_helper_unittest.cc", "../browser/android/favicon_helper_unittest.cc",
"../browser/android/history/history_deletion_bridge_unittest.cc",
"../browser/android/history_report/data_observer_unittest.cc", "../browser/android/history_report/data_observer_unittest.cc",
"../browser/android/history_report/delta_file_backend_leveldb_unittest.cc", "../browser/android/history_report/delta_file_backend_leveldb_unittest.cc",
"../browser/android/history_report/delta_file_commons_unittest.cc", "../browser/android/history_report/delta_file_commons_unittest.cc",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment