328707 - Manual proxy configuration does not reject URLs entered (as hostnames)

Status: RESOLVED FIXED

(Firefox :: Settings UI, enhancement)

Description

[mozilla]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; .NET CLR 2.0.50727; Avalon 6.0.5070; WinFX RunTime 3.0.50727)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

In trying to set up Firefox, I copied my proxy settings from Internet Explorer.  IE uses this format: http://192.168.1.1.  Firefox uses 192.168.0.1.  No validation is performed - Firefox accepts the erroneous input data and then fails to connect.  This isn't helped by there being no examples of manual proxy configuration in the Help documentation or on the website.  Come on guys, if you want to get people to switch from IE, either do things the same way or make it obvious you're doing it differently.

Reproducible: Always

Steps to Reproduce:
1. Open Options
2. Click on Connection Settings
3. Enter http://<your-proxy-ip-address
4. Try to access an Internet web site.

Actual Results:  
You get the Firefox Try Again page.

Expected Results:  
Displayed a page from the web site.

Comment 1

[Steve England [:stevee]]

Reporter, do you still see this problem with the latest Firefox 2? If not, can you please close this bug as WORKSFORME. Thanks!

Comment 2

[Marco Bonardo [:mak]]

this is still valid on branch and trunk, there is no check on text entered into proxy address configuration, and there is no example of what should be put there.

A check could be done to ensure that proxy address textbox content is at least a valid ip

Comment 3

[benc]

Which version of IE? Can you provide a screen snapshot?

Meanwhile, I'm trying to fix the online/help docs, so the usage is clearer.

Comment 4

[Manish Goregaokar [:manishearth]]

Reproe'd on FF 23.

Comment 5

[Jaydeep Das]

This shouldn't be hard to fix. A simple regex match for valid IP address/Hostname should do the job. Firefox already handles the scenario when the specified port is 0.

// mozilla-unified/browser/components/preferences/dialogs/connection.js
var gConnectionsDialog = {
  beforeAccept(event) {
    ...
    // If the proxy server(when specified) is invalid or the port is set to 0 then cancel submission.
    for (let prefName of ["http", "ssl", "socks"]) {
      let proxyPortPref = Preferences.get(
        "network.proxy." + prefName + "_port"
      );
      let proxyPref = Preferences.get("network.proxy." + prefName);
      // Only worry about ports which are currently active. If the share option is on, then ignore
      // all ports except the HTTP and SOCKS port
      if (
        proxyPref.value != "" &&
        proxyPortPref.value == 0 &&
        (prefName == "http" || prefName == "socks" || !shareProxiesPref.value)
      ) {
        document
          .getElementById("networkProxy" + prefName.toUpperCase() + "_Port")
          .focus();
        event.preventDefault();
        return;
      }
     // check if the specified Hostname/IP Address is valid.
      const validIPandHostNameRegex = /^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)+([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])$/;
      if (
        proxyPref.value != "" &&
        !validIPandHostNameRegex.test(proxyPref.value) &&
        (prefName == "http" || prefName == "socks" || !shareProxiesPref.value)
      ) {
        document
          .getElementById("networkProxy" + prefName.toUpperCase())
          .focus();
        event.preventDefault();
        return;
      }
    }

 ...
  },

Comment 6

[Jaydeep Das]

(In reply to Jaydeep Das from comment #5)

This shouldn't be hard to fix. A simple regex match for valid IP address/Hostname should do the job. Firefox already handles the scenario when the specified port is 0.

// mozilla-unified/browser/components/preferences/dialogs/connection.js
var gConnectionsDialog = {
  beforeAccept(event) {
    ...
    // If the proxy server(when specified) is invalid or the port is set to 0 then cancel submission.
    for (let prefName of ["http", "ssl", "socks"]) {
      let proxyPortPref = Preferences.get(
        "network.proxy." + prefName + "_port"
      );
      let proxyPref = Preferences.get("network.proxy." + prefName);
      // Only worry about ports which are currently active. If the share option is on, then ignore
      // all ports except the HTTP and SOCKS port
      if (
        proxyPref.value != "" &&
        proxyPortPref.value == 0 &&
        (prefName == "http" || prefName == "socks" || !shareProxiesPref.value)
      ) {
        document
          .getElementById("networkProxy" + prefName.toUpperCase() + "_Port")
          .focus();
        event.preventDefault();
        return;
      }
     // check if the specified Hostname/IP Address is valid.
      const validIPandHostNameRegex = /^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)+([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])$/;
      if (
        proxyPref.value != "" &&
        !validIPandHostNameRegex.test(proxyPref.value) &&
        (prefName == "http" || prefName == "socks" || !shareProxiesPref.value)
      ) {
        document
          .getElementById("networkProxy" + prefName.toUpperCase())
          .focus();
        event.preventDefault();
        return;
      }
    }

 ...
  },

Comment 7

[Jaydeep Das]

Request for Comments

Comment 8

[Jaydeep Das]

Attachment: Bug 328707 - Allow only valid IP/Hostname for Proxy Config, r=Gijs

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Thanks, Jaydeep! The patch looks good, and I've queued it for autolanding.

Great work! Are you interested in more work in about:preferences? Bug 1844935, for example, might be interesting?

Comment 10

[Jaydeep Das]

Thanks Mike. Yes I would be interested in working in more bug fixes. It was a great learning experience. I will take a look on that bug.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/790866f1dba5
Allow only valid IP/Hostname for Proxy Config, r=mconley,necko-reviewers,valentin

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/790866f1dba5