Integrating Online Wallet into Chrome.

chrome/browser/autofill/wallet/full_wallet.cc

+// Copyright (c) 2012 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/autofill/wallet/full_wallet.h"
+
+#include "base/logging.h"
+#include "base/string_number_conversions.h"
+#include "base/values.h"
+

[Dan Beam, 2012/11/30 19:55:52]

nit: remove double \n

[ahutter, 2012/12/01 04:06:51]

Done.

+
+namespace {
+
+size_t kPanSize = 16;
+size_t kBinSize = 6;
+size_t kCvnSize = 3;
+
+}  // end anonymous namespace
+
+namespace wallet {
+
+FullWallet::FullWallet(int expiration_month,
+             int expiration_year,
+             const std::string& iin,
+             const std::string& encrypted_rest,
+             Address* billing_address,
+             Address* shipping_address,
+             std::vector<std::string> required_actions)
+    : expiration_month_(expiration_month),
+      expiration_year_(expiration_year),
+      iin_(iin),
+      encrypted_rest_(encrypted_rest),
+      billing_address_(billing_address),
+      shipping_address_(shipping_address),
+      required_actions_(required_actions) {}

[Dan Beam, 2012/11/30 19:55:52]

do you need to DCHECK() any of these pointers?

[ahutter, 2012/12/01 04:06:51]

Except for testing, this constructor should only be used in CreateFullWallet.

+
+FullWallet::~FullWallet() {
+}
+
+FullWallet* FullWallet::CreateFullWallet(DictionaryValue* dictionary) {

[Dan Beam, 2012/11/30 19:55:52]

nit: why isn't this also a scoped_ptr<FullWallet> or taking an out param (i.e.
bool FullWallet::CreateFullWallet(Dictionary*, FullWallet*))? put in another way
- who owns or deletes this memory?

[ahutter, 2012/12/01 04:06:51]

Header file comment specifies that the caller owns the memory.  Is there a
different style you would prefer to make that more clear.

[Dan Beam, 2012/12/05 19:34:59]

I guess if it's called Create*() that's implied enough.

+  DCHECK(dictionary);
+  ListValue* required_actions_list;
+  std::vector<std::string> required_actions;
+  if (dictionary->GetList("required_action", &required_actions_list)) {
+    for (size_t i = 0; i < required_actions_list->GetSize(); i++) {
+      std::string action;
+      if (required_actions_list->GetString(i, &action))
+        required_actions.push_back(action);
+    }
+    if (required_actions.size() > 0) {
+      return new FullWallet(-1, -1, "", "", NULL, NULL, required_actions);
+    }
+  } else {
+    VLOG(1) << "Response from Google wallet missing required actions";

[Dan Beam, 2012/11/30 19:55:52]

nit: DVLOG unless you want this to compile into release builds

[ahutter, 2012/12/01 04:06:51]

Done.

+  }
+
+  int expiration_month;
+  if (!dictionary->GetInteger("expiration_month", &expiration_month)) {
+    DLOG(ERROR) << "Response from Google wallet missing expiration month";
+    return NULL;
+  }

[Dan Beam, 2012/11/30 19:55:52]

nit: \n after each if() IMO

[ahutter, 2012/12/01 04:06:51]

Done.

+  int expiration_year;
+  if (!dictionary->GetInteger("expiration_year", &expiration_year)) {
+    DLOG(ERROR) << "Response from Google wallet missing expiration year";
+    return NULL;
+  }
+  std::string iin;
+  if (!dictionary->GetString("iin", &iin)) {
+    DLOG(ERROR) << "Response from Google wallet missing iin";
+    return NULL;
+  }
+  std::string encrypted_rest;
+  if (!dictionary->GetString("rest", &encrypted_rest)) {
+    DLOG(ERROR) << "Response from Google wallet missing rest";
+    return NULL;
+  }
+  DictionaryValue* billing_address_dict;
+  if (!dictionary->GetDictionary("billing_address", &billing_address_dict)) {
+    DLOG(ERROR) << "Response from Google wallet missing billing address";
+    return NULL;
+  }
+  Address* billing_address =
+      Address::CreateIdedAddress(billing_address_dict);
+  if (!billing_address) {
+    DLOG(ERROR) << "Response from Google wallet has malformed billing address";
+    return NULL;
+  }
+
+  DictionaryValue* shipping_address_dict;
+  Address* shipping_address;
+  if (dictionary->GetDictionary("shipping_address", &shipping_address_dict)) {
+    shipping_address =
+        Address::CreateIdedAddress(shipping_address_dict);
+  } else {
+    VLOG(1) << "Response from Google wallet missing shipping address";
+  }
+
+  return new FullWallet(expiration_month,
+                        expiration_year,
+                        iin,
+                        encrypted_rest,
+                        billing_address,
+                        shipping_address,
+                        required_actions);
+}
+
+bool FullWallet::operator==(const FullWallet& other) const {
+  if (expiration_month_ != other.expiration_month_)
+    return false;
+  if (expiration_year_ != other.expiration_year_)
+    return false;
+  if (iin_.compare(other.iin_) != 0)
+    return false;
+  if (encrypted_rest_.compare(other.encrypted_rest_) != 0)
+    return false;
+  if (billing_address_.get() && other.billing_address_.get()) {
+    if (*billing_address_.get() != *other.billing_address_.get())
+      return false;
+  } else if (billing_address_.get() || other.billing_address_.get()) {
+    return false;
+  }
+  if (shipping_address_.get() && other.shipping_address_.get()) {
+    if (*shipping_address_.get() != *other.shipping_address_.get())
+      return false;
+  } else if (shipping_address_.get() || other.shipping_address_.get()) {
+    return false;
+  }
+  if (required_actions_ != other.required_actions_)
+    return false;
+  return true;
+}
+
+bool FullWallet::operator!=(const FullWallet& other) const {
+  return !(*this == other);
+}
+
+const std::string FullWallet::GetPAN(void* otp, size_t length) {

[Dan Beam, 2012/11/30 19:55:52]

nit: generally abbreviations that are 3 or more letters are Camelcased rather
than ALLUPPER (i.e. prefer HttpUrl instead of HTTPURL)

[ahutter, 2012/12/01 04:06:51]

Done.

+  if (cvn_.empty())
+    DecryptCardInfo((uint8*) otp, length);
+  return pan_;
+}
+
+const std::string FullWallet::GetCVN(void* otp, size_t length) {
+  if (pan_.empty())
+    DecryptCardInfo((uint8*) otp, length);
+  return cvn_;
+}
+
+void FullWallet::DecryptCardInfo(uint8* otp, size_t length) {

[Dan Beam, 2012/11/30 19:55:52]

if you'd like to add comments to any of these steps, I think the reader'd
benefit. for instance, I have little to no way to verify your code works as we'd
anticipate just by looking at it as I have no idea what it should logically be
doing.

[ahutter, 2012/12/01 04:06:51]

Done.

+  std::vector<uint8> operating_data;
+  if (!base::HexStringToBytes(encrypted_rest_, &operating_data)) {
+    DLOG(ERROR) << "Failed to parse rest";
+    return;
+  }
+  DCHECK_EQ(length, operating_data.size());
+  uint8* result = new uint8[length];

[Dan Beam, 2012/11/30 19:55:52]

I'm a bit of a C++ n00b, but why can't you simply do

  uint8 result[length];

and then later:

  std::string hex_decrypted = base::HexEncode(&result, length);

[ahutter, 2012/12/01 04:06:51]

The linter doesn't like arrays the aren't sized by a constant.

+  for (size_t i = 0; i < length; i++)
+    result[i] = otp[i] ^ operating_data[i];
+  std::string hex_decrypted = base::HexEncode(result, length);
+  delete[] result;
+  int64 decrypted;
+  if (!base::HexStringToInt64(hex_decrypted, &decrypted)) {
+    DLOG(ERROR) << "Failed to parse decrypted";

[Dan Beam, 2012/11/30 19:55:52]

decrypted? maybe decrypted needs a better? sentences that end with descriptive

[ahutter, 2012/12/01 04:06:51]

Done.

+    return;
+  }
+  std::string card_info = base::Int64ToString(decrypted);

[Dan Beam, 2012/11/30 19:55:52]

nit: are we in a \n famine? :P

[ahutter, 2012/12/01 04:06:51]

Done.

+
+  size_t padded_length = (kPanSize - kBinSize + kCvnSize);

[Dan Beam, 2012/11/30 19:55:52]

nit: these () aren't necessary, and the C++ style guide recommends unnecessary
() for at least return values
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Return_Values)
so I'd say that's probably applicable...

[ahutter, 2012/12/01 04:06:51]

Done.

+  if (card_info.size() != padded_length)
+    card_info.insert(card_info.begin(), padded_length - card_info.size(), '0');
+
+  size_t split = kPanSize - kBinSize;
+  cvn_ = card_info.substr(split);
+  pan_ = iin_ + card_info.substr(0, split);
+}
+
+}  // end wallet namespace