Skip to content

GitLab

T
tangled
Commit dca98e1e authored by rouslan's avatar rouslan Committed by Commit bot
Browse files
Options

Verify behavior of PaymentRequest constructor. 

This patch adds web platform tests that verify behavior of
PaymentRequest constructor. To match the spec, PaymentRequest
constructor in this patch has been changed to:
- Check for secure context before validating any parameters.
- Check the amount value first before checking the rest of the fields in
  each line item.
- Determine the selected shipping option even if requestShipping is
  false.
- Validate shipping options even if requestShipping is false.
- Validate shipping options before checking for duplicate shipping
  option IDs.
- Allow empty line item labels (but discourage via a console warning).
- Allow empty shipping option ID (but discourage via a console warning).
- Allow empty list of modifiers.

Because of changes in the WebIDL, certain conditions are not possible,
so they are ensured via DCHECK() in this patch instead of if statements
before this patch:
- Total is always present.
- Every line item has a label and amount.
- Every amount has value and currency.
- Every shipping option has an ID.
- Shipping type is always valid.

Invalid shipping type ("delivery", "shipping", or "pickup") is now
impossible because the WebIDL specifies it as an enum. Therefore,
there's no need for additional validation there.

To help developers better decipher the behavior of the API, error
messages have been improved in this patch. For example:
  "'-1/3' is not a valid amount for total."

Spec:
https://w3c.github.io/browser-payment-api/#constructor

BUG=705252

Review-Url: https://codereview.chromium.org/2851383002
Cr-Commit-Position: refs/heads/master@{#469369}
parent 31438b99
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 503 additions and 265 deletions
components/payments/content/payment_details_validation.cc
View file @ dca98e1e
......@@ -20,11 +20,6 @@ bool validateShippingOptionOrPaymentItem(
const T& item,
const payments::mojom::PaymentItemPtr& total,
std::string* error_message) {
if (item->label.empty()) {
*error_message = "Item label required";
return false;
}
if (!item->amount) {
*error_message = "Currency amount required";
return false;
......
third_party/WebKit/LayoutTests/external/wpt/payment-request/payment-request-constructor.https.html 0 → 100644
View file @ dca98e1e
<!DOCTYPE html>
<!-- Copyright © 2017 Chromium authors and World Wide Web Consortium, (Massachusetts Institute of Technology, ERCIM, Keio University, Beihang). -->
<meta charset="utf-8">
<title>Test for PaymentRequest Constructor</title>
<link rel="help" href="https://w3c.github.io/browser-payment-api/#constructor">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
'use strict';
test(() => {
assert_equals((new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
id: 'foo',
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
})).id, 'foo');
}, 'Use provided request ID');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
});
});
}, 'If the length of the methodData sequence is zero, then throw a TypeError');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: [],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
});
});
}, 'If the length of the paymentMethod.supportedMethods sequence is zero, ' +
'then throw a TypeError');
test(() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
data: ['some-data'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
});
}, 'Method data must be JSON-serializable object (a list in this case)');
test(() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
data: {
some: 'data',
},
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
});
}, 'Method data must be JSON-serializable object (a dictionary in this case)');
test(() => {
const recursiveDictionary = {};
recursiveDictionary.foo = recursiveDictionary;
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
data: recursiveDictionary,
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
});
});
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
data: 'a string',
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
});
});
}, 'Rethrow any exceptions of JSON-serializing paymentMethod.data into ' +
'a string');
const invalidMonetaryAmounts = ['-', 'notdigits', 'ALSONOTDIGITS', '10.', '.99',
'-10.', '-.99', '10-', '1-0', '1.0.0', '1/3', '', null,
];
for (const amount of invalidMonetaryAmounts) {
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: amount,
},
},
});
});
}, 'If details.total.amount.value is not a valid decimal monetary value ' +
'(in this case "' + amount + '"), then throw a TypeError');
}
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '-1.00',
},
},
});
});
}, 'If the first character of details.total.amount.value is ' +
'U+002D HYPHEN-MINUS, then throw a TypeError');
for (const amount of invalidMonetaryAmounts) {
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
displayItems: [{
label: '',
amount: {
currency: 'USD',
value: amount,
},
}],
});
});
}, 'For each item in details.displayItems: if item.amount.value is not ' +
'a valid decimal monetary value (in this case "' + amount +
'"), then throw a TypeError');
}
test(() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
modifiers: [{
supportedMethods: ['basic-card'],
data: ['some-data'],
}],
});
}, 'Modifier data must be JSON-serializable object (a list in this case)');
test(() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
modifiers: [{
supportedMethods: ['basic-card'],
data: {
some: 'data',
},
}],
});
}, 'Modifier data must be JSON-serializable object (a dictionary in this ' +
'case)');
test(() => {
const recursiveDictionary = {};
recursiveDictionary.foo = recursiveDictionary;
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
modifiers: [{
supportedMethods: ['basic-card'],
data: recursiveDictionary,
}],
});
});
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
modifiers: [{
supportedMethods: ['basic-card'],
data: 'a string',
}],
});
});
}, 'Rethrow any exceptions of JSON-serializing modifier.data into a string');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {});
});
}, 'Total is required');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
amount: {
currency: 'USD',
value: '1.00',
},
},
});
});
}, 'Label is required');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
},
});
});
}, 'Amount is required');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
},
},
});
});
}, 'Amount value is required');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
value: '1.00',
},
},
});
});
}, 'Amount currency is required');
test(() => {
assert_throws({
name: 'TypeError',
},
() => {
new PaymentRequest([{
supportedMethods: ['basic-card'],
}], {
total: {
label: '',
amount: {
currency: 'USD',
value: '1.00',
},
},
}, {
shippingType: 'invalid',
});
});
}, 'Shipping type should be valid');
</script>
third_party/WebKit/LayoutTests/payments/payment-request-interface-expected.txt
View file @ dca98e1e
......@@ -47,6 +47,7 @@ PASS Invalid basic card parameters should not throw even when method name is "ba
PASS Android Pay parameters for network token without environment key should not throw.
PASS Invalid Android Pay parameters should not throw when method name is not "https://android.com/pay".
PASS Invalid Android Pay parameters should not throw even when method name is "https://android.com/pay".
PASS Array value for payment method specific data parameter should not throw
PASS abort() without show() should reject with error
PASS PaymentRequest constructor should throw for incorrect parameter types.
PASS PaymentRequest constructor should throw for undefined required parameters.
......@@ -60,7 +61,6 @@ PASS Null supportedMethods in modifiers should throw TypeError.
PASS Undefined supportedMethods in modifiers should throw TypeError.
PASS Empty supportedMethods in modifiers should throw TypeError.
PASS Absence of supportedMethods in modifiers should throw TypeError.
PASS Empty modifiers should throw TypeError.
PASS Empty details should throw
PASS Null items should throw
PASS Null shipping options should throw
......@@ -69,7 +69,6 @@ PASS Null for shppingType should throw a TypeError
PASS Array value for shppingType should throw a TypeError
PASS Object value for shppingType should throw a TypeError
PASS Numeric value for shppingType should throw a TypeError
PASS Array value for payment method specific data parameter should throw
PASS String value for payment method specific data parameter should throw
PASS Numeric value for payment method specific data parameter should throw
PASS Infinite JSON value for one of the payment method specific data pieces should throw
......
third_party/WebKit/LayoutTests/payments/payment-request-interface.html
View file @ dca98e1e
......@@ -272,6 +272,10 @@ test(function() {
new PaymentRequest([{'supportedMethods': ['https://android.com/pay'], 'data': {'allowedCardNetworks': 0}}], buildDetails());
}, 'Invalid Android Pay parameters should not throw even when method name is "https://android.com/pay".');
test(function() {
new PaymentRequest([{'supportedMethods': ['foo'], 'data': []}], buildDetails());
}, 'Array value for payment method specific data parameter should not throw');
promise_test(function(t) {
return promise_rejects(t, null, new PaymentRequest([{'supportedMethods': ['foo']}], buildDetails()).abort());
}, 'abort() without show() should reject with error');
......@@ -313,9 +317,6 @@ generate_tests(assert_throws, [
['Absence of supportedMethods in modifiers should throw TypeError.', null, function() {
new PaymentRequest([{'supportedMethods': ['foo']}], {'total': buildItem(), 'modifiers': [{'total': buildItem()}]})
}],
['Empty modifiers should throw TypeError.', null, function() {
new PaymentRequest([{'supportedMethods': ['foo']}], {'total': buildItem(), 'modifiers': []})
}],
['Empty details should throw', null, function() {
new PaymentRequest([{'supportedMethods': ['foo']}], {})
}],
......@@ -342,9 +343,6 @@ generate_tests(assert_throws, [
}],
// Payment method specific data should be a JSON-serializable object.
['Array value for payment method specific data parameter should throw', null, function() {
new PaymentRequest([{'supportedMethods': ['foo'], 'data': []}], buildDetails(), {})
}],
['String value for payment method specific data parameter should throw', null, function() {
new PaymentRequest([{'supportedMethods': ['foo'], 'data': 'foo'}], buildDetails(), {})
}],
......
third_party/WebKit/Source/modules/payments/PaymentRequestDetailsTest.cpp
View file @ dca98e1e
......@@ -112,8 +112,7 @@ std::ostream& operator<<(std::ostream& out, DetailsTestCase test_case) {
switch (test_case.mod_type_) {
case kPaymentTestOverwriteValue:
out << "is overwritten by ";
out << test_case.value_to_use_;
out << "is overwritten by \"" << test_case.value_to_use_ << "\"";
break;
case kPaymentTestRemoveKey:
out << "is removed";
......@@ -143,106 +142,6 @@ TEST_P(PaymentRequestDetailsTest, ValidatesDetails) {
scope.GetExceptionState().Code());
}
INSTANTIATE_TEST_CASE_P(
MissingData,
PaymentRequestDetailsTest,
testing::Values(DetailsTestCase(kPaymentTestDetailTotal,
kPaymentTestDataAmount,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailTotal,
kPaymentTestDataValue,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailTotal,
kPaymentTestDataLabel,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailItem,
kPaymentTestDataAmount,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailItem,
kPaymentTestDataValue,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailItem,
kPaymentTestDataLabel,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailShippingOption,
kPaymentTestDataAmount,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailShippingOption,
kPaymentTestDataValue,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailShippingOption,
kPaymentTestDataId,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailShippingOption,
kPaymentTestDataLabel,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailModifierTotal,
kPaymentTestDataAmount,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailModifierTotal,
kPaymentTestDataValue,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailModifierTotal,
kPaymentTestDataLabel,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailModifierItem,
kPaymentTestDataAmount,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailModifierItem,
kPaymentTestDataValue,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError),
DetailsTestCase(kPaymentTestDetailModifierItem,
kPaymentTestDataLabel,
kPaymentTestRemoveKey,
"",
true,
kV8TypeError)));
INSTANTIATE_TEST_CASE_P(
EmptyData,
PaymentRequestDetailsTest,
......@@ -256,8 +155,7 @@ INSTANTIATE_TEST_CASE_P(
kPaymentTestDataLabel,
kPaymentTestOverwriteValue,
"",
true,
kV8TypeError),
false),
DetailsTestCase(kPaymentTestDetailItem,
kPaymentTestDataValue,
kPaymentTestOverwriteValue,
......@@ -268,8 +166,7 @@ INSTANTIATE_TEST_CASE_P(
kPaymentTestDataLabel,
kPaymentTestOverwriteValue,
"",
true,
kV8TypeError),
false),
DetailsTestCase(kPaymentTestDetailShippingOption,
kPaymentTestDataValue,
kPaymentTestOverwriteValue,
......@@ -280,14 +177,12 @@ INSTANTIATE_TEST_CASE_P(
kPaymentTestDataId,
kPaymentTestOverwriteValue,
"",
true,
kV8TypeError),
false),
DetailsTestCase(kPaymentTestDetailShippingOption,
kPaymentTestDataLabel,
kPaymentTestOverwriteValue,
"",
true,
kV8TypeError),
false),
DetailsTestCase(kPaymentTestDetailModifierTotal,
kPaymentTestDataValue,
kPaymentTestOverwriteValue,
......@@ -298,8 +193,7 @@ INSTANTIATE_TEST_CASE_P(
kPaymentTestDataLabel,
kPaymentTestOverwriteValue,
"",
true,
kV8TypeError),
false),
DetailsTestCase(kPaymentTestDetailModifierItem,
kPaymentTestDataValue,
kPaymentTestOverwriteValue,
......@@ -310,8 +204,7 @@ INSTANTIATE_TEST_CASE_P(
kPaymentTestDataLabel,
kPaymentTestOverwriteValue,
"",
true,
kV8TypeError)));
false)));
INSTANTIATE_TEST_CASE_P(
ValidCurrencyCodeFormat,
......
third_party/WebKit/Source/modules/payments/PaymentRequestTest.cpp
View file @ dca98e1e
......@@ -48,17 +48,6 @@ TEST(PaymentRequestTest, SupportedMethodListRequired) {
EXPECT_EQ(kV8TypeError, scope.GetExceptionState().Code());
}
TEST(PaymentRequestTest, TotalRequired) {
V8TestingScope scope;
MakePaymentRequestOriginSecure(scope.GetDocument());
PaymentRequest::Create(scope.GetExecutionContext(),
BuildPaymentMethodDataForTest(), PaymentDetailsInit(),
scope.GetExceptionState());
EXPECT_TRUE(scope.GetExceptionState().HadException());
EXPECT_EQ(kV8TypeError, scope.GetExceptionState().Code());
}
TEST(PaymentRequestTest, NullShippingOptionWhenNoOptionsAvailable) {
V8TestingScope scope;
MakePaymentRequestOriginSecure(scope.GetDocument());
......@@ -274,22 +263,6 @@ TEST(PaymentRequestTest, PickupShippingTypeWhenShippingTypeIsPickup) {
EXPECT_EQ("pickup", request->shippingType());
}
TEST(PaymentRequestTest, DefaultShippingTypeWhenShippingTypeIsInvalid) {
V8TestingScope scope;
MakePaymentRequestOriginSecure(scope.GetDocument());
PaymentDetailsInit details;
details.setTotal(BuildPaymentItemForTest());
PaymentOptions options;
options.setRequestShipping(true);
options.setShippingType("invalid");
PaymentRequest* request = PaymentRequest::Create(
scope.GetExecutionContext(), BuildPaymentMethodDataForTest(), details,
options, scope.GetExceptionState());
EXPECT_EQ("shipping", request->shippingType());
}
TEST(PaymentRequestTest, RejectShowPromiseOnInvalidShippingAddress) {
V8TestingScope scope;
PaymentRequestMockFunctionScope funcs(scope.GetScriptState());
......
third_party/WebKit/Source/modules/payments/PaymentsValidators.cpp
View file @ dca98e1e
......@@ -48,13 +48,16 @@ bool PaymentsValidators::IsValidCurrencyCodeFormat(
}
bool PaymentsValidators::IsValidAmountFormat(const String& amount,
const String& item_name,
String* optional_error_message) {
if (ScriptRegexp("^-?[0-9]+(\\.[0-9]+)?$", kTextCaseSensitive)
.Match(amount) == 0)
return true;
if (optional_error_message)
*optional_error_message = "'" + amount + "' is not a valid amount format";
if (optional_error_message) {
*optional_error_message =
"'" + amount + "' is not a valid amount format for " + item_name;
}
return false;
}
......
third_party/WebKit/Source/modules/payments/PaymentsValidators.h
View file @ dca98e1e
......@@ -30,6 +30,7 @@ class MODULES_EXPORT PaymentsValidators final {
// Returns true if |amount| is a valid currency code as defined in ISO 20022
// CurrencyAnd30Amount.
static bool IsValidAmountFormat(const String& amount,
const String& item_name,
String* optional_error_message);
// Returns true if |code| is a valid ISO 3166 country code.
......
third_party/WebKit/Source/modules/payments/PaymentsValidatorsTest.cpp
View file @ dca98e1e
......@@ -106,14 +106,16 @@ class PaymentsAmountValidatorTest : public testing::TestWithParam<TestCase> {};
TEST_P(PaymentsAmountValidatorTest, IsValidAmountFormat) {
String error_message;
EXPECT_EQ(GetParam().expected_valid, PaymentsValidators::IsValidAmountFormat(
GetParam().input, &error_message))
EXPECT_EQ(GetParam().expected_valid,
PaymentsValidators::IsValidAmountFormat(
GetParam().input, "test value", &error_message))
<< error_message;
EXPECT_EQ(GetParam().expected_valid, error_message.IsEmpty())
<< error_message;
EXPECT_EQ(GetParam().expected_valid,
PaymentsValidators::IsValidAmountFormat(GetParam().input, nullptr));
PaymentsValidators::IsValidAmountFormat(GetParam().input,
"test value", nullptr));
}
INSTANTIATE_TEST_CASE_P(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment