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

Add steps for parsing the blocklist and testing blocklistedness #239

Merged
merged 6 commits into from
Oct 12, 2023

Conversation

nondebug
Copy link
Collaborator

@nondebug nondebug commented Aug 4, 2023

@nondebug nondebug requested a review from reillyeon August 4, 2023 22:59
index.bs Outdated Show resolved Hide resolved
Copy link
Collaborator

@reillyeon reillyeon left a comment

Choose a reason for hiding this comment

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

Some more hints on how to not depend on ECMAscript to parse the blocklist.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated
1. Let |idVendor| be the result of interpreting |components|[0] as a
hexadecimal number.
1. Let |idProduct| be the result of interpreting |components|[1] as a
hexadecimal number.
1. If the [=list/size=] of |components| is:
* 2: Let |bcdDevice| be <code>0xFFFF</code>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line is no longer necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is still needed, otherwise bcdDevice will be uninitialized when we create the USBBlocklistEntry.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, then this needs to be swapped with the "if" above so that it is set to this default when components only has 2 elements.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't understand the issue. If we move the if lower then |components|[1] will be an OOB access if the line doesn't have any ':' code points.

How about:

      1. Let |components| be the result of [=strictly splitting=] |line|
         starting from the beginning of |line| on code point <code>':'</code>.
      1. If the [=list/size=] of |components| is not 2 or 3,
         [=iteration/continue=].
      1. Let |idVendor| be the result of interpreting |components|[0] as a
         hexadecimal number.
      1. Let |idProduct| be the result of interpreting |components|[1] as a
         hexadecimal number.
      1. Let |bcdDevice| be <code>0xFFFF</code>.
      1. If the [=list/size=] of |components| is 3, set |bcdDevice| to the
         result of interpreting |components|[2] as a hexadecimal number.
      1. [=list/Append=] a new {{USBBlocklistEntry}} with |idVendor|,
         |idProduct|, and |bcdDevice| to |blocklist|.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That LGTM.

index.bs Outdated
1. Let |idVendor| be the result of interpreting |components|[0] as a
hexadecimal number.
1. Let |idProduct| be the result of interpreting |components|[1] as a
hexadecimal number.
1. If the [=list/size=] of |components| is:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. If the [=list/size=] of |components| is:
1. If the [=list/size=] of |components| is 3:

@nondebug nondebug merged commit 59614e7 into main Oct 12, 2023
2 checks passed
github-actions bot added a commit that referenced this pull request Oct 12, 2023
SHA: 59614e7
Reason: push, by nondebug

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Oct 12, 2023
SHA: 59614e7
Reason: push, by nondebug

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants