Skip to content

Commit

Permalink
[STTS] Revert special case handling of non utf8 strings
Browse files Browse the repository at this point in the history
Bug: 1290500
Change-Id: Ibe0c5dd656c37e4950cfca27e7c26b4bbd5ecb8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3646087
Reviewed-by: Travis Skare <skare@chromium.org>
Commit-Queue: Jeffrey Cohen <jeffreycohen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002907}
  • Loading branch information
Jeffrey Cohen authored and Chromium LUCI CQ committed May 12, 2022
1 parent 1bf6155 commit 94184d0
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 45 deletions.
25 changes: 3 additions & 22 deletions components/send_tab_to_self/send_tab_to_self_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ absl::optional<syncer::ModelError> SendTabToSelfBridge::ApplySyncChanges(

std::unique_ptr<SendTabToSelfEntry> remote_entry =
SendTabToSelfEntry::FromProto(specifics, clock_->Now());
if (!remote_entry || remote_entry->GetGUID().empty()) {
if (!remote_entry) {
continue; // Skip invalid entries.
}
if (remote_entry->IsExpired(clock_->Now())) {
Expand Down Expand Up @@ -331,12 +331,6 @@ const SendTabToSelfEntry* SendTabToSelfBridge::AddEntry(
guid, url, trimmed_title, shared_time, local_device_name_,
target_device_cache_guid);

// In the case where the entry was not created properly do not commit it to
// the batch data.
if (entry->GetGUID().empty()) {
return nullptr;
}

std::unique_ptr<ModelTypeStore::WriteBatch> batch =
store_->CreateWriteBatch();
// This entry is new. Add it to the store and model.
Expand Down Expand Up @@ -623,21 +617,8 @@ void SendTabToSelfBridge::DoGarbageCollection() {
auto entry = entries_.begin();
while (entry != entries_.end()) {
DCHECK_EQ(entry->first, entry->second->GetGUID());
if (entry->second == nullptr) {
std::string to_delete = entry->first;
entry++;
DeleteEntry(to_delete);
removed.push_back(to_delete);
continue;
}
std::string guid = entry->second->GetGUID();
// In the case of an invalid entry remove it here.
if (guid.empty()) {
entry++;
DeleteEntry(guid);
removed.push_back(guid);
continue;
}

std::string guid = entry->first;
bool expired = entry->second->IsExpired(clock_->Now());
entry++;
if (expired) {
Expand Down
19 changes: 3 additions & 16 deletions components/send_tab_to_self/send_tab_to_self_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/strings/string_util.h"
#include "components/send_tab_to_self/proto/send_tab_to_self.pb.h"
#include "components/sync/protocol/send_tab_to_self_specifics.pb.h"
#include "third_party/icu/source/common/unicode/unistr.h"

namespace send_tab_to_self {

Expand Down Expand Up @@ -48,12 +47,6 @@ SendTabToSelfEntry::SendTabToSelfEntry(
opened_(false) {
DCHECK(!guid_.empty());
DCHECK(url_.is_valid());
if (!base::IsStringUTF8(guid_) || !base::IsStringUTF8(title_) ||
!base::IsStringUTF8(target_device_sync_cache_guid_) ||
!base::IsStringUTF8(device_name_)) {
// Set GUID as an empty strings to make entry invalid.
guid_ = std::string();
}
}

SendTabToSelfEntry::~SendTabToSelfEntry() {}
Expand Down Expand Up @@ -120,13 +113,7 @@ std::unique_ptr<SendTabToSelfEntry> SendTabToSelfEntry::FromProto(
const sync_pb::SendTabToSelfSpecifics& pb_entry,
base::Time now) {
std::string guid(pb_entry.guid());
std::string title(pb_entry.title());
std::string device_name(pb_entry.device_name());
std::string target_device_sync_cache_guid(
pb_entry.target_device_sync_cache_guid());
if (guid.empty() || !base::IsStringUTF8(guid) || !base::IsStringUTF8(title) ||
!base::IsStringUTF8(device_name) ||
!base::IsStringUTF8(target_device_sync_cache_guid)) {
if (guid.empty()) {
return nullptr;
}

Expand All @@ -143,8 +130,8 @@ std::unique_ptr<SendTabToSelfEntry> SendTabToSelfEntry::FromProto(

// Protobuf parsing enforces utf8 encoding for all strings.
auto entry = std::make_unique<SendTabToSelfEntry>(
guid, url, title, shared_time, device_name,
target_device_sync_cache_guid);
guid, url, pb_entry.title(), shared_time, pb_entry.device_name(),
pb_entry.target_device_sync_cache_guid());

if (pb_entry.opened()) {
entry->MarkOpened();
Expand Down
13 changes: 6 additions & 7 deletions components/send_tab_to_self/send_tab_to_self_entry_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ TEST(SendTabToSelfEntry, IsExpired) {
EXPECT_FALSE(entry.IsExpired(base::Time::FromTimeT(11)));
}

// Tests that the send tab to self entry rejects strings that are not utf8
// gracefully.
// Tests that the send tab to self entry rejects strings that are not utf8.
TEST(SendTabToSelfEntry, InvalidStrings) {
const char16_t term[1] = {u'\uFDD1'};
std::string invalid_utf8;
Expand All @@ -117,24 +116,24 @@ TEST(SendTabToSelfEntry, InvalidStrings) {
SendTabToSelfEntry invalid1("1", GURL("http://example.com"), invalid_utf8,
base::Time::FromTimeT(10), "device", "device");

EXPECT_EQ(std::string(), invalid1.GetGUID());
EXPECT_EQ("1", invalid1.GetGUID());

SendTabToSelfEntry invalid2(invalid_utf8, GURL("http://example.com"), "title",
base::Time::FromTimeT(10), "device", "device");

EXPECT_EQ(std::string(), invalid2.GetGUID());
EXPECT_EQ(invalid_utf8, invalid2.GetGUID());

SendTabToSelfEntry invalid3("1", GURL("http://example.com"), "title",
base::Time::FromTimeT(10), invalid_utf8,
"device");

EXPECT_EQ(std::string(), invalid3.GetGUID());
EXPECT_EQ("1", invalid3.GetGUID());

SendTabToSelfEntry invalid4("1", GURL("http://example.com"), "title",
base::Time::FromTimeT(10), "device",
invalid_utf8);

EXPECT_EQ(std::string(), invalid4.GetGUID());
EXPECT_EQ("1", invalid4.GetGUID());

std::unique_ptr<sync_pb::SendTabToSelfSpecifics> pb_entry =
std::make_unique<sync_pb::SendTabToSelfSpecifics>();
Expand All @@ -148,7 +147,7 @@ TEST(SendTabToSelfEntry, InvalidStrings) {
std::unique_ptr<SendTabToSelfEntry> invalid_entry(
SendTabToSelfEntry::FromProto(*pb_entry, base::Time::FromTimeT(10)));

EXPECT_EQ(invalid_entry, nullptr);
EXPECT_EQ(invalid_entry->GetGUID(), invalid_utf8);
}

// Tests that the send tab to self entry is correctly encoded to
Expand Down

0 comments on commit 94184d0

Please sign in to comment.