New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 737121 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Omnibox incorrectly parses a search string as a URL

Reported by michaels...@gmail.com, Jun 27 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce the problem:
1. Paste the string `npm ERR! peer dep missing: @angular/animations@4.2.4,` into the Omnibox.

What is the expected behavior?
A Google search for the error message.

What went wrong?
The Ominbox parser will rewrite the string to the URL `http://angular/animations@4.2.4,` and attempts direct navigation, rather than searching the original string.

Did this work before? N/A 

Chrome version: 59.0.3071.115  Channel: stable
OS Version: 10.0
Flash Version: 

Using the string `npm ERR! peer dep missing: angular/animations@4.2.4,` (without the @ before angular) triggers the expected behaviour.

The incorrect behaviour is also visible in the Omnibox text colouration; please see screenshots.

Present in Stable and Canary Version 61.0.3142.0 (Official Build) (64-bit)
 
black-omnibox.JPG
12.9 KB View Download
grey-omnibox.JPG
13.2 KB View Download
Labels: Needs-Triage-M59 Needs-Bisect

Comment 3 by hdodda@chromium.org, Jun 28 2017

Cc: hdodda@chromium.org
Labels: -Needs-Bisect M-61 OS-Linux OS-Mac
Status: Untriaged (was: Unconfirmed)
Tested the issue on windows 10 & 7 , Mac Os 10.12.5 and ubuntu 14.04 using chrome M59 #59.0.3071.115 and canary M61 #61.0.3142.3 and issue is reproduced.

Issue is seen from m30  #30.0.1549.0 and is a non-regression issue.

Marking it as untraiged for further inputs on this.

Thanks!
737121.mp4
685 KB View Download
Components: -UI UI>Browser>Omnibox
Labels: -Pri-2 Pri-3
Status: Available (was: Untriaged)
Wow, that's pretty crazy that the omnibox think that looks like a URL.  Thanks for reporting.

The key part seems to be the "@alphanumeric/".  It looks like almost any string in the form ...@alphanumeric/... gets interpreted as a URL by default.  Here's another example:

this ! I say` is a crazy @bug/ who'd have thought it? N0t m3.

That looks navigable to me; it's basically username@host/path.

My question would be, is there some obvious thing that makes this _not_ a username + host + path?
I would think punctuation is usernames is extremely rare.  I might even go so far as to say spaces are probably pretty rare, and don't need to supported strongly in the omnibox (make this UNKNOWN rather than URL).
So your proposal would be, look for non-alphanumeric characters in the username, and assume UNKNOWN in that case?

The would still mean

crazy@bug/ who'd have thought it? N0t m3.

...is parsed as "URL".  Maybe that's OK.
That's certainly a better heuristic than what we have.
For reference, here's the code fragment that needs to be revised to take into account username.  The code is currently too lenient.
  // Trailing slashes force the input to be treated as a URL.
  if (parts->path.is_nonempty()) {
    base::char16 c = text[parts->path.end() - 1];
    if ((c == '\\') || (c == '/'))
      return metrics::OmniboxInputType::URL;
  }
https://cs.chromium.org/chromium/src/components/omnibox/browser/autocomplete_input.cc?l=364-369
Owner: mpear...@chromium.org
Status: Assigned (was: Available)
Since I already figured out where the code goes, I'll fix this sometime.
 Issue 747972  has been merged into this issue.
Status: Started (was: Assigned)
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56f290842c6f2f23291baf3a1e05bc4a091a93a1

commit 56f290842c6f2f23291baf3a1e05bc4a091a93a1
Author: Mark Pearson <mpearson@chromium.org>
Date: Thu Sep 14 19:05:47 2017

Omnibox - Default to Search if user is unlikely to be using URL auth

Inputs such as
parse error line 75 @class/subclass.cc:25
can be generously interpreted as a URL using HTTP/HTTPS auth,
i.e., http://username:password@site/path
in this case with an omitted scheme, the username "parse error line 75 ",
and omitted password, a hostname "class" and a path "/subclass.cc:25".

When interpreted like this, the omnibox will identify the input type as URL
and make the default action a navigation to the (likely bad) URL

I believe most of the time an interpretation that involves pretending
the username has a space (and the user is trying to do http auth with this
username) is probably wrong unless there is strong evidence to the contrary.
I take strong evidence as the user explicitly typing http/https.

After this change, the default for inputs that contain a space will be of type
UNKNOWN.  The default action will be to search.

I decided not to go as far as to prohibit non-alphanumeric characters in the
username (which was one solution discussed on the bug).  I think it's likely
that users with other character sets may use non-Latin characters in their
username.  I'll stick with this narrower prohibition for now.

Tested interactively and with unit tests.

Bug:  737121 
Change-Id: I251d750b3aa43d81733b8f88cfc1674bfaa3f003
Reviewed-on: https://chromium-review.googlesource.com/660695
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502007}
[modify] https://crrev.com/56f290842c6f2f23291baf3a1e05bc4a091a93a1/components/omnibox/browser/autocomplete_input.cc
[modify] https://crrev.com/56f290842c6f2f23291baf3a1e05bc4a091a93a1/components/omnibox/browser/autocomplete_input_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment