(linux Firefox aviary cvs 20040926) While the "browse by first search match" feature is useful, it doesn't make sense to search for strings that obviously are meant as URLs, such as http://that-box-under-your-desk/pr0n/ or https://that-box-under-your-desk/pr0n/ -- curiously enough, ftp://that-box-under-your-desk/pr0n/ works as expected. Not doing the I'm Feeling Lucky search for non-resolving urls, where the user has properly specified the protocol, would fix many of the issues marked as dup of bug 231720, without introducing typo-guessing heuristics. To reproduce: Type http://that-box-under-your-desk/pr0n/ or http://that-box-under-your-desk/pr0n/ into location bar and hit enter (assuming that-box-under-your-desk doesn't resolve). For comparison: Type ftp://that-box-under-your-desk/pr0n/ into location bar and hit enter (assuming that-box-under-your-desk doesn't resolve). Actual results: Loads first google search result for that-box-under-your-desk Expected results: Same results for http(s) as for ftp: Alert: "that-box-under-your-desk could not be found. Please check the name and try again."

...and in case the code is all the same as seamonkey IKs, this would be more or less bug 95390.

Or dupe to bug 231720.

This is expressly not specifically about malformed urls. This is specifically about recognizing that nobody types http://this-is-a-search-term or https://this-is-a-search-term meaning "this-is-a-search-term" as a search term. ( And if they did intend a search, you shouldn't be stripping the "http://" or "https://" from the search terms anyhow :P )

(In reply to comment #0) Also reported as bug 272112.

*** Bug 272112 has been marked as a duplicate of this bug. ***

If any domain is mistyped with https the redirect goes to Paypal. This breaches the security model of HTTPS; if a secure HTTPS url is indicated and certificates are being expected to be checked, then the browser should not make any adjustments arbitrarily to the URL typed in to URL bar, and/or make changes seen obvious. As it was discovered by payments people (Gordon Katz of KatzGlobal.com), and as everyone in that world is panicing about phishing, I think this could be major. It currently it appears mostly embarrassing rather than exploitable. I can't quite see how to exploit it but phishers are more persistent than I. (Comments copied from #289793)

*** Bug 245330 has been marked as a duplicate of this bug. ***

*** Bug 302974 has been marked as a duplicate of this bug. ***

*** Bug 231720 has been marked as a duplicate of this bug. ***

*** Bug 235786 has been marked as a duplicate of this bug. ***

-> All/All

I blame http://lxr.mozilla.org/seamonkey/source/docshell/base/nsWebShell.cpp#701 That seems to be the "oops, host doesn't resolve, let's screw with the user's input" code. It also appears to do it only for http/https schemes! The easy fix is to just change it to disable keywords if there is a scheme at all, but I'm not sure why it allows http://word in the first place...

it seems like people are panicking too much but it works by simply clicking on a url which a company could have mistyped, however I don't understand how paypal gets this; isn't google the only one who could ever get the information? When the "i'm feeling lucky" search occurs, don't any security parameters in the URL get wiped after google redirects it? the only thing that could be bad is already fixed - localhost really goes to 127.0.0.1 as it should. then again "google" should always go to google and not have to pass through google's "i'm feeling lucky" search just to save milliseconds.

*** Bug 317405 has been marked as a duplicate of this bug. ***

*** Bug 319517 has been marked as a duplicate of this bug. ***

(In reply to comment #12) > I'm not sure why it allows http://word in the first place... Because it's quite common to have a default search domain specified in the resolver (i.e. /etc/resolv.conf on Linux), enabling you to omit the domain name for local hosts. Removing this ability would reduce usability drastically.

Oh FFS. Will you PLEASE READ the damn bug! We're talking about the auto-searching code, and it allows "http://foo" to trigger an automatic search for "foo" if the host does not resolve.

I've also seen this be a problem on web forums when someone posts an obviously invalid URL as a joke (like http://rofl/) but due to this "feature" the link works and goes to whichever website is #1 on Google at the moment, causing confusion all around.

Justin, that's bug 310826, although a fix for this bug might fix that too.

This removes the explicit exception for the http and https schemes for doing keyword searches when the host lookup fails. Please note that this exception was put in deliberately, due to the way other stuff happened. See <https://bugzilla.mozilla.org/show_bug.cgi?id=143080#c17>. I could not reproduce the mentioned behaviour, but it could make this really hard to fix.

Fwiw, that patch just disables keywords completely. That's fine by me (not that I'm the one to make that decision), but if that's what we're doing we should remove the code, not just leave dead code in the tree.

Hm, odd. I couldn't test is because stuff was failing to compile elsewhere.

*** Bug 330962 has been marked as a duplicate of this bug. ***

The patch doesn't actually disable it here, it just stops it doing it for single words and other host-like things. Anyway, this is being screwed about with in an excessively invasive way over in bug 331522, so I'm not going to try and do anything here until that's done.

*** Bug 332962 has been marked as a duplicate of this bug. ***

So where do we actually create a URI from this string? Is that happening in docshell, or out in the chrome?

(In reply to comment #26) > So where do we actually create a URI from this string? Is that happening in > docshell, or out in the chrome? in docshell, from what I can see: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/docshell/base/nsDocShell.cpp&rev=1.785&mark=2799,2806#2778 handleURLBarCommand calls BrowserLoadURL which calls loadURI, in the standard case.

OK. So would it make sense to disable keyword lookup at that point? That is, if the "allow keyword" flag is set, before calling into fixup try to ExtractScheme or whatever on the string. If that succeeds, toggle off the flag. roc, darin, biesi, what do you think?

sounds like a good idea.

I agree.

You could simply disable keyword search if the string can be parsed into a nsIURI.

Would it make sense to modify the URIFixup API to indicate that information via an out parameter, perhaps?

Comment on attachment 217936 [details] [diff] [review] Like so-ish? is there a point in using URI Fixup when we already have a valid URI?

> is there a point in using URI Fixup when we already have a valid URI? Yes. For example, consider what URI fixup does with: view-source:google.com I could add a comment to that effect if desired.

Comment on attachment 217936 [details] [diff] [review] Like so-ish? >Index: docshell/base/nsDocShell.cpp >+ nsAutoString uriString(aURI); >+ // Cleanup the empty spaces that might be on each end. >+ uriString.Trim(" "); >+ // Eliminate embedded newlines, which single-line text fields now allow: >+ uriString.StripChars("\r\n"); >+ NS_ENSURE_TRUE(!uriString.IsEmpty(), NS_ERROR_FAILURE); >+ >+ rv = NS_NewURI(getter_AddRefs(uri), uriString); >+ if (uri) { >+ aLoadFlags = aLoadFlags & ~LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP; >+ } >+ >+ if (sURIFixup) { >+ // Call the fixup object. This will clobber the rv from >+ // NS_NewURI above, but that's fine with us. > PRUint32 fixupFlags = 0; > if (aLoadFlags & LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP) { > fixupFlags |= nsIURIFixup::FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP; > } > rv = sURIFixup->CreateFixupURI(NS_ConvertUTF16toUTF8(aURI), fixupFlags, > getter_AddRefs(uri)); Notice that this is doing a conversion from UTF16 to UTF8 twice for the URI string. NS_NewURI takes a nsAString, so you might as well convert up-front, and save some effort. Is there any reason not to pass the trim'd and strip'd URI string to CreateFixupURI?

Will do the UTF8 thing up front (and pass the trimmed version to the fixup service). Do you want an updated patch with those changes?

Comment on attachment 217936 [details] [diff] [review] Like so-ish? no need :)

Comment on attachment 217936 [details] [diff] [review] Like so-ish? hm, ok... please do add the comment. it sort of uglifies this code because it converts string->uri twice, oh well

Comment on attachment 217936 [details] [diff] [review] Like so-ish? damn product changes losing review flags!

Fixed on trunk.

Comment on attachment 218097 [details] [diff] [review] Updated to review comments a=darin for 1.8.1

I'll land this on branch once bug 331522 lands there.

Hmm, now the I'm Feeling Lucky search doesn't happen, but it sends you to http://www.foobar.com/ if you type in http://foobar/ - is that really desirable? Seems like if you give a full URL you don't want it trying random similar hostnames - this could be a security issue if you go to http://localnetwork/script?importantpassword without thinking when you're away from the network, so it sends that query to localnetwork.com... (which it will, assuming localnetwork.com exists)

That looks like a separate bug to me. "Do not autocomplete URLs when GET parameters are used". Or something.

Yeah, that's a totally separate issue. I'm not touching that one with a 10-foot pole, personally. ;)

*** Bug 337836 has been marked as a duplicate of this bug. ***

Please land this patch on the MOZILLA_1_8_BRANCH. Thanks!

Fixed on 1.8 branch.

*** Bug 344826 has been marked as a duplicate of this bug. ***

*** Bug 356307 has been marked as a duplicate of this bug. ***

verified w/ ff4.0b1 :)