Add a way to specify that nsDefaultURIFixup should obey the domain whitelist when not using keyword searches

RESOLVED FIXED in Firefox 35

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Unfocused, Assigned: Unfocused)

Tracking

30 Branch
Firefox 35
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Bug 693808 added a domain whitelist for URI fixups. Unfortunately, it currently only applied when using keyword searches. For bug 951624 I want to use this while not allowing keyword searches (so it can reliably handle those separately).

So I'd like to introduce a new flag that specifies that the domain whitelist is required. This flag would be implied when FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP is specified.
This is part of the effort to split up bug 951624, which keeps growing in size.
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Points: --- → 5
Flags: qe-verify-
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Created attachment 8477202 [details] [diff] [review]
Patch v3

Split out from the latest patch in bug 951624, with review comments from bug 951624 comment 42 and bug 951624 comment 43 resolved.
Attachment #8477202 - Flags: review?(bugs)
Hi Blair, apologies for the delay.  Bug has been added to IT 34.3.  Thanks.
Iteration: --- → 34.3
Flags: needinfo?(mmucci)

Comment 4

4 years ago
Might not be trivial to merge this with bug 494092, but looking both anyway.

Updated

4 years ago
Iteration: 34.3 → 35.1

Comment 5

4 years ago
Comment on attachment 8477202 [details] [diff] [review]
Patch v3

>+bool nsDefaultURIFixup::IsDomainWhitelisted(const nsAutoCString aAsciiHost,
>+                                            const uint32_t aDotLoc) {
Nit, { goes to its own line

>     /**
>      * Allow the fixup to use a keyword lookup service to complete the URI.
>      * The fixup object implementer should honour this flag and only perform
>      * any lengthy keyword (or search) operation if it is set.
>+     *
>+     * Implies FIXUP_FLAG_REQUIRE_WHITELISTED_HOST.
>      */
>     const unsigned long FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP = 1;
It doesn't really imply that.
FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP doesn't _require_ a whitelisted host


>+     * For an input that may be just a domain with only 1 level (eg, "mozilla"),
>+     * require that the host be whitelisted.
>+     *
>+     * Implied by FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP.
>+     */
>+    const unsigned long FIXUP_FLAG_REQUIRE_WHITELISTED_HOST = 4;
Drop 'Implied by FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP' but add perhaps a comment that 
FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP overrides FIXUP_FLAG_REQUIRE_WHITELISTED_HOST.
Attachment #8477202 - Flags: review?(bugs) → review+
Depends on: 494092
Blocks: 494092
No longer depends on: 494092

Comment 7

4 years ago
Backed out in https://hg.mozilla.org/integration/fx-team/rev/ea6607178b55 for xpshell bustage
https://hg.mozilla.org/mozilla-central/rev/71ed0213343d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.