1304826 - Sees hexdecimal code as URL and asks me to go there, nothing happens.

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[email]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160728203720

Steps to reproduce:

Enter a hexdecimal (?) code in the address bar, for example:

0x1f00ffff

Then confirm you want to go there, when Firefox asks you to.


Actual results:

Nothing, keeps 'connecting'


Expected results:

Should not ask me at all to go there as it is no valid tld/url

Comment 1

[:Gijs (he/him)]

The reason for this behaviour is that technically hex is a valid way of describing an IP address (which is really just 4 ints, which you could represent as an 8-'digit' hex number). So 0x1f00ffff is like 31.0.255.255, and something like 0x7f000001 would be 127.0.0.1 ie localhost.

We detect this case on release for decimal numbers, but the check in the frontend code doesn't realize the hex number is also an IP.

This is slightly better on Nightly and Aurora (developer edition) (that is, it doesn't have this problem with the hex code in question) because it normalizes decimal and hexadecimal IP addresses in the network layer, and we detect the IP address based on this normalized URL.

However, there is still a case that doesn't 'work' on Nightly/Aurora, for longer hexes, e.g. if this was a 64-bit number and you typed in:

0x7f0000017f000001

that's technically a way of getting to localhost, which the DNS service tells us about, so we will prompt you. We shouldn't.

To fix this bug, we need to expand the regexp in this code:

https://dxr.mozilla.org/mozilla-central/rev/66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd/browser/base/content/browser.js#703-714

so it also works for hex numbers. We probably want something like:

/^(?:\d+|0x[a-f0-9]+)$/i

and then add a test like this one:

https://dxr.mozilla.org/mozilla-central/rev/66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd/browser/base/content/test/urlbar/browser_urlbarSearchSingleWordNotification.js#96-105

both for the small and large hex IP addresses, to ensure we test and don't re-break this behaviour.

Comment 2

[Saad]

First timer here, can I take this?

(In reply to :Gijs Kruitbosch from comment #1)
> The reason for this behaviour is that technically hex is a valid way of
> describing an IP address (which is really just 4 ints, which you could
> represent as an 8-'digit' hex number). So 0x1f00ffff is like 31.0.255.255,
> and something like 0x7f000001 would be 127.0.0.1 ie localhost.
> 
> We detect this case on release for decimal numbers, but the check in the
> frontend code doesn't realize the hex number is also an IP.
> 
> This is slightly better on Nightly and Aurora (developer edition) (that is,
> it doesn't have this problem with the hex code in question) because it
> normalizes decimal and hexadecimal IP addresses in the network layer, and we
> detect the IP address based on this normalized URL.
> 
> However, there is still a case that doesn't 'work' on Nightly/Aurora, for
> longer hexes, e.g. if this was a 64-bit number and you typed in:
> 
> 0x7f0000017f000001
> 
> that's technically a way of getting to localhost, which the DNS service
> tells us about, so we will prompt you. We shouldn't.
> 
> To fix this bug, we need to expand the regexp in this code:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd/browser/base/content/browser.js#703-
> 714
> 
> so it also works for hex numbers. We probably want something like:
> 
> /^(?:\d+|0x[a-f0-9]+)$/i
> 
> and then add a test like this one:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd/browser/base/content/test/urlbar/
> browser_urlbarSearchSingleWordNotification.js#96-105
> 
> both for the small and large hex IP addresses, to ensure we test and don't
> re-break this behaviour.

Comment 3

[:Gijs (he/him)]

(In reply to Saad from comment #2)
> First timer here, can I take this?

Sure, let me know if you need more help beyond comment #1 . Do you have source code and a build set up?

Comment 4

[Saad]

(In reply to :Gijs Kruitbosch from comment #3)
> Sure, let me know if you need more help beyond comment #1 . Do you have
> source code and a build set up?

I don't, but I'm following this to set up the build environment. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build

Comment 5

[:Gijs (he/him)]

(In reply to Saad from comment #4)
> (In reply to :Gijs Kruitbosch from comment #3)
> > Sure, let me know if you need more help beyond comment #1 . Do you have
> > source code and a build set up?
> 
> I don't, but I'm following this to set up the build environment.
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Simple_Firefox_build

Great, do let me know if you get stuck or anything. :-)

Comment 6

[Saad]

(In reply to :Gijs Kruitbosch from comment #5)
> Great, do let me know if you get stuck or anything. :-)

So, I ran into this error.

````
$ ./mach build
 0:03.99 c:\mozilla-build\mozmake\mozmake.EXE -f client.mk -s configure
 0:06.77 client.mk:202: c:/mozilla-source/mozilla-central/obj-i686-pc-mingw32/.mozconfig.mk: No such file or directory
 0:08.98 Clobber not needed.
 0:13.18 Generating c:/mozilla-source/mozilla-central/configure
 0:13.44 Generating c:/mozilla-source/mozilla-central/js/src/configure
 0:13.62 cd c:/mozilla-source/mozilla-central/obj-i686-pc-mingw32
 0:13.66 c:/mozilla-source/mozilla-central/configure
 0:14.49 Creating Python environment
 0:35.08 New python executable in c:\mozilla-source\mozilla-central\obj-i686-pc-mingw32\_virtualenv\Scripts\python2.7.exe
 0:35.08 Also creating executable in c:\mozilla-source\mozilla-central\obj-i686-pc-mingw32\_virtualenv\Scripts\python.exe
 0:35.08 Installing setuptools, pip, wheel...done.
 0:36.21 running build_ext
 0:36.21
 0:36.21 building 'psutil._psutil_windows' extension
 0:36.21
 0:36.21 error: Microsoft Visual C++ 9.0 is required. Get it from http://aka.ms/vcpython27
 0:36.22
 0:36.22
 0:36.22 Error processing command. Ignoring because optional. (optional:setup.py:python/psutil:build_ext:--inplace)
 0:36.22 c:\mozilla-source\mozilla-central\python\mozbuild\mozbuild\virtualenv.py:376: UserWarning: Hacking environment to allow binary Python extensions to build. You can make this warning go away by installing Visual Studio 2008. You can download the Express Edition installer from http://go.microsoft.com/?linkid=7729279
 0:36.22   warnings.warn('Hacking environment to allow binary Python '
 0:36.22 Reexecuting in the virtualenv
 0:37.23 checking for a shell... C:/mozilla-build/msys/bin/sh.exe
 0:38.12 checking for host system type... i686-pc-mingw32
 0:38.12 checking for target system type... i686-pc-mingw32
 0:38.12 checking whether cross compiling... no
 0:38.18 checking for pkg_config... not found
 0:38.19 checking for yasm... c:/mozilla-build/yasm/yasm.EXE
 0:38.37 checking yasm version... 1.3.0
 0:38.39 checking for the target C compiler... 'c:/PROGRA~2/MICROS~2.0/VC/BIN/amd64/cl.EXE'
 0:38.93 checking whether the target C compiler can be used... no
 0:38.93 DEBUG: <truncated - see config.log for full output>
 0:38.93 DEBUG: | #elif __linux__
 0:38.93 DEBUG: | %KERNEL "Linux"
 0:38.93 DEBUG: | #elif _WIN32 || __CYGWIN__
 0:38.93 DEBUG: | %KERNEL "WINNT"
 0:38.94 DEBUG: | #elif __NetBSD__
 0:38.94 DEBUG: | %KERNEL "NetBSD"
 0:38.94 DEBUG: | #elif __APPLE__
 0:38.94 DEBUG: | %KERNEL "Darwin"
 0:38.94 DEBUG: | #endif
 0:38.94 DEBUG: | #if _MSC_VER || __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
 0:38.94 DEBUG: | %ENDIANNESS "little"
 0:38.94 DEBUG: | #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
 0:38.94 DEBUG: | %ENDIANNESS "big"
 0:38.94 DEBUG: | #endif
 0:38.95 DEBUG: Executing: `cl.EXE -E 'c:\users\dumpty\appdata\local\temp\conftest.g8ir0u.c'`
 0:38.95 DEBUG: COMPILER = msvc
 0:38.95 DEBUG: VERSION = 190023026
 0:38.95 DEBUG: CPU = x86_64
 0:38.95 DEBUG: KERNEL = WINNT
 0:38.95 DEBUG: ENDIANNESS = little
 0:38.95 ERROR: Target C compiler target CPU (x86_64) does not match --target CPU (i686)
 0:38.99 *** Fix above errors and then restart with\
 0:39.00                "c:/mozilla-build/mozmake/mozmake.EXE -f client.mk build"
 0:39.01 client.mk:373: recipe for target 'configure' failed
 0:39.01 mozmake.EXE: *** [configure] Error 1
2
````

Comment 7

[Saad]

Never mind, people over at #introduction helped me fix it. :) I will let you know if I run into some other errors.

Comment 8

[Saad]

(In reply to :Gijs Kruitbosch from comment #5)
> Great, do let me know if you get stuck or anything. :-)

So, I made the changes and all the tests passed and now it's working as expected. 
But now I'm not sure about how to push these changes to mozilla-central.

Comment 9

[:Gijs (he/him)]

(In reply to Saad from comment #8)
> (In reply to :Gijs Kruitbosch from comment #5)
> > Great, do let me know if you get stuck or anything. :-)
> 
> So, I made the changes and all the tests passed and now it's working as
> expected. 
> But now I'm not sure about how to push these changes to mozilla-central.

The first step would be submitting a patch to this bug so it can be reviewed. The easiest way to do this would be to use mozreview. Commit your changes (using a commit message that contains the bug number, a description of your change, and ends with something like "r?gijs"), and then push them to the 'review' alias (which should have been set up as part of running ./mach mercurial-setup ). Then either confirm publishing the review request when hg prompts you, or open the URL in your browser and click the 'publish' button there. You might need to have logged in to mozreview for this to work.

If you're struggling to make that work, after having committed your work you can also use "hg export" to generate a patch file that you can manually attach to this bug, and set the review flags on.

#introduction might be able to help with this, or you can ping me either on here or on IRC. :-)

Comment 10

[Saad]

Attachment: Bug 1304826 - Expanded the regexp so that large hex numbers are not recognized as IP address.

Review commit: https://reviewboard.mozilla.org/r/83046/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83046/

Comment 11

[:Gijs (he/him)]

Comment on attachment 8797420 [details]
Bug 1304826 - Expanded the regexp so that large hex numbers are not recognized as IP address.

https://reviewboard.mozilla.org/r/83046/#review81686

Great! This is nearly perfect - can you make the change below, use "hg commit --amend" to update the commit, and push it to mozreview again? Then I can r+ it and we can get it landed. :-)

::: browser/base/content/test/urlbar/browser_urlbarSearchSingleWordNotification.js:111
(Diff revision 1)
> +
> +add_task(function* test_navigate_small_hex_number() {
> +  let tab = gBrowser.selectedTab = gBrowser.addTab("about:blank");
> +  yield BrowserTestUtils.browserLoaded(tab.linkedBrowser);
> +  yield* runURLBarSearchTest({
> +    valueToOpen: "0x1f00ffff",

Automated tests aren't allowed to talk to the outside world. To avoid that happening here in case someone starts running an actual thing at this IP address, let's just use 0x7f000001 for this value (which points to localhost).

Comment 12

[:Gijs (he/him)]

Comment on attachment 8797420 [details]
Bug 1304826 - Expanded the regexp so that large hex numbers are not recognized as IP address.

https://reviewboard.mozilla.org/r/83046/#review81690

Actually, I've changed my mind - right now no connection will be made to this IP address, and if this ever changes the test will break anyway, so it shouldn't matter. Let's just land this as-is.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6602778444ff
Expanded the regexp so that large hex numbers are not recognized as IP address. r=Gijs

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6602778444ff

Comment 15

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 48.0a1 (2016-04-01) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Nightly 

Build ID   : 20161018030211
User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0

[bugday-20161019]

Comment 16

[:Gijs (he/him)]

Thanks!