Implement CSVReader and CSVWriter to be used for password import and export.

components/password_manager/core/browser/import/csv_reader.cc

Index: components/password_manager/core/browser/import/csv_reader.cc
diff --git a/components/password_manager/core/browser/import/csv_reader.cc b/components/password_manager/core/browser/import/csv_reader.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8e6ebbdfc1cf541d2cf5bc9cb7bc163e07fd0b3a
--- /dev/null
+++ b/components/password_manager/core/browser/import/csv_reader.cc
@@ -0,0 +1,106 @@
+// Copyright 2014 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 "components/password_manager/core/browser/import/csv_reader.h"
+
+#include "base/logging.h"
+#include "base/strings/string_util.h"
+#include "third_party/re2/re2/re2.h"
+
+namespace {
+
+// Regular expression that matches and captures the first row in CSV formatted
+// data. Will throw away the potential trailing EOL character.
+const char kFirstRowRE[] =
+    // Non-greedily match and capture sequences of 1.) arbitrary characters
+    // inside correctly matched double-quotes, or 2.) characters other than the
+    // double-quote and EOL. Note that because literal double-quotes are escaped
+    // as two double-quotes, they do not need special treatment.

[vasilii, 2014/08/06 17:42:17]

Though the comment matches the regexp, it's not clear to me why it matches the
first row. What's the algorithm?
What is the sense of '*?' in the end?

[engedy, 2014/08/08 17:06:13]

PTAL.

+    "^((?:\"[^\"]*\"|[^\"\\n])*?)"
+    // Match and throw away EOL, or match end-of-string.
+    "(?:\n|$)";
+
+// Regular expression that matches and captures the value of the first field in
+// a CSV formatted row of data. Will throw away the potential trailing comma,
+// but not the enclosing double-quotes if the value is quoted.
+const char kFirstFieldRE[] =
+    // Non-greedily match and capture sequences of 1.) arbitrary characters
+    // inside correctly matched double-quotes, or 2.) characters other than the
+    // double-quote and the field separator comma (,).
+    "^((?:\"[^\"]*\"|[^\",])*?)"
+    // Match and throw away the field separator, or match end-of-string.
+    "(?:,|$)";
+
+// Parses and stores the fields of the next row in |remaining_csv_piece| into
+// |fields|.
+bool ParseNextCSVRow(re2::StringPiece* remaining_csv_piece,
+                     std::vector<std::string>* fields) {
+  static RE2 first_row_regex(kFirstRowRE);
+  static RE2 first_field_regex(kFirstFieldRE);

[vasilii, 2014/08/06 17:42:17]

This is thread unsafe.
I'm also pretty sure that we have compilation error for this case
(-Wexit-time-destructors)

[engedy, 2014/08/08 17:06:13]

I have moved this as non-static local variables to CSVReader::Read for now. Do
you have a better idea where these could live in order to avoid recompiling the
RE for all files?

[vasilii, 2014/08/11 11:56:38]

Do you really need it? If yes, there is LazyInstance<>.

[engedy, 2014/08/12 11:27:11]

I think LazyInstance<> cannot be used with classes whose constructors take
arguments.

Therefore I have introduced a CSVParser class here to hold instances of the
compiled RegExes, so at least we can re-use them for all rows of the same file.
I would prefer reusing them across files, ideally, but I do not see how that
would be possible :-(.

[vasilii, 2014/08/12 12:20:01]

You could hardcode the const char* constant into the type using templates but
don't do it. 99% users will never import passwords. The remaining 1% will import
it once during the session.

[engedy, 2014/08/12 13:02:53]

Agreed. The current solution with CSVParse seems fine, then.

+
+  fields->clear();
+
+  std::string row;
+  if (!RE2::Consume(remaining_csv_piece, first_row_regex, &row))
+    return false;
+
+  re2::StringPiece field;
+  re2::StringPiece remaining_row_piece(row);
+  while (!remaining_row_piece.empty()) {
+    if (!RE2::Consume(&remaining_row_piece, first_field_regex, &field))
+      return false;
+    if (field.starts_with("\"") && field.ends_with("\"")) {
+      CHECK_GE(field.size(), 2);
+      field.remove_prefix(1);
+      field.remove_suffix(1);
+    }
+    std::string field_copy(field.as_string());
+    ReplaceSubstringsAfterOffset(&field_copy, 0, "\"\"", "\"");
+    fields->push_back(std::string());
+    fields->back().swap(field_copy);
+  }
+  return true;
+}
+
+}  // namespace
+
+namespace password_manager {
+
+// static
+bool CSVReader::Read(const base::StringPiece& csv,
+                     std::vector<std::string>* column_names,
+                     std::vector<ColumnNameToValueMap>* records) {
+  DCHECK(column_names);
+  DCHECK(records);
+
+  column_names->clear();
+  records->clear();
+
+  // Normalize EOL sequences so that we uniformly use a single LF character.
+  std::string normalized_csv(csv.as_string());
+  ReplaceSubstringsAfterOffset(&normalized_csv, 0, "\r\n", "\n");

[vasilii, 2014/08/06 17:42:17]

What if the field contain \r\n. It's valid.

[engedy, 2014/08/08 17:06:13]

Hmm, do you have a particular case in mind where this might be a problem?

[vasilii, 2014/08/11 11:56:38]

Not really. Do you want to support the case when we have \n inside the field?

[engedy, 2014/08/12 11:27:11]

I do not think that this would be a particularly frequent use-case for our
purposes, but using the same reasoning as before (with how the C standard
library does it) I think normalizing line endings uniformly everywhere makes
sense.

+
+  // Read header row.
+  re2::StringPiece remaining_csv_piece(normalized_csv);
+  if (!ParseNextCSVRow(&remaining_csv_piece, column_names))
+    return false;
+
+  // Reader data records rows.
+  std::vector<std::string> fields;
+  while (!remaining_csv_piece.empty()) {
+    if (!ParseNextCSVRow(&remaining_csv_piece, &fields))
+        return false;

[vasilii, 2014/08/06 17:42:17]

indent

[engedy, 2014/08/08 17:06:13]

Done.

+
+    records->push_back(ColumnNameToValueMap());
+    for (size_t i = 0; i < column_names->size(); ++i) {
+      std::string& target = records->back()[(*column_names)[i]];

[vasilii, 2014/08/06 17:42:16]

column_names->at(i) looks nicer IMO

[tfarina, 2014/08/06 18:00:52]

might be nicer, but it throws an exception. I'd say to keep using operator[]

[Peter Kasting, 2014/08/07 03:59:50]

Well, it doesn't actually throw, since we've disabled exceptions.  But yes, in
general it would probably be better if people never used at(), since it doesn't
mean the same thing as operator[](), so when it's used in code it's never clear
if the author actually intended the difference or not.

[engedy, 2014/08/08 17:06:13]

Acknowledged.

+      if (i < fields.size())
+        target.swap(fields[i]);

[vasilii, 2014/08/06 17:42:17]

What if there are more fields than columns?

[engedy, 2014/08/08 17:06:13]

I have changed to behavior, PTAL: Extra fields are ignored. Missing fields will
have no corresponding key-value pair in the record's ColumnNameToValueMap.

+    }
+  }
+
+  return true;
+}
+
+}  // namespace password_manager