[rac] Use stale libaddressinput data if download fails

third_party/libaddressinput/chromium/cpp/src/retriever.cc

Index: third_party/libaddressinput/chromium/cpp/src/retriever.cc
diff --git a/third_party/libaddressinput/chromium/cpp/src/retriever.cc b/third_party/libaddressinput/chromium/cpp/src/retriever.cc
index eebba3eb2d78b9b5d590f6aa8a4ba2819d39d7c1..a767a443a734e0cc2da2a5f9a0374b92cf21efec 100644
--- a/third_party/libaddressinput/chromium/cpp/src/retriever.cc
+++ b/third_party/libaddressinput/chromium/cpp/src/retriever.cc
@@ -22,22 +22,141 @@
 
 #include <cassert>
 #include <cstddef>
+#include <cstdlib>
+#include <ctime>
 #include <map>
 #include <string>
-#include <utility>
 
 #include "fallback_data_store.h"
+#include "time_to_string.h"
+#include "util/md5.h"
 #include "util/stl_util.h"
 
 namespace i18n {
 namespace addressinput {
 
+namespace {
+
+// The number of seconds after which data is considered stale. The staleness
+// threshold is 30 days:
+//    30 days *
+//    24 hours per day *
+//    60 minutes per hour *
+//    60 seconds per minute.
+static const double kStaleDataAgeInSeconds = 30.0 * 24.0 * 60.0 * 60.0;
+
+// The prefix for the timestamp line in the header.
+const char kTimestampPrefix[] = "timestamp=";
+const size_t kTimestampPrefixLength = sizeof kTimestampPrefix - 1;
+
+// The prefix for the checksum line in the header.
+const char kChecksumPrefix[] = "checksum=";
+const size_t kChecksumPrefixLength = sizeof kChecksumPrefix - 1;
+
+// The separator between lines of header and data.
+const char kSeparator = '\n';
+
+// Places the header value into |header_value| parameter and erases the header
+// from |data|. Returns |true| if the header format is valid.
+bool UnwrapHeader(const char* header_prefix,

[Evan Stade, 2014/01/23 01:44:50]

nit: put this about Unwrap

also, s/UnwrapHeader/ExtractHeader

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

+                  size_t header_prefix_length,
+                  std::string* data,
+                  std::string* header_value) {
+  assert(header_prefix != NULL);
+  assert(data != NULL);
+  assert(header_value != NULL);
+
+  if (data->compare(
+          0, header_prefix_length, header_prefix, header_prefix_length) != 0) {
+    return false;
+  }
+
+  std::string::size_type separator_position =
+      data->find(kSeparator, header_prefix_length);
+  if (separator_position == std::string::npos) {
+    return false;
+  }
+
+  header_value->assign(
+      *data, header_prefix_length, separator_position - header_prefix_length);
+  data->erase(0, separator_position + 1);
+
+  return true;
+}
+
+// Returns |data| with attached checksum and current timestamp. Format:
+//
+//    timestamp=<timestamp>
+//    checksum=<checksum>
+//    <data>
+//
+// The timestamp is the time_t that was returned from time(NULL) function. The
+// timestamp does not need to be portable because it is written and read only by
+// Retriever. The value is somewhat human-readable: it is the number of seconds
+// since the epoch.
+//
+// The checksum is the 32-character hexadecimal MD5 checksum of <data>. It is
+// meant to protect from random file changes on disk.
+std::string Wrap(const std::string& data) {

[Evan Stade, 2014/01/23 01:44:50]

s/Wrap/PrependTimestamp

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

+  std::string wrapped;
+  wrapped.append(kTimestampPrefix, kTimestampPrefixLength);
+  wrapped.append(TimeToString(time(NULL)));
+  wrapped.push_back(kSeparator);
+
+  wrapped.append(kChecksumPrefix, kChecksumPrefixLength);
+  wrapped.append(MD5String(data));
+  wrapped.push_back(kSeparator);
+  wrapped.append(data);
+
+  return wrapped;
+}
+
+// Strips out the timestamp and checksum from |data|. Validates the checksum.
+// Compares the parsed timestamp with current time and saves the difference
+// into |age_in_seconds|.
+//
+// The parameters should not be NULL. Does not take ownership of its
+// parameters.
+//
+// Returns |true| if |data| is correctly formatted and has the correct
+// checksum.
+bool Unwrap(std::string* data, double* age_in_seconds) {

[Evan Stade, 2014/01/23 01:44:50]

I'd prefer if there were separate in and out params for |data| rather than an
in-out param.

[Evan Stade, 2014/01/23 01:44:50]

s/Unwrap/VerifyAndExtractTimestamp

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

+  assert(data != NULL);
+  assert(age_in_seconds != NULL);
+
+  std::string timestamp_string;
+  if (!UnwrapHeader(
+           kTimestampPrefix, kTimestampPrefixLength, data, &timestamp_string)) {
+    return false;
+  }
+
+  time_t timestamp = atol(timestamp_string.c_str());
+  if (timestamp < 0) {
+    return false;
+  }
+
+  *age_in_seconds = difftime(time(NULL), timestamp);
+  if (*age_in_seconds < 0.0) {
+    return false;
+  }
+
+  std::string checksum;
+  if (!UnwrapHeader(kChecksumPrefix, kChecksumPrefixLength, data, &checksum)) {
+    return false;
+  }
+
+  return checksum == MD5String(*data);
+}
+
+}  // namespace
+
 Retriever::Retriever(const std::string& validation_data_url,
                      scoped_ptr<Downloader> downloader,
                      scoped_ptr<Storage> storage)
     : validation_data_url_(validation_data_url),
       downloader_(downloader.Pass()),
-      storage_(storage.Pass()) {
+      storage_(storage.Pass()),
+      stale_data_() {
   assert(validation_data_url_.length() > 0);
   assert(validation_data_url_[validation_data_url_.length() - 1] == '/');
   assert(storage_ != NULL);
@@ -66,14 +185,18 @@ void Retriever::Retrieve(const std::string& key,
 void Retriever::OnDataRetrievedFromStorage(bool success,
                                            const std::string& key,
                                            const std::string& stored_data) {
-  // TODO(rouslan): Add validation for data integrity and freshness. If a
-  // download fails, then it's OK to use stale data.
-  if (success) {
+  std::string unwrapped = stored_data;
+  double age_in_seconds = 0.0;
+  bool valid_format = Unwrap(&unwrapped, &age_in_seconds);
+  if (success && valid_format && age_in_seconds < kStaleDataAgeInSeconds) {

[Evan Stade, 2014/01/23 01:44:50]

if (!success) you shouldn't call Unwrap

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

     scoped_ptr<Callback> retrieved = GetCallbackForKey(key);
     if (retrieved != NULL) {
-      (*retrieved)(success, key, stored_data);
+      (*retrieved)(success, key, unwrapped);
     }
   } else {

[Evan Stade, 2014/01/23 01:44:50]

nit: prefer early return over else {} that goes to the end of the function.

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

+    if (success && valid_format) {
+      stale_data_[key] = unwrapped;
+    }
     downloader_->Download(GetUrlForKey(key),
                           BuildCallback(this, &Retriever::OnDownloaded));
   }
@@ -84,13 +207,23 @@ void Retriever::OnDownloaded(bool success,
                              const std::string& downloaded_data) {
   const std::string& key = GetKeyForUrl(url);
   std::string response;
+  std::map<std::string, std::string>::iterator stale_data_it =
+      stale_data_.find(key);
+
   if (success) {
-    storage_->Put(key, downloaded_data);
+    storage_->Put(key, Wrap(downloaded_data));
     response = downloaded_data;

[Evan Stade, 2014/01/23 01:44:50]

unnecessary copy (potentially quite large)

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

+  } else if (stale_data_it != stale_data_.end()) {
+    success = true;
+    response = stale_data_it->second;

[Evan Stade, 2014/01/23 01:44:50]

unnecessary copy (potentially quite large)

[please use gerrit instead, 2014/01/23 23:11:05]

Done.

   } else {
     success = FallbackDataStore::Get(key, &response);
   }
 
+  if (stale_data_it != stale_data_.end()) {
+    stale_data_.erase(stale_data_it);
+  }
+
   scoped_ptr<Callback> retrieved = GetCallbackForKey(key);
   if (retrieved != NULL) {
     (*retrieved)(success, key, response);