fix: do not throw from getUrl on an invalid Host or request target

lib/request.js

@@ -458,6 +458,17 @@ function patch(Request) {
         if (this._cacheURL !== this.url) {
             var protocol = this.isSecure() ? 'https://' : 'http://';
             var base = protocol + (this.headers.host || 'localhost');
+
+            // The Host header and the request target are both client-supplied
+            // and Node's HTTP parser validates neither as a URL, so either can
+            // be something new URL() rejects - a "foo|bar" host, or an
+            // absolute-form target like "http://a:99999/x". getUrl() is called
+            // from Router.lookup on every request, where nothing catches, so
+            // throwing here takes down the process. Fall back instead.
+            if (!URL.canParse(base)) {
+                base = protocol + 'localhost';
+            }
+
             // For origin-form targets (starting with "/"), concatenate into
             // one URL string instead of new URL(this.url, base), which would
             // treat a leading "//" as a network-path reference and drop part
@@ -467,10 +478,16 @@ function patch(Request) {
             // path as "//evil.com/x" on the original host. Other forms
             // (e.g. "*" for "OPTIONS *") don't start with "/" and keep the
             // two-arg form.
-            this._url =
-                this.url.charAt(0) === '/'
+            if (this.url.charAt(0) === '/') {
+                this._url = URL.canParse(base + this.url)
                     ? new URL(base + this.url)
-                    : new URL(this.url, base);
+                    : new URL(base + '/');
+            } else {
+                this._url = URL.canParse(this.url, base)
+                    ? new URL(this.url, base)
+                    : new URL(base + '/');
+            }
+
             this._cacheURL = this.url;
         }
         return this._url;

test/request.test.js

@@ -1,6 +1,9 @@
 'use strict';
 /* eslint-disable func-names */
 
+var http = require('http');
+var net = require('net');
+
 var restifyClients = require('restify-clients');
 var validator = require('validator');
 
@@ -374,3 +377,57 @@ test(module, 'getUrl result is cached across calls', function(t) {
         t.end();
     });
 });
+
+test(module, 'should not crash when the Host header is not a valid URL host', function(t) {
+    SERVER.get('/hosthdr', function(req, res, next) {
+        res.send({ pathname: req.path() });
+        return next();
+    });
+
+    // The Host header is client-supplied and Node does not validate it as a
+    // URL authority. getUrl() must not throw on it: it is called from
+    // Router.lookup on every request, where nothing catches, so a throw
+    // here takes down the process.
+    var opts = {
+        agent: false,
+        headers: { Host: 'foo|bar' },
+        hostname: '127.0.0.1',
+        method: 'GET',
+        path: '/hosthdr',
+        port: PORT
+    };
+
+    http.request(opts, function(res) {
+        t.equal(res.statusCode, 200);
+        res.resume();
+        res.on('end', function() {
+            t.end();
+        });
+    }).end();
+});
+
+test(module, 'should not crash when the request target is not a valid URL', function(t) {
+    // An absolute-form request target is client-supplied and Node's HTTP
+    // parser does not validate it as a URL either, so getUrl() must not
+    // throw on it. Needs a raw socket: http.request only emits
+    // origin-form targets.
+    var response = '';
+    var socket = net.connect(PORT, '127.0.0.1', function() {
+        socket.write(
+            'GET http://a:99999/x HTTP/1.1\r\n' +
+                'Host: 127.0.0.1:' +
+                PORT +
+                '\r\n' +
+                'Connection: close\r\n\r\n'
+        );
+    });
+
+    socket.on('data', function(chunk) {
+        response += chunk;
+    });
+
+    socket.on('close', function() {
+        t.ok(/^HTTP\/1\.1 \d{3}/.test(response), 'server sent a response');
+        t.end();
+    });
+});