Error handling while parsing policy strings about webappsec-permissions-policy HOT 6 CLOSED

It's a bit out of date, but from our existing draft...

Future versions of this document may define additional such members and user agents must ignore unknown members when parsing the object.

I feel pretty strongly that we should be ignoring, for the sake of future compatibility, any unrecognized feature names. This way, any implementation can add new features that only they support, and other browsers can ignore them.

Agreed.

Similarly (though less strongly,) if we see an origin that we don't recognize, or doesn't even look like an origin, we should ignore that as well. Currently, we have two non-originy-origins, "self" and "*". I can imagine that in the future, there might be a need for a third one, or that some browsers may recognize as origins strings that other ones don't. In that case, it would be best to ignore them if we encounter them.

Agreed.

On the other hand, some errors might render a whole part of the feature policy string unusable.

Three cases here..

1. Malformed JFV -> abort entire policy.
2. Valid JFV but invalid format (e.g. not an Array): we can ignore the directive, or abort the policy.
3. Valid JFV and policy format but unrecognized feature/origin values.. ignore feature and all of its origins or just the origin we don't recognize.

from webappsec-permissions-policy.

I don't have strong opinions on this, but it sounds kind of like CSS parsing error handling. There's a lot we did wrong there, so it'd be good to avoid the same mistakes. @tabatkins @esprehn might have thoughts from CSS land.

I agree that unknown features and unknown origins should be ignored.

from webappsec-permissions-policy.

CSS did two big things right:

1. Defined error-handling in a good way such that you can pick up and keep going at the next reasonable spot; this lets us introduce totally new types of syntax that were unexpected before (like the new --foo properties).
2. Allow most named things to be repeated, with only the last recognized repetition being used. This way you can write something both in the hot-new-syntax, and in the old syntax that doesn't have as much features, and have it work in both old and new agents.

It's fine to not go quite as hard on 1, and enforce reasonable syntax forms if the syntax is expected to be very stable. I think this is just JSON? If so, anything that's invalid JSON should be ignored, and valid JSON that has the wrong "shape" should have the wrong-shaped parts ignored, but the rest kept. (In your example, the "geolocation" key should be ignored for not having an Array value, but other keys should be kept.)

Adding 2 would be great, so you can specify multiple "geolocation" keys and we use the last one that's recognized - that way we can add the new object form in the future and still provide a less-powerful array value for legacy UAs.

from webappsec-permissions-policy.

Yeah...that matches my intuition as well. We shouldn't have a custom parser here. Let's just have normal JSON. That seems good for the platform as a whole to minimize custom parsing. Any keys or values that are invalid are just ignored.

That work for everyone?

from webappsec-permissions-policy.

👍

from webappsec-permissions-policy.

The result of this discussion has informed the newest updates to the spec. The parsing algorithms in https://wicg.github.io/feature-policy/#algorithms encode this behaviour:

• If the header is invalid JSON, or does not contain objects, then the resulting declared policy is empty
• Ignore unparsable directives
• Defer to JSON for handling of duplicate keys
• Ignore unrecognized features
• Ignore features with unparsable allowlists
• Ignore unparsable origins

from webappsec-permissions-policy.