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

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

This is part of the effort to split up bug 951624, which keeps growing in size.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: 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.

Comment 3

[Marco Mucci [:MarcoM]]

Hi Blair, apologies for the delay.  Bug has been added to IT 34.3.  Thanks.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/fx-team/rev/7f58c77bb165

Comment 7

[Nigel Babu [:nigelb]]

Backed out in https://hg.mozilla.org/integration/fx-team/rev/ea6607178b55 for xpshell bustage

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/fx-team/rev/71ed0213343d

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/71ed0213343d