fix common.urlJoin when target has query string

lib/http-proxy/common.js

@@ -49,7 +49,7 @@ common.setupOutgoing = function(outgoing, options, req, forward) {
   if (options.auth) {
     outgoing.auth = options.auth;
   }
-  
+
   if (options.ca) {
       outgoing.ca = options.ca;
   }
@@ -170,35 +170,35 @@ common.hasEncryptedConnection = function(req) {
  */
 
 common.urlJoin = function() {
-    //
-    // We do not want to mess with the query string. All we want to touch is the path.
-    //
+  //
+  // join url and merge all query string.
+  //
   var args = Array.prototype.slice.call(arguments),
-      lastIndex = args.length - 1,
-      last = args[lastIndex],
-      lastSegs = last.split('?'),
-      retSegs;
-
-  args[lastIndex] = lastSegs.shift();
+    queryParams = [],
+    queryParamRaw = '',
+    retSegs;
+
+  args.forEach((url, index) => {
+    var qpStart = url.indexOf('?')
+    if (qpStart !== -1) {
+      queryParams.push(url.substring(qpStart + 1))
+      args[index] = url.substring(0, qpStart)
+    }
+  });
+  queryParamRaw = queryParams.filter(Boolean).join('&');
 
   //
   // Join all strings, but remove empty strings so we don't get extra slashes from
   // joining e.g. ['', 'am']
   //
-  retSegs = [
-    args.filter(Boolean).join('/')
-        .replace(/\/+/g, '/')
-        .replace('http:/', 'http://')
-        .replace('https:/', 'https://')
-  ];
+  retSegs = args.filter(Boolean).join('/')
+    .replace(/\/+/g, '/')
+    .replace('http:/', 'http://')
+    .replace('https:/', 'https://');
 
   // Only join the query string if it exists so we don't have trailing a '?'
   // on every request
-
-  // Handle case where there could be multiple ? in the URL.
-  retSegs.push.apply(retSegs, lastSegs);
-
-  return retSegs.join('?')
+  return queryParamRaw ? retSegs + '?' + queryParamRaw : retSegs
 };
 
 /**

test/lib-http-proxy-common-test.js

@@ -227,6 +227,15 @@ describe('lib/http-proxy/common.js', function () {
       expect(outgoing.path).to.eql('/forward/?foo=bar//&target=http://foobar.com/?a=1%26b=2&other=2');
     })
 
+    it('target path has query string', function () {
+      var outgoing = {};
+      common.setupOutgoing(outgoing, {
+        target: { path: '/forward?f=1' },
+      }, { url: '/src?s=1' });
+
+      expect(outgoing.path).to.eql('/forward/src?f=1&s=1');
+    })
+
     //
     // This is the proper failing test case for the common.join problem
     //