- From: Travis Leithead <notifications@github.com>
- Date: Mon, 03 Apr 2017 15:43:38 -0700
- To: w3ctag/spec-reviews <spec-reviews@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3ctag/spec-reviews/issues/152/291295498@github.com>
Read through the PaymentRequest API spec. Here's some notes. In general this spec is very functionally complete. I would have loved many more examples, as this API is highly async, and it was tricky building up the mental map of what happens when and how. An organization by "feature" might have made it much more readable, with a lead-in code example :) --- In `PaymentRequest(methodData, details, options) constructor MUST act as follows` Step 5 throws if `total.amount.value` is negative. Can't the merchant push a negative (return) transaction? (Same with 10.2.2.1.3). currencySystem - I read the current spec as the user agent doesn't validate this in any way, but passes along this string to a payment app as part of the payment app matching process. At what point would a bogus value fail? Would love to see another code example around the shipping updates process flow before delving into the details. Nit: `true is there is a pending updateWith() call` should be `if` (typo) Note: I like that `canMakePayment` works off post-validated constructor param data. I assume it was considered as a static method as well (in order to avoid allocating the `PaymentRequest` constructor). The current design centralizes parameter validation and simplifies what `canMakePayment` would otherwise have to do. Section 18 (Security): Pass-through strings that may be presented to the user are targets for attack. May want to collect this in the security considerations section. (E.g., some advice like: don't include serialized html strings, etc.) Or describe what sanitization steps the UA may want to take to reject suspicious strings before displaying them...? Bit of an API mis-alignment: `[...] then it should call updateWith() and provide a PaymentDetailsUpdate dictionary, or a promise for one, containing changed values that [...]` yet the API signature for `updateWith` requires a Promise with no option for just passing a `PaymentDetailsUpdate` dictionary. In the statement: `When setting the payerPhone value, the user agent SHOULD format the phone number to adhere to [E.164]. Otherwise, set it to null.` is the intent to set the value to null when the UA cannot format the number into [E.164] format? I wasn't sure how to read that sentence. In `Exposing available payment methods` there is mention about repeated calls to discover what payment methods are available, and the claim that `The fact that a successful match to a payment method causes a user interface to be displayed mitigates the disclosure risk.` I'm not sure I follow--`show` is the API to trigger the UI--before that paymentMethods dictionary is provided in the `PaymentRequest` constructor, and the `canMakePayment` is accessible independently of `show`--so how is this actually mitigated? --- Will move on to Payment Method Identifiers next... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/w3ctag/spec-reviews/issues/152#issuecomment-291295498
Received on Monday, 3 April 2017 22:44:10 UTC