Fix: Invalid proxy settings should be handled and not crash (#11)

Author: timotheeguerin

.travis.yml

@@ -3,10 +3,6 @@ node_js:
 - 9
 - 8
 
-branches:
-  only:
-  - master
-
 install:
 - npm install
 script:

package.json

@@ -1,6 +1,6 @@
 {
   "name": "get-proxy-settings",
-  "version": "0.1.8",
+  "version": "0.1.9",
   "description": "Retrieve proxy settings specified by the system",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",

src/proxy.spec.ts

@@ -0,0 +1,50 @@
+import { expect } from "chai";
+import { parseWindowsProxySetting } from "./proxy";
+describe("proxy", () => {
+    describe("parseWindowsProxySetting", () => {
+        it("should return null if null or undefined", () => {
+            expect(parseWindowsProxySetting(null)).to.be.null;
+            expect(parseWindowsProxySetting(undefined)).to.be.null;
+        });
+
+        it("should return same url for http and https if just a url is provided", () => {
+            const value = parseWindowsProxySetting("http://localhost:8888");
+            expect(value).not.to.be.null;
+            expect(value.http.host).to.eql("localhost");
+            expect(value.http.port).to.eql("8888");
+            expect(value.https.host).to.eql("localhost");
+            expect(value.https.port).to.eql("8888");
+        });
+
+        it("should coresponding http and https urls", () => {
+            const value = parseWindowsProxySetting("http=http://localhost:8888;https=http://localhost:9999");
+            expect(value).not.to.be.null;
+            expect(value.http.host).to.eql("localhost");
+            expect(value.http.port).to.eql("8888");
+            expect(value.https.host).to.eql("localhost");
+            expect(value.https.port).to.eql("9999");
+        });
+
+        it("should assign https automatically if missing but http provided", () => {
+            const value = parseWindowsProxySetting("http=http://localhost:8888");
+            expect(value).not.to.be.null;
+            expect(value.http.host).to.eql("localhost");
+            expect(value.http.port).to.eql("8888");
+            expect(value.https.host).to.eql("localhost");
+            expect(value.https.port).to.eql("8888");
+        });
+
+        it("should assign http automatically if missing but https provided", () => {
+            const value = parseWindowsProxySetting("https=http://localhost:8888");
+            expect(value).not.to.be.null;
+            expect(value.http.host).to.eql("localhost");
+            expect(value.http.port).to.eql("8888");
+            expect(value.https.host).to.eql("localhost");
+            expect(value.https.port).to.eql("8888");
+        });
+
+        it("should return null if invalid url is provided", () => {
+            expect(parseWindowsProxySetting("invalid-url")).to.be.null;
+        });
+    });
+});

src/proxy.ts

@@ -1,9 +1,9 @@
 import * as npmConfLoader from "npm-conf";
+import * as url from "url";
 import { ProxyAuthenticationRequiredError } from "./proxy-errors";
 import { ProxyCredentials, ProxySetting, ProxySettings } from "./proxy-settings";
 import { validateProxySetting } from "./validate";
 import { Hive, openKey } from "./winreg";
-
 const npmConf = npmConfLoader();
 
 export async function getProxySettings(): Promise<ProxySettings> {
@@ -71,11 +71,40 @@ export function getEnvProxy(): ProxySettings {
     return { http: new ProxySetting(httpProxy), https: new ProxySetting(httpsProxy) };
 }
 
-function parseWindowsProxySetting(proxySetting: string): ProxySettings {
+export function parseWindowsProxySetting(proxySetting: string): ProxySettings {
+    if (!proxySetting) { return null; }
+    if (isValidUrl(proxySetting)) {
+        const setting = new ProxySetting(proxySetting);
+        return {
+            http: setting,
+            https: setting,
+        };
+    }
     const settings = proxySetting.split(";").map(x => x.split("=", 2));
-    const result: ProxySettings = {};
+    const result = {};
     for (const [key, value] of settings) {
-        result[key] = new ProxySetting(value);
+        if (value) {
+            result[key] = new ProxySetting(value);
+        }
+    }
+
+    return processResults(result);
+}
+
+function isValidUrl(value: string) {
+    const obj = url.parse(value);
+    return Boolean(obj.hostname);
+}
+
+function processResults(results: { [key: string]: ProxySetting }) {
+    const { http, https } = results;
+    if (http && https) {
+        return { http, https };
+    } else if (http) {
+        return { http, https: http };
+    } else if (https) {
+        return { http: https, https };
+    } else {
+        return null;
     }
-    return result;
 }