Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(245)

Unified Diff: chrome/browser/spellchecker/spellcheck_custom_dictionary.cc

Issue 11414282: Improve reliability of custom spelling dictionary file. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Created 8 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
diff --git a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
index 4510332e59e3032ee7b35bf26f640ff6f3f2885c..27b1d9754094dc39f2f3720b6d066553ac0f83b8 100644
--- a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
+++ b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
@@ -7,6 +7,8 @@
#include <functional>
#include "base/file_util.h"
+#include "base/files/important_file_writer.h"
+#include "base/md5.h"
#include "base/string_split.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_constants.h"
@@ -17,6 +19,41 @@
using content::BrowserThread;
using chrome::spellcheck_common::WordList;
+namespace {
+
+const FilePath::CharType BACKUP_EXTENSION[] = FILE_PATH_LITERAL("backup");
+const char CHECKSUM_PREFIX[] = "checksum_v1 = ";
+
+// Loads the lines from the file at |file_path| into the |lines| container. If
+// the file has a valid checksum, then returns |true|. If the file has an
+// invalid checksum, then returns |false| and clears |lines|.
+bool LoadFile(FilePath file_path, std::vector<std::string>* lines) {
+ lines->clear();
+ std::string contents;
+ file_util::ReadFileToString(file_path, &contents);
+ size_t pos = contents.rfind(CHECKSUM_PREFIX);
+ if (pos != std::string::npos) {
+ std::string checksum = contents.substr(pos + strlen(CHECKSUM_PREFIX));
+ contents = contents.substr(0, pos);
+ if (checksum != base::MD5String(contents))
+ return false;
+ }
+ TrimWhitespaceASCII(contents, TRIM_ALL, &contents);
+ base::SplitString(contents, '\n', lines);
+ return true;
+}
+
+bool IsValidWord(const std::string& word) {
+ return IsStringUTF8(word) && word.length() <= 128 && word.length() > 0 &&
+ std::string::npos == word.find_first_of(kWhitespaceASCII);
+}
+
+bool IsInvalidWord(const std::string& word) {
+ return !IsValidWord(word);
+}
+
+} // namespace
+
SpellcheckCustomDictionary::SpellcheckCustomDictionary(Profile* profile)
: SpellcheckDictionary(profile),
custom_dictionary_path_(),
@@ -49,35 +86,21 @@ void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
WordList* custom_words) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
- std::string contents;
- file_util::ReadFileToString(custom_dictionary_path_, &contents);
- if (contents.empty()) {
- custom_words->clear();
+ LoadDictionaryFileReliably(custom_words);
+ if (custom_words->empty())
return;
- }
-
- base::SplitString(contents, '\n', custom_words);
- // Erase duplicates.
+ // Clean up the dictionary file contents by removing duplicates and invalid
+ // words.
std::sort(custom_words->begin(), custom_words->end());
custom_words->erase(std::unique(custom_words->begin(), custom_words->end()),
custom_words->end());
+ custom_words->erase(std::remove_if(custom_words->begin(),
+ custom_words->end(),
+ IsInvalidWord),
+ custom_words->end());
- // Clear out empty words.
- custom_words->erase(remove_if(custom_words->begin(), custom_words->end(),
- mem_fun_ref(&std::string::empty)), custom_words->end());
-
- // Write out the clean file.
- std::stringstream ss;
- for (WordList::iterator it = custom_words->begin();
- it != custom_words->end();
- ++it) {
- ss << *it << '\n';
- }
- contents = ss.str();
- file_util::WriteFile(custom_dictionary_path_,
- contents.c_str(),
- contents.length());
+ SaveDictionaryFileReliably(*custom_words);
}
void SpellcheckCustomDictionary::SetCustomWordList(WordList* custom_words) {
@@ -87,13 +110,15 @@ void SpellcheckCustomDictionary::SetCustomWordList(WordList* custom_words) {
if (custom_words)
std::swap(words_, *custom_words);
- std::vector<Observer*>::iterator it;
- for (it = observers_.begin(); it != observers_.end(); ++it)
- (*it)->OnCustomDictionaryLoaded();
+ FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryLoaded());
}
bool SpellcheckCustomDictionary::AddWord(const std::string& word) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (!IsValidWord(word)) {
+ NOTREACHED();
groby-ooo-7-16 2012/12/11 19:25:48 nit: Why is this NOTREACHED? Are we really guarant
please use gerrit instead 2012/12/11 22:32:58 Removed.
+ return false;
+ }
if (!CustomWordAddedLocally(word))
return false;
@@ -108,9 +133,7 @@ bool SpellcheckCustomDictionary::AddWord(const std::string& word) {
i.GetCurrentValue()->Send(new SpellCheckMsg_WordAdded(word));
}
- std::vector<Observer*>::iterator it;
- for (it = observers_.begin(); it != observers_.end(); ++it)
- (*it)->OnCustomDictionaryWordAdded(word);
+ FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryWordAdded(word));
return true;
}
@@ -118,6 +141,7 @@ bool SpellcheckCustomDictionary::AddWord(const std::string& word) {
bool SpellcheckCustomDictionary::CustomWordAddedLocally(
const std::string& word) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(IsValidWord(word));
WordList::iterator it = std::find(words_.begin(), words_.end(), word);
if (it == words_.end()) {
@@ -131,22 +155,20 @@ bool SpellcheckCustomDictionary::CustomWordAddedLocally(
void SpellcheckCustomDictionary::WriteWordToCustomDictionary(
const std::string& word) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+ DCHECK(IsValidWord(word));
- // Stored in UTF-8.
- DCHECK(IsStringUTF8(word));
-
- std::string word_to_add(word + "\n");
- if (!file_util::PathExists(custom_dictionary_path_)) {
- file_util::WriteFile(custom_dictionary_path_, word_to_add.c_str(),
- word_to_add.length());
- } else {
- file_util::AppendToFile(custom_dictionary_path_, word_to_add.c_str(),
- word_to_add.length());
- }
+ WordList custom_words;
+ LoadDictionaryFileReliably(&custom_words);
+ custom_words.push_back(word);
+ SaveDictionaryFileReliably(custom_words);
}
bool SpellcheckCustomDictionary::RemoveWord(const std::string& word) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (!IsValidWord(word)) {
+ NOTREACHED();
+ return false;
+ }
if (!CustomWordRemovedLocally(word))
return false;
@@ -161,9 +183,7 @@ bool SpellcheckCustomDictionary::RemoveWord(const std::string& word) {
i.GetCurrentValue()->Send(new SpellCheckMsg_WordRemoved(word));
}
- std::vector<Observer*>::iterator it;
- for (it = observers_.begin(); it != observers_.end(); ++it)
- (*it)->OnCustomDictionaryWordRemoved(word);
+ FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryWordRemoved(word));
return true;
}
@@ -171,6 +191,7 @@ bool SpellcheckCustomDictionary::RemoveWord(const std::string& word) {
bool SpellcheckCustomDictionary::CustomWordRemovedLocally(
const std::string& word) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(IsValidWord(word));
WordList::iterator it = std::find(words_.begin(), words_.end(), word);
if (it != words_.end()) {
@@ -183,40 +204,32 @@ bool SpellcheckCustomDictionary::CustomWordRemovedLocally(
void SpellcheckCustomDictionary::EraseWordFromCustomDictionary(
const std::string& word) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
- DCHECK(IsStringUTF8(word));
+ DCHECK(IsValidWord(word));
WordList custom_words;
- LoadDictionaryIntoCustomWordList(&custom_words);
-
- const char empty[] = {'\0'};
- const char separator[] = {'\n', '\0'};
- file_util::WriteFile(custom_dictionary_path_, empty, 0);
- for (WordList::iterator it = custom_words.begin();
- it != custom_words.end();
- ++it) {
- std::string word_to_add = *it;
- if (word.compare(word_to_add) != 0) {
- file_util::AppendToFile(custom_dictionary_path_, word_to_add.c_str(),
- word_to_add.length());
- file_util::AppendToFile(custom_dictionary_path_, separator, 1);
- }
- }
+ LoadDictionaryFileReliably(&custom_words);
+ if (custom_words.empty())
+ return;
+
+ WordList::iterator it = std::find(custom_words.begin(),
+ custom_words.end(),
+ word);
+ if (it != custom_words.end())
+ custom_words.erase(it);
+
+ SaveDictionaryFileReliably(custom_words);
}
void SpellcheckCustomDictionary::AddObserver(Observer* observer) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- observers_.push_back(observer);
+ observers_.AddObserver(observer);
}
void SpellcheckCustomDictionary::RemoveObserver(Observer* observer) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- std::vector<Observer*>::iterator it = std::find(observers_.begin(),
- observers_.end(),
- observer);
- if (it != observers_.end())
- observers_.erase(it);
+ observers_.RemoveObserver(observer);
}
WordList* SpellcheckCustomDictionary::LoadDictionary() {
@@ -234,3 +247,43 @@ void SpellcheckCustomDictionary::SetCustomWordListAndDelete(
SetCustomWordList(custom_words);
delete custom_words;
}
+
+void SpellcheckCustomDictionary::LoadDictionaryFileReliably(
+ WordList* custom_words) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+ // Load the contents and verify the checksum.
+ if (LoadFile(custom_dictionary_path_, custom_words))
+ return;
+
+ // Checksum is not valid. See if there's a backup.
+ FilePath backup = custom_dictionary_path_.AddExtension(BACKUP_EXTENSION);
+ if (!file_util::PathExists(backup))
+ return;
+
+ // Load the backup and verify its checksum.
+ if (!LoadFile(backup, custom_words))
+ return;
+
+ // Backup checksum is valid. Restore the backup.
+ file_util::CopyFile(backup, custom_dictionary_path_);
+}
+
+void SpellcheckCustomDictionary::SaveDictionaryFileReliably(
+ const WordList& custom_words) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+ std::stringstream content;
+ for (WordList::const_iterator it = custom_words.begin();
+ it != custom_words.end();
+ ++it) {
+ content << *it << '\n';
+ }
+ std::string checksum = base::MD5String(content.str());
+ content << CHECKSUM_PREFIX << checksum;
+
+ file_util::CopyFile(custom_dictionary_path_,
+ custom_dictionary_path_.AddExtension(BACKUP_EXTENSION));
+ base::ImportantFileWriter::WriteFileAtomically(custom_dictionary_path_,
+ content.str());
+}

Powered by Google App Engine
This is Rietveld 408576698