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

Jump to bottom

feat(mv3): ➕ Adding hash function to generate predict… #1273

Merged
merged 1 commit into from
Sep 15, 2023
Merged

feat(mv3): ➕ Adding hash function to generate predict… #1273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions add-on/src/lib/redirect-handler/blockOrObserve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import debug from 'debug'
import { fastHashCode } from 'fast-hash-code'
import browser from 'webextension-polyfill'
import { CompanionState } from '../../types/companion.js'
import { IFilter, IRegexFilter, RegexFilter } from './baseRegexFilter.js'
Expand Down Expand Up @@ -281,8 +282,10 @@ function saveAndGenerateRule (
regexSubstitution: string,
excludedInitiatorDomains: string[] = []
): browser.DeclarativeNetRequest.Rule {
// We need to generate a random ID for the rule.
const id = Math.floor(Math.random() * 29999)
// We need to generate a positive number as an id.
const id = fastHashCode(`${regexFilter}:${regexSubstitution}:${excludedInitiatorDomains.join(':')}`, {
Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable, but the link provided by fast-hash-code to the implementation reasoning is no longer available. It makes me wonder about collisions. webCrypto.subtle.digest should do what we want and is available in service workers, is there a reason we didn't use that?

does id need to be a number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

id needs to be a number, that too >=1. https://developer.chrome.com/docs/extensions/reference/declarativeNetRequest/#type-Rule

I can do something like:

function id (text = 'hello world') {
  // new Int8Array(await crypto.subtle.digest('SHA-1', new TextEncoder().encode('hello world'))).reduce((a, v) => a + v, 0)
  
  const digest = await crypto.subtle.digest('SHA-1', new TextEncoder().encode(text)
  const digestArr = new Int8Array(digest)
  const sum = digestArr.reduce((a, v) => a + v, 0)
  return Math.abs(sum) // always positive.
}

will that be better? I still have no clue about collisions. The collisions earlier were in the space of 30k values, I think the package does a homogeneous distribution.

forcePositive: true
})
// We need to save the regex filter and ID to check if the rule already exists later.
savedRegexFilters.set(regexFilter, { id, regexSubstitution })
return generateAddRule(id, regexFilter, regexSubstitution, excludedInitiatorDomains)
Expand Down
13 changes: 13 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
"css-loader": "6.7.2",
"download-cli": "1.1.1",
"exponential-backoff": "^3.1.0",
"fast-hash-code": "^2.1.0",
"file-loader": "6.2.0",
"fs-promise": "2.0.3",
"get-firefox": "5.0.0",
Expand Down