Spec prohibits currency validation but doesn't define what it is

[adrianba]

In the definition of CurrencyAmount, the spec defines the currency field saying:

user agents MUST not attempt to validate this string.

The spec does not describe what it means to validate the string. It would be better if instead of a negative, the spec could describe positively what MUST happen. Presumably there is something bad that might happen as a result of some kind of validation? The spec is trying to avoid this and make sure that something does happen but it doesn't say what this is.

[adrianba]

This kind of requirement for user agents to not validate something is typically the kind of thing we would ignore in implementation. It is a common practice to do some kind of validation on all untrusted input. For example, if somebody passes in a huge multi-megabyte string for currency code then we will probably reject the call. We might also want to interpret the currency amount to do something in the user interface (e.g., not everyone knows that GBP is the same as £).

I propose that we remove this clause from the spec.

[nickjshearer]

Presumably there is something bad that might happen as a result of some kind of validation?

I have a feeling this is to do with currency codes that aren't valid international standards, especially in relation to Bitcoin. For example, in Swift NSLocale.ISOCurrencyCodes().contains("BTC") will evaluate to false. I guess the intent here was to prevent situations where user agents enforce the currency field contains an actual ISO code.

I wouldn't consider interpreting the field (for, say, displaying a currency symbol) as validation, but as you say, I think user agents will want to enforce requirements on that field, such as around length.

The wording should probably be revisited and modified, or we should add some constraints to the currency field around length and valid characters.

[sideshowbarker]

git blame shows that user agents MUST not attempt to validate this string requirement was added by @adrianhopebailie in 563ecd5, maybe in response to w3c/webpayments#57

So maybe @adrianhopebailie can shed some light on the rationale behind it.

[kirkalx]

See #101 for some discussion on this.

[adrianhopebailie]

This was put in to ensure that we don't restrict currency codes unneccessarily to only ISO approved codes.

In general I am not in favor of the API placing restrictions on the data it is passed unless there is a VERY strong rationale. Doing so forces the browser to understand more about the semantics of the data than should be necessary.

I am +1 to @adrianba 's suggestion of changing the wording to allow user agents to hygene check the data and avoid obviously wrong input such as massive strings or similar but would like to preserve the intent of the original edit.

I understand the need to understand some semantics for display purposes but think that the ability to pass in any new currency (like loyalty points, a new crypto-currency, or similar) is essential. Perhaps a future version of the API might provide a way for the website to pass in display hints for the currency such as a size limited string that can use UTF16 encoded symbols or 3 char strings?

Personally, I am waiting for the opportunity to invent my own new currency called Rainbows which I will accept as payment for my collection of unicorns and when I do that I'd like to be able to use U+1F308 or 🌈 as my currency code.

[adrianba]

I am +1 to @adrianba 's suggestion of changing the wording to allow user agents to hygene check the data and avoid obviously wrong input such as massive strings or similar but would like to preserve the intent of the original edit.

I am proposing to remove the text. I don't think it is useful. If someone wants to keep it then they should propose a positive description of what MUST happen and not try to describe what MUST NOT.

[adrianhopebailie]

PROPOSAL

Change the following text:

The most common identifiers are three-letter alphabetic codes as defined by [ISO4217] 
(for example, "USD" for US Dollars) however any string is considered valid and user agents 
MUST not attempt to validate this string.

to this:

The most common identifiers are three-letter alphabetic codes as defined by [ISO4217] 
(for example, "USD" for US Dollars) however any string SHOULD be considered valid to
accommodate other currency identifiers.

This addresses the desire to do some validation and also explains why we aren't restricting this to a 3 char alphanumeric.

[ianbjacobs]

It may be useful to be more explicit about conformance requirements from an "author" perspective and from an "implementation" perspective. What do you think of this change:

"Developers SHOULD identify currencies with the three-letter codes defined in [ISO4217]. User agents MUST NOT treat other values as errors because developers MAY identify currencies in other ways."

Ian

[halindrome]

That second "MAY" is fine, but shouldn't be uppercase. The may is implied by the initial SHOULD I think.

[adrianba]

User agents MUST NOT treat other values as errors

This is still phrased as a negative. It would be better to phrase it positively. In other words, "user agents MUST do something (but what?) with currency strings even if they don't recognize them".

[halindrome]

Hmm. Good point. From a testing perspective, it is arguably impossible to test a negative (on an arbitrary field).

[ianbjacobs]

@adrianba,

This is still phrased as a negative. It would be better to phrase it positively. In other words, "user agents MUST do something (but what?) with currency strings even if they don't recognize them".

Does the browser have anything "positive" to do here other than pass data to the payment app?

As far as "negative" behavior, I don't know what error handling (if any) we plan to
specify. RIght now the spec says " The following fields MUST be supplied for a CurrencyAmount to be valid:" but other than an implicit definition of "validity" there is
no user agent behavior specified. If we were to specify error handling, then it would make
sense to say as part of that "Non-ISO4271 strings are not errors."

Ian

[adrianba]

@ianbjacobs, as I've said, I don't think we need to say anything here and have proposed to remove this clause. The advocates for it obviously have something in mind that should happen. By saying that a user agent should not do something, they believe some positive thing should happen as a consequence. I'm not sure why this clause was added and I don't want to guess. If there actually is a positive expected outcome then the spec should describe it explicitly. Then we can determine whether that is a reasonable expectation.

[kirkalx]

currency is a string containing a currency identifier. Some identifiers will be three-letter alphabetic codes as defined by [[!ISO4217]](for example, "USD" for US Dollars), however not all currencies in use will have been issued such a code and some common currency identifiers (e.g. "BTC" for Bitcoin) are specifically non-compliant with that specification. Any currency code string is considered valid and user agents MUST NOT reject any
particular identifier value for any reason other than unreasonable length.

Anybody? Bueller?

[adamroach]

This issue ties in very closely with #185, and I think the proposal I make in there would provide a reasonably clear path forward. I'll reiterate them here:

1. Any three-letter code MUST be an ISO 4217 code
2. The payments API document will define a short list of codes of the form "+XXX" corresponding to well-known non-ISO-registered value instruments (e.g., "+BTC")
3. Any other currencies can be represented by a URL, as proposed in Currency Types and Rendering #185.

If we can come to consensus around that as a general direction, I'm happy to draft text to this effect in the form of a PR.

[ianbjacobs]

@adamroach,

I am ok with this and it seems developer friendly. Do implementers see too much parsing complexity?

Ian

[rsolomakhin]

Parsing this would be fine. We can improve the security of our
implementations by:

• Constraining to a reasonable length, for example 2048 bytes.
• Defining a format for the longer messages. For example, allowing only URL
  format.

--Rouslan

On Tue, May 17, 2016, 6:04 AM ianbjacobs notifications@github.com wrote:

@adamroach https://github.com/adamroach,

I am ok with this and it seems developer friendly. Do implementers see too
much parsing complexity?

Ian

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#175 (comment)