Extract the Enabled() check in TRRService::IsDomainBlocked into TRR::MaybeBlockRequest
Categories
(Core :: Networking: DNS, defect, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox88 | --- | fixed |
People
(Reporter: nhnt11, Assigned: nhnt11)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-triaged])
Attachments
(1 file)
Enabled() is an API that essentially tells consumers whether or not to use TRR for a request, given the current confirmation state (prefs + computed state). It accepts the request's TRR mode as a parameter in order to do its logic.
Currently IsDomainBlocked() is calling Enabled() without any argument, as a way to return (true) early if we shouldn't use TRR anyway.
The fallout of this is that when the global TRR mode is 0, for requests that have TRR_FIRST_MODE explicitly set, the first call to Enabled along with the request's TRR mode returns true, but later in the flow when we check whether the domain is blocked, the second call to Enable() from IsDomainBlocked returns false, so IsDomainBlocked returns true and we end up falling back to Do53 for the request.
I would argue that the correct behavior here is that we should ensure that Enabled() is called at the original time of deciding whether to use TRR for a request, and IsDomainBlocked should assume that the caller is acting responsibly and has already gotten the green light from Enabled() by the time blocklisting checks are reached.
By this logic, we should just remove the early return in IsDomainBlocked.
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Comment 1•3 years ago
|
||
Currently, we are checking Enabled() without passing the request mode in IsDomainBlocked.
This isn't the right place to do this, it's cleaner if IsDomainBlocked trusts the caller
to have checked Enabled already. We should call Enabled, with the request mode, in
TRR::MaybeBlockDomain() and return early if it's false.
|
||
Updated•3 years ago
|
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/74f32f852769 Extract the Enabled() check in TRRService::IsDomainBlocked into TRR::MaybeBlockRequest. r=necko-reviewers,valentin
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Updated•3 years ago
|
|
||
Updated•3 years ago
|
|
||
Comment 3•3 years ago
|
||
| bugherder | ||
Description
•