Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect use of options variable: rename to pkOptions #1805

Merged
merged 1 commit into from Oct 5, 2022

Conversation

emlun
Copy link
Member

@emlun emlun commented Sep 22, 2022

Fixes #1752. This is mutually exclusive with PR #1806. I couldn't decide which approach is better, but I'm slightly favouring this one over #1806.

This fixes the issue by introducing a new variable pkOptions instead of re-assigning the existing variable options, so that options can keep its original value.


Preview | Diff

_§5.1.3. Create a New Credential_ and _§5.1.4. Use an Existing Credential to
Make an Assertion_ both declare their **options** parameter as the
`Credential[Creation|Request]Options` object inherited from CredMan:

>**options**
>This argument is a `CredentialCreationOptions` object whose
>_options_.`publicKey` member contains a `PublicKeyCredentialCreationOptions`
>object [...]

Both also re-assign the _options_ variable:

>Let _options_ be the value of _options_.`publicKey`.

But both then also reference _options_.`signal`, which is a member of
`Credential[Creation|Request]Options` but not
`PublicKeyCredential[Creation|Request]Options`:

>If _options_.`signal` is present and aborted, throw the _options_.`signal`’s
abort reason.

_§5.1.4. Use an Existing Credential to Make an Assertion_ also incorrectly
references _options_.`mediation` in a similar way.

This fixes the issue by introducing a new variable _pkOptions_ instead of
re-assigning the existing variable _options_, so that _options_ can keep its
original value.
@emlun emlun added this to the L3-WD-01 milestone Sep 22, 2022
@emlun emlun self-assigned this Sep 22, 2022
@emlun emlun changed the title Fix incorrect use of options variables in create() and get() Fix incorrect use of options variable: rename to pkOptions Sep 22, 2022
@emlun emlun added the stat:puntable Issue or PR that is candidate to move to a later milestone label Oct 3, 2022
Copy link
Contributor

@agl agl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this one better too.

@emlun emlun merged commit e0d10dd into main Oct 5, 2022
@emlun emlun deleted the issue-1752-incorrect-options-variable branch October 5, 2022 19:13
github-actions bot added a commit that referenced this pull request Oct 5, 2022
SHA: e0d10dd
Reason: push, by @emlun

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat:puntable Issue or PR that is candidate to move to a later milestone type:editorial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect use of options variable in create() and get() definitions
2 participants