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.

Sign up for GitHub

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
Merged

Add steps for parsing the blocklist and testing blocklistedness #239

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7f28ba6
Add steps for parsing the blocklist and testing blocklistedness
nondebug Aug 4, 2023
00ee632
Remove Exposed=() and change invocation style
nondebug Aug 4, 2023
528592a
simplify blocklist parsing steps
nondebug Oct 10, 2023
39c50be
Replace ECMAscript String with infra string algorithms
nondebug Oct 10, 2023
0c7ace0
Replace Number.parseInt
nondebug Oct 10, 2023
a256a3a
set bcdDevice default value
nondebug Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 15 additions & 21 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,33 +2320,27 @@ The result of <dfn>parsing the blocklist</dfn> at a {{URL}} |url| is a [=list=]
of {{USBBlocklistEntry}} objects produced by the following algorithm:

1. Fetch |url| and let |contents| be its body, decoded as UTF-8.
1. Let |lines| be the result of invoking
{{String/split(separator, limit)}} on |contents| with separator
<code>'\n'</code>.
1. Let |lines| be the result of [=strictly splitting=] |contents| starting
from the beginning of |contents| on code point <code>'\n'</code>.
1. Let |blocklist| be an empty [=list=].
1. [=list/For each=] |line| of |lines|:
1. Let |commentBegin| be the result of invoking
|line|.{{String/indexOf()}} with <code>'#'</code>.
1. If |commentBegin| is not -1, set |line| to the result of invoking
|line|.{{String/substring()}} with 0 and |commentBegin|.
1. Set |line| to the result of |line|.{{String/trimEnd()}}.
1. Let |components| be the result of invoking
|line|.{{String/split(separator, limit)}} with separator
<code>':'</code>.
1. Set |line| to the result of [=collecting a sequence of code points=]
not equal to <code>'#'</code> from |line| starting from the beginning
of |line|.
1. Set |line| to the result of [=stripping leading and trailing ASCII
whitespace=] from |line|.
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 invoking
{{Number}}.{{Number/parseInt(string, radix)}} with |components|[0]
and 16, or [=iteration/continue=] if a {{TypeError}} is thrown.
1. Let |idProduct| be the result of invoking
{{Number}}.{{Number/parseInt(string, radix)}} with |components|[1]
and 16, or [=iteration/continue=] if a {{TypeError}} is thrown.
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:

* 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.

* 3: Let |bcdDevice| be the result of invoking
{{Number}}.{{Number/parseInt(string, radix)}} with
|components|[2] and 16, or [=iteration/continue=] if a
{{TypeError}} is thrown.
* 3: Let |bcdDevice| be the result of interpreting |components|[2] as
a hexadecimal number.
1. [=list/Append=] a new {{USBBlocklistEntry}} with |idVendor|,
|idProduct|, and |bcdDevice| to |blocklist|.
1. Return |blocklist|.
Expand Down
Loading