search

w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials
https://w3c.github.io/webauthn/
Other
1.19k stars 172 forks source link

Replace `USVString` with `DOMString` #2203

Open zacknewman opened 1 week ago

zacknewman commented 1 week ago

It would appear that the same justification that was used for #2115 applies to the remaining areas where USVString is used:

As recommended by the Web IDL spec:

Specifications should only use USVString for APIs that perform text processing and need a string of scalar values to operate on. Most APIs that use strings should instead be using DOMString, which does not make any interpretations of the code units in the string. When in doubt, use DOMString.

Currently the only places where USVString is used are the following extensions:

Should these all be changed to DOMString?

agl commented 1 day ago

These extensions do, however, process their inputs.

The prf extension needs to perform base64url decoding of the values, so we need a valid input string. The AppId extensions need to convert the input to UTF-8 and hash it. In both cases, the input needs to be valid strings and invalid UTF-16 would cause an error.

So perhaps these are correctly USVString?

zacknewman commented 1 day ago

That argument applies to many of the DOMStrings in the spec (i.e., many areas where you see DOMString have to be valid Unicode). Making these lone 3 USVStrings into DOMStrings is a much smaller change, makes the spec consistent, and does not make it more fragile since there are more requirements than just valid Unicode. base64url decoding is defined on arbitrary bytes. If the input is not valid UTF-8, then the decoding will either fail or decode into something invalid which is possible even when the input is valid UTF-8.

The prf extension is also inconsistent with how the base64URL encoding of a Credential ID is typed elsewhere. For example, RegistrationResponseJSON.id is a DOMString.

It would be nice to pick one approach and stick with it:

The first approach will cause a lot of DOMStrings to be changed to USVString—including some of which revert previous changes from USVString to DOMString—and I don't think it appreciably makes the spec that much more type safe since many things require additional properties anyway (i.e.,USVString won't make it infallible anyway).

The second approach is much easier, aligns with the recommendation, and aligns with previous PRs that changed USVStrings to DOMStrings.