Closed
Bug 1162821
Opened 9 years ago
Closed 9 years ago
"Did you mean to go to gmail" notification bar appears when a local "gmail" doesn't exist
Categories
(Core :: Networking: DNS, defect)
Core
Networking: DNS
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: Gavin, Assigned: mcmanus)
References
Details
Attachments
(2 files)
159.55 KB,
image/png
|
Details | |
5.08 KB,
patch
|
sworkman
:
review+
|
Details | Diff | Splinter Review |
I just launched today's Nightly with a new profile, and typed "gmail" into the location bar. I got this: https://cloudup.com/cApBwbGdz0W ...except that "gmail" does not resolve on my network, and so clicking "Yes, go to gmail" just spins forever until the hostname lookup times out. Shouldn't that bar not have appeared? (I can't seem to reproduce this by typing other simple words... Possible something is wonky with my dns, I guess? :/ )
Comment 1•9 years ago
|
||
From IRC, the name "gmail" is resolving to 127.0.53.53, which is the "name collision" address (https://www.icann.org/news/announcement-2-2014-08-01-en), being returned as "gmail" is a generic TLD. This can be reproduced for me by executing the following in the browser console: > Cc["@mozilla.org/network/dns-service;1"].getService(Ci.nsIDNSService).asyncResolve("gmail.", 0, (request, record, status) => {console.log("got", record.getNextAddrAsString(), status);}, Services.tm.mainThread); Which results in: > "got" "127.0.53.53" 0 Not clear if the dns service is doing the right thing by not throwing in that case...
Reporter | ||
Comment 2•9 years ago
|
||
Yeah, it sounds like this is specific to "gmail", and it's because gmail is more magical now (http://www.iana.org/domains/root/db/gmail.html). Probably we should have special treatment for 127.0.53.53 somewhere - not sure if that should be in the front-end or in the nsIDNSService back-end.
Summary: "Did you mean to go to <hostname>" notification bar appears when <hostname> doesn't exist → "Did you mean to go to gmail" notification bar appears when a local "gmail" doesn't exist
Comment 3•9 years ago
|
||
I get the same with "channel." too, so I think it's true for any of the new generic TLDs
Comment 4•9 years ago
|
||
Steve/Patrick, would it be wrong to change the DNS code to throw an error (return non-NS_OK) here instead of just returning this address?
Flags: needinfo?(sworkman)
Flags: needinfo?(mcmanus)
Assignee | ||
Comment 5•9 years ago
|
||
heh. 1] Its kind of orthogonal, but should the frontend uri fixup be offering a fqdn (www.gmail.com) or even better a full origin? (https://www.gmail.com) Offering just a single token is generally going to be ambiguous. 2] as for name collision records, I think its ok to deal with them inside DNS. we should just add a flag to asyncResolve() to allow them on a per query basis. I can write this.
Flags: needinfo?(mcmanus)
Comment 6•9 years ago
|
||
(In reply to Patrick McManus [:mcmanus] from comment #5) > heh. > > 1] Its kind of orthogonal, but should the frontend uri fixup be offering a > fqdn (www.gmail.com) or even better a full origin? (https://www.gmail.com) > Offering just a single token is generally going to be ambiguous. The point of the notification bar is if you type "word" into the url bar and the DNS service returns something for "word". This works well for local aliases / hostnames, so if you're a webdev who aliased "pear" to point to localhost, or in enterprise and you have something like "secretdocs" as a host, you can type that and while we will load a search page first, but do an async DNS query simultaneously, and if that returns something, we'll show the bar. If you click the button on the bar, we'll save the fact that it's a magical single-word host thing and never do a web search again for that input. So what Gavin is seeing is a gmail/yahoo/whatever search page with a search for "gmail", but because DNS returns something we offer "did you really mean http://gmail/", thinking it's a real thing. The name collision record messes with that logic, hence the confusion. Does that help make sense of the behaviour? Maybe we should include the "http://" and trailing slash...
Comment 7•9 years ago
|
||
Pat's got this, and I agree: this could be handled inside DNS.
Flags: needinfo?(sworkman)
Assignee | ||
Comment 8•9 years ago
|
||
interestingly I never knew quite how this bit of front end worked because I have one of those obnoxious ISP DNS resolvers that will return its own search page for anything that really ought to be NXDOMAIN :)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8603465 -
Flags: review?(sworkman)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e541e910c27f
Comment 11•9 years ago
|
||
Comment on attachment 8603465 [details] [diff] [review] filter dns name collision records Review of attachment 8603465 [details] [diff] [review]: ----------------------------------------------------------------- LGTM r=me. ::: netwerk/dns/DNS.cpp @@ +269,5 @@ > PRNetAddr tmpAddr; > void *iter = nullptr; > do { > iter = PR_EnumerateAddrInfo(iter, prAddrInfo, 0, &tmpAddr); > + bool addIt = iter && (really minor annoying) nit: addIt made me think of iter - s/addIt/okToAdd/ ::: netwerk/dns/nsDNSService2.cpp @@ +1,1 @@ > + Extra line. ::: netwerk/dns/nsIDNSService.idl @@ +164,5 @@ > */ > const unsigned long RESOLVE_DISABLE_IPV4 = (1 << 7); > + > + /** > + * If set, allow name collision results (127.0.53.53) which are normally filtered. "... allow IPv4 name collision ..." ?? - do we need to be explicit that IPv6 isn't included?
Attachment #8603465 -
Flags: review?(sworkman) → review+
Assignee | ||
Updated•9 years ago
|
Component: Location Bar → Networking: DNS
Product: Firefox → Core
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6b1185fcb09
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b6b1185fcb09
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•