w3c / payment-method-id

Payment Method Identifiers specification
https://w3c.github.io/payment-method-id/
Other
23 stars 20 forks source link

Changes to address Action #40 #21

Closed adrianhopebailie closed 7 years ago

adrianhopebailie commented 7 years ago

The spec has been updated to refer to Secure Contexts concept of a potentially secure URL. https://www.w3.org/Payments/WG/track/actions/40 and #17

The inline issue regarding locating the manifest (related to #19) has been updated to indicate that the mechanism for locating the manifest will be defined in the manifest specification. https://www.w3.org/Payments/WG/track/actions/50

ianbjacobs commented 7 years ago

Hi @adrianhopebailie,

Several comments:

OLD: <div class="issue" title="Payment method manifest specification in progress">The Web Payments Working Group is developing a payment method manifest specification for the information a payment method owner would want to publish. Payment method identifiers that are URLs will help user agents locate these manifest files using a mechanism defined in that specification. <a href="https://github.com/zkoch/zkoch.github.io/blob/master/payment-manifest.md">Payment Manifest Proposal</a>.</div>

NEW: <div class="issue" title="Payment method manifest specification in progress">The Web Payments Working Group is developing a Payment Method Manifest specification for machine readable information that describes how that method participates in the PaymentRequest ecosystem. That specification will define a mechanism to discover a payment method manifest file given a payment method identifier.</div>

=========

Your new text around syntax is overly constraining:

"When URLs are used for payment method identifiers they MUST be absolute URLs including a scheme, host, port and path. 

The text is overly constraining because some of those parts are optional anyway. Suggested alternative:

When URLs are used for payment method identifiers they MUST be absolute URLs that include at most the following URL components: scheme, host, port, and path.

=======

Please delete this:

The port MAY be excluded if the scheme is a special scheme.

It is no longer necessary given the previous edit, and we should also not be defining general URL rules; that's out of our scope.

==========

IMPORTANT: You implemented something other than what the WG resolved. The WG resolved that URLs that do not conform to the syntax limitations will be thrown out during matching. Instead, you implemented an algorithm that ignores the query and fragment, but does not throw out the entire URL. Please fix that.

This sentence will also need to change:

Identifier URLs SHOULD have null query and fragment values as these will be ignored when matching identifiers.

I like your idea of updating the algorithm. The algorithm should say something like "If either URL includes any components other than those permitted by this specification, the test returns false."

If you want to keep a sentence about the impact of the syntax restriction on matching, you could say this (taking all the edits into account):

When URLs are used for payment method identifiers they MUST be absolute URLs that include at most the following URL components: scheme, host, port, and path. URLs that include any other components MUST be ignored during matching. The URL MUST be a potentially trustworthy URL as defined in the [[SECURE-CONTEXTS]] specification.

adrianhopebailie commented 7 years ago

The WG resolved that URLs that do not conform to the syntax limitations will be thrown out during matching. Instead, you implemented an algorithm that ignores the query and fragment, but does not throw out the entire URL.

That was not my understanding of the WG resolution so perhaps we need to raise this on the next call?

ianbjacobs commented 7 years ago

Hi Adrian,

Here's the suggestion from Rouslan that I thought captured the requirement [1]:

"I suggest we preserve the original intent of requiring no fragment identifiers or query string. I suggest that the user agent ignore any URL that has a fragment / query string rather than ignoring the offending parts... that reduces the cost of parsing... browser can do a match on the URL and reject it if not well-formed"

Thus, the UA would reject the entire URL (and not match with it).

+1 to double checking at the next call.

Ian

[1] https://www.w3.org/2017/01/26-wpwg-minutes#item01

adrianhopebailie commented 7 years ago

You're right, but even though I was on the call I didn't pick up that subtle difference. Let's take to the group and get a final resolution.

adrianhopebailie commented 7 years ago

Updated per WG resolution to discard URL's that contain a query string or fragment.

@ianbjacobs can you re-review please