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 blocking algorithm (isMalicious) #5376

Open
wants to merge 9 commits into
base: feature/cris/malicious-site-protection/load-initial-dataset
Choose a base branch
from

Conversation

CrisBarreiro
Copy link
Contributor

@CrisBarreiro CrisBarreiro commented Dec 10, 2024

Copy link
Contributor Author

CrisBarreiro commented Dec 10, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 41dbcee to 45d1aaa Compare December 10, 2024 10:29
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch from 11f793b to 3f61183 Compare December 10, 2024 10:29
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 45d1aaa to ca1717c Compare December 10, 2024 10:50
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch 2 times, most recently from 79521f7 to 0974e8e Compare December 10, 2024 13:35
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from ca1717c to 212e3b7 Compare December 10, 2024 13:37
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch 2 times, most recently from 872e9ff to 9097bc2 Compare December 10, 2024 13:48
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 212e3b7 to 7c5e5c2 Compare December 10, 2024 15:27
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch from 9097bc2 to bf19279 Compare December 10, 2024 15:27
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 7c5e5c2 to 54f233d Compare December 11, 2024 14:03
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch from bf19279 to 8b35a0e Compare December 11, 2024 14:03
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 54f233d to 0f7697a Compare December 11, 2024 14:12
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch 2 times, most recently from e771493 to 46d5fc0 Compare December 11, 2024 15:00
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 0f7697a to 7c8c545 Compare December 16, 2024 10:47
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch from a1f08e1 to 3f18b3e Compare December 16, 2024 10:47
Comment on lines 110 to 143
fun isMalicious_invokesOnSiteBlockedAsync_whenUrlIsMaliciousAndNeedsToGoToNetwork() = runTest {
val url = Uri.parse("https://malicious.com")
val hostname = url.host!!
val hash = messageDigest.digest(hostname.toByteArray()).joinToString("") { "%02x".format(it) }
val hashPrefix = hash.substring(0, 8)
val filter = Filter(hash, ".*whatever.*")
var onSiteBlockedAsyncCalled = false

whenever(maliciousSiteRepository.containsHashPrefix(hashPrefix)).thenReturn(true)
whenever(maliciousSiteRepository.getFilter(hash)).thenReturn(filter)
whenever(maliciousSiteRepository.matches(hashPrefix))
.thenReturn(listOf(Match(hostname, url.toString(), ".*malicious.*", hash)))

realMaliciousSiteProtection.isMalicious(url) {
onSiteBlockedAsyncCalled = true
}

assertTrue(onSiteBlockedAsyncCalled)
}
Copy link

Choose a reason for hiding this comment

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

The test assertion needs to wait for the coroutine to complete. Add advanceUntilIdle() after calling isMalicious() to ensure the async callback has a chance to execute before checking onSiteBlockedAsyncCalled. Without this, the test may fail intermittently since it's racing against the coroutine.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch 2 times, most recently from d585fa1 to e310dad Compare December 17, 2024 09:21
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch 2 times, most recently from 3fec1c3 to 0aef382 Compare December 17, 2024 09:39
@CrisBarreiro CrisBarreiro changed the title Add blocking algorithm Add blocking algorithm (isMalicious) Dec 17, 2024
@CrisBarreiro CrisBarreiro marked this pull request as ready for review December 17, 2024 09:42
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 1ea8bd3 to 768449f Compare December 18, 2024 15:42
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch from 0aef382 to 0d79e69 Compare December 18, 2024 15:42
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/load-initial-dataset branch from 768449f to d6c864d Compare December 19, 2024 17:11
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch from cbc7bf5 to 0c8a08a Compare December 19, 2024 17:11
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/blocking-algorithm branch from ef6e36e to 75f7e5d Compare December 20, 2024 10:18
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