Merge pull request #8112 from dannyzaken/danny-fixes

fixed construct_url to handle ipv6 addresses

src/server/system_services/system_server.js

@@ -1170,29 +1170,36 @@ function _list_s3_addresses(system) {
             }).toString()
         }];
     }
-    return system.system_address
-        .filter(addr =>
-            addr.service === 's3' &&
-            addr.api === 's3' &&
-            addr.secure
-        )
-        .sort((addr1, addr2) => {
-            // Prefer external addresses.
-            if (addr1.kind !== addr2.kind) {
-                return addr1.kind === 'EXTERNAL' ? -1 : 1;
-            }
 
-            // Prefer addresses with higher weight.
-            return Math.sign(addr2.weight - addr1.weight);
-        })
-        .map(addr => {
-            const { kind, hostname, port } = addr;
-            const url = url_utils.construct_url({ protocol: 'https', hostname, port });
-            return {
-                kind: kind,
-                address: url.toString()
-            };
-        });
+    try {
+        return system.system_address
+            .filter(addr =>
+                addr.service === 's3' &&
+                addr.api === 's3' &&
+                addr.secure
+            )
+            .sort((addr1, addr2) => {
+                // Prefer external addresses.
+                if (addr1.kind !== addr2.kind) {
+                    return addr1.kind === 'EXTERNAL' ? -1 : 1;
+                }
+
+                // Prefer addresses with higher weight.
+                return Math.sign(addr2.weight - addr1.weight);
+            })
+            .map(addr => {
+                const { kind, hostname, port } = addr;
+                const url = url_utils.construct_url({ protocol: 'https', hostname, port });
+                return {
+                    kind: kind,
+                    address: url.toString()
+                };
+            });
+    } catch (err) {
+        dbg.error('list_s3_addresses: failed to list s3 addresses', err);
+        return [];
+    }
+
 }
 
 async function _get_endpoint_groups() {

src/test/unit_tests/jest_tests/test_url_utils.test.js

@@ -0,0 +1,89 @@
+/* Copyright (C) 2016 NooBaa */
+'use strict';
+
+const { construct_url } = require('../../../util/url_utils');
+
+
+describe('test url_utils.js', () => {
+
+    describe('construct_url', () => {
+        it('should construct a valid URL', () => {
+            const def = {
+                protocol: 'https',
+                hostname: 'localhost',
+                port: 3000
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('https://localhost:3000/');
+        });
+
+        it('should throw an error when hostname is missing', () => {
+            const def = {
+                protocol: 'http',
+                port: 3000
+            };
+            expect(() => construct_url(def)).toThrow('Invalid definition, hostname is mandatory');
+        });
+
+        it('should construct a valid URL with default protocol', () => {
+            const def = {
+                hostname: 'localhost',
+                port: 3000
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('http://localhost:3000/');
+        });
+
+        it('should construct a valid URL with IPV6 hostname', () => {
+            const def = {
+                hostname: '2403:4800:54:710::eea',
+                port: 3000
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('http://[2403:4800:54:710::eea]:3000/');
+        });
+
+        it('should construct a valid URL with IPV6 and hostname wrapped', () => {
+            const def = {
+                hostname: '[2403:4800:54:710::eea]',
+                port: 3000
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('http://[2403:4800:54:710::eea]:3000/');
+        });
+
+        it('should construct a valid URL with default protocol and no port', () => {
+            const def = {
+                hostname: 'localhost'
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('http://localhost/');
+        });
+
+        it('should construct a valid URL with default protocol and no port and IPV6 hostname', () => {
+            const def = {
+                hostname: '2403:4800:54:710::eea'
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('http://[2403:4800:54:710::eea]/');
+        });
+
+        it('should construct a valid URL with IPV4 hostname', () => {
+            const def = {
+                hostname: '127.0.0.1'
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('http://127.0.0.1/');
+        });
+
+        it('should construct a valid URL with IPV4 hostname and port', () => {
+            const def = {
+                hostname: '127.0.0.1',
+                port: 443
+            };
+            const url = construct_url(def);
+            expect(url.toString()).toBe('http://127.0.0.1:443/');
+        });
+    });
+
+});

src/util/url_utils.js

@@ -4,6 +4,7 @@
 const _ = require('lodash');
 const url = require('url');
 const querystring = require('querystring');
+const net = require('net');
 
 const QUICK_PARSE_REGEXP = /^\s*(\w+:)?(\/\/)?(([^:/[\]]*)|\[([a-fA-F0-9:.]*)\])?(:\d*)?(\/[^?#]*)?(\?[^#]*)?(#.*)?\s*$/;
 
@@ -47,11 +48,17 @@ function quick_parse(url_string, parse_query_string) {
 }
 
 function construct_url(def) {
-    const { protocol = 'http', hostname, port } = def;
+    const { protocol = 'http', port } = def;
+    let { hostname } = def;
     if (!hostname) {
         throw new Error('Invalid definition, hostname is mandatory');
     }
 
+    // check if hostname is an IPV6. if hostname is already wrapped with brackets, net.isIPv6 returns false.
+    if (net.isIPv6(hostname)) {
+        hostname = `[${hostname}]`;
+    }
+
     return new URL(port ?
         `${protocol || 'http'}://${hostname}:${port}` :
         `${protocol || 'http'}://${hostname}`