[Web Payments] Common Payments validation

chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java

 // Copyright 2016 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.
 
 package org.chromium.chrome.browser.payments;
 
 import android.app.Activity;
 import android.graphics.Bitmap;
 import android.os.Handler;
 import android.text.TextUtils;
@@ skipping 523 matching lines @@
     /**
      * Sets the total, display line items, and shipping options based on input and returns the
      * status boolean. That status is true for valid data, false for invalid data. If the input is
      * invalid, disconnects from the client. Both raw and UI versions of data are updated.
      *
      * @param details The total, line items, and shipping options to parse, validate, and save in
      *                member variables.
      * @return True if the data is valid. False if the data is invalid.
      */
     private boolean parseAndValidateDetailsOrDisconnectFromClient(PaymentDetails details) {
-        if (details == null) {
-            disconnectFromClientWithDebugMessage("Payment details required");
+        if (!PaymentValidator.validatePaymentDetails(details)) {
+            disconnectFromClientWithDebugMessage("Invalid payment details");
             recordAbortReasonHistogram(
                     PaymentRequestMetrics.ABORT_REASON_INVALID_DATA_FROM_RENDERER);
             return false;
-        }
-
-        if (!hasAllPaymentItemFields(details.total)) {
-            disconnectFromClientWithDebugMessage("Invalid total");
-            recordAbortReasonHistogram(
-                    PaymentRequestMetrics.ABORT_REASON_INVALID_DATA_FROM_RENDERER);
-            return false;
         }
 
         String totalCurrency = details.total.amount.currency;
         CurrencyStringFormatter formatter =
                 new CurrencyStringFormatter(totalCurrency, Locale.getDefault());
 
-        if (!formatter.isValidAmountCurrencyCode(details.total.amount.currency)) {
-            disconnectFromClientWithDebugMessage("Invalid total amount currency");
-            recordAbortReasonHistogram(
-                    PaymentRequestMetrics.ABORT_REASON_INVALID_DATA_FROM_RENDERER);
-            return false;
-        }
-
-        if (!formatter.isValidAmountValue(details.total.amount.value)
-                || details.total.amount.value.startsWith("-")) {
-            disconnectFromClientWithDebugMessage("Invalid total amount value");
-            recordAbortReasonHistogram(
-                    PaymentRequestMetrics.ABORT_REASON_INVALID_DATA_FROM_RENDERER);
-            return false;
-        }
-
         // Total is never pending.
         LineItem uiTotal = new LineItem(
                 details.total.label, formatter.getFormattedCurrencyCode(),
                 formatter.format(details.total.amount.value), /* isPending */ false);
 
         List<LineItem> uiLineItems = getValidatedLineItems(details.displayItems, totalCurrency,
                 formatter);
-        if (uiLineItems == null) {
-            disconnectFromClientWithDebugMessage("Invalid line items");
-            recordAbortReasonHistogram(
-                    PaymentRequestMetrics.ABORT_REASON_INVALID_DATA_FROM_RENDERER);
-            return false;
-        }
 
         mUiShoppingCart = new ShoppingCart(uiTotal, uiLineItems);
         mRawTotal = details.total;
         mRawLineItems = Arrays.asList(details.displayItems);
 
         mUiShippingOptions = getValidatedShippingOptions(details.shippingOptions, totalCurrency,
                 formatter);
-        if (mUiShippingOptions == null) {
-            disconnectFromClientWithDebugMessage("Invalid shipping options");
-            recordAbortReasonHistogram(
-                    PaymentRequestMetrics.ABORT_REASON_INVALID_DATA_FROM_RENDERER);
-            return false;
-        }
 
         return true;
     }
 
     /**
      * Returns true if all fields in the payment item are non-null and non-empty.
      *
      * @param item The payment item to examine.
      * @return True if all fields are present and non-empty.
      */
     private static boolean hasAllPaymentItemFields(PaymentItem item) {
         // "label", "currency", and "value" should be non-empty.
         return item != null && !TextUtils.isEmpty(item.label) && item.amount != null
                 && !TextUtils.isEmpty(item.amount.currency)
                 && !TextUtils.isEmpty(item.amount.value);
     }
 
     /**
      * Validates a list of payment items and returns their parsed representation or null if invalid.

[please use gerrit instead, 2016/10/31 14:31:15]

s/Validates/Converts/

Please remove "or null if invalid".

[Kevin Bailey, 2016/10/31 20:32:00]

Done.

      *
      * @param items The payment items to parse and validate.
      * @param totalCurrency The currency code for the total amount of payment.
      * @param formatter A formatter and validator for the currency amount value.
      * @return A list of valid line items or null if invalid.

[please use gerrit instead, 2016/10/31 14:31:15]

Please remove "or null if invalid" from this comment.

[Kevin Bailey, 2016/10/31 20:32:00]

Done.

      */
     private static List<LineItem> getValidatedLineItems(

[please use gerrit instead, 2016/10/31 14:31:15]

Please remove the "validated" keyword from the function. This function should no
longer do any validation. That's being done inside of
PaymentValidator.validatePaymentDetails() now.

[Kevin Bailey, 2016/10/31 20:32:00]

Done.

             PaymentItem[] items, String totalCurrency, CurrencyStringFormatter formatter) {
         // Line items are optional.
         if (items == null) return new ArrayList<>();
 
         List<LineItem> result = new ArrayList<>(items.length);
         for (int i = 0; i < items.length; i++) {
             PaymentItem item = items[i];
 
             if (!hasAllPaymentItemFields(item)) return null;

[please use gerrit instead, 2016/10/31 14:31:14]

You can either assert that this is true or remove this statement all together.
This function should no longer return null.

[Kevin Bailey, 2016/10/31 20:32:00]

Done.

 
             // All currencies must match.
             if (!item.amount.currency.equals(totalCurrency)) return null;

[please use gerrit instead, 2016/10/31 14:31:14]

Ditto

[Kevin Bailey, 2016/10/31 20:32:00]

Done.

 
             // Value should be in correct format.
             if (!formatter.isValidAmountValue(item.amount.value)) return null;

[please use gerrit instead, 2016/10/31 14:31:15]

Ditto

[Kevin Bailey, 2016/10/31 20:32:00]

Done.

 
             result.add(new LineItem(
                     item.label, "", formatter.format(item.amount.value), item.pending));
         }
 
         return result;
     }
 
     /**

[please use gerrit instead, 2016/10/31 14:31:15]

Same validation comments for this function.

[Kevin Bailey, 2016/10/31 20:32:00]

Done.

      * Validates a list of shipping options and returns their parsed representation or null if
      * invalid.
      *
      * @param options The raw shipping options to parse and validate.
      * @param totalCurrency The currency code for the total amount of payment.
      * @param formatter A formatter and validator for the currency amount value.
      * @return The UI representation of the shipping options or null if invalid.
      */
     private static SectionInformation getValidatedShippingOptions(PaymentShippingOption[] options,
             String totalCurrency, CurrencyStringFormatter formatter) {
@@ skipping 570 matching lines @@
                 "PaymentRequest.CheckoutFunnel.Aborted", abortReason,
                 PaymentRequestMetrics.ABORT_REASON_MAX);
 
         if (abortReason == PaymentRequestMetrics.ABORT_REASON_ABORTED_BY_USER) {
             mJourneyLogger.recordJourneyStatsHistograms("UserAborted");
         } else {
             mJourneyLogger.recordJourneyStatsHistograms("OtherAborted");
         }
     }
 }