Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #6146: Add eTLD+1 check when debouncing
Browse files Browse the repository at this point in the history
  • Loading branch information
cuba committed Oct 11, 2022
1 parent 31cc0bb commit e8cf253
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
3 changes: 2 additions & 1 deletion Client/WebFilters/DebouncingResourceDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ public class DebouncingResourceDownloader {
}

guard let extractedURL = try rule.extractRedirectURL(from: url),
url.host != extractedURL.host,
url.origin != extractedURL.origin,
url.baseDomain != extractedURL.baseDomain,
extractedURL.scheme == "http" || extractedURL.scheme == "https" else {
return nil
}
Expand Down
9 changes: 9 additions & 0 deletions Tests/ClientTests/Resources/debouncing.json
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,15 @@
"action": "regex-path",
"param": "^/(.*)/(.*)$"
},
{
"include": [
"http://a.click.example/bounce?url=*"
],
"exclude": [
],
"action": "redirect",
"param": "url"
},
{
"include": [
"*://brave-long-regex.example/*"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ class DebouncingResourceDownloaderTests: XCTestCase {
let redirectChain = downloader.redirectChain(for: startURL)

// Then
XCTAssertEqual(redirectChain.last?.url, finalURL)
// We stop at the intermediate URL because of same eTLD+1 restriction
XCTAssertEqual(redirectChain.last?.url, intermediateURL)
}

// Test a long redirect chain that bounces through the original URL's domain,
Expand Down Expand Up @@ -374,6 +375,36 @@ class DebouncingResourceDownloaderTests: XCTestCase {
)
}

/// Test several restrictions to debouncing
func testDebounceRestrictions() {
// Given
// A constructed url
let baseURL = URL(string: "https://a.click.example/bounce")!
let startURL1 = baseURL.addRedirectParam(URL(string: "https://a.click.example/dest")!)
let startURL2 = baseURL.addRedirectParam(URL(string: "https://b.click.example/dest")!)
let startURL3 = baseURL.addRedirectParam(URL(string: "ftp://b.click.example/dest")!)

// When
// A redirect is returned
let redirectChain1 = downloader.redirectChain(for: startURL1)
let redirectChain2 = downloader.redirectChain(for: startURL2)
let redirectChain3 = downloader.redirectChain(for: startURL3)

// Then
XCTAssertNil(
redirectChain1.last,
"This should be nil because origin matches"
)
XCTAssertNil(
redirectChain2.last,
"This should be nil because eTLD+1 matches"
)
XCTAssertNil(
redirectChain3.last,
"This should be nil because the schema is not http or https"
)
}

/// Test regex rule where `prepend_scheme` is not specified and `base64` action is specified
func testRegexRuleWithBase64DecodedURL() {
// Given
Expand Down

0 comments on commit e8cf253

Please sign in to comment.