Skip to content

Commit 70ed1c4

Browse files
koolcsamccone
authored andcommitted
[Bugfix] Allow for multiple ? in outgoing urls.
Without this fix urls that had multiple ? in them would drop sections of the url since before there was an assumption of there only being one.
1 parent 361d4e3 commit 70ed1c4

File tree

2 files changed

+7
-5
lines changed

2 files changed

+7
-5
lines changed

lib/http-proxy/common.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ common.urlJoin = function() {
141141
lastSegs = last.split('?'),
142142
retSegs;
143143

144-
args[lastIndex] = lastSegs[0];
144+
args[lastIndex] = lastSegs.shift();
145145

146146
//
147147
// Join all strings, but remove empty strings so we don't get extra slashes from
@@ -155,7 +155,9 @@ common.urlJoin = function() {
155155

156156
// Only join the query string if it exists so we don't have trailing a '?'
157157
// on every request
158-
lastSegs[1] && retSegs.push(lastSegs[1]);
158+
159+
// Handle case where there could be multiple ? in the URL.
160+
retSegs.concat(lastSegs);
159161

160162
return retSegs.join('?')
161163
};

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -203,13 +203,13 @@ describe('lib/http-proxy/common.js', function () {
203203
expect(outgoing.path).to.eql('/forward/static/path');
204204
})
205205

206-
it('should not modify the query string', function () {
206+
it.only('should not modify the query string', function () {
207207
var outgoing = {};
208208
common.setupOutgoing(outgoing, {
209209
target: { path: '/forward' },
210-
}, { url: '/?foo=bar//&target=http://foobar.com/' });
210+
}, { url: '/?foo=bar//&target=http://foobar.com/?a=1%26b=2&other=2' });
211211

212-
expect(outgoing.path).to.eql('/forward/?foo=bar//&target=http://foobar.com/');
212+
expect(outgoing.path).to.eql('/forward/?foo=bar//&target=http://foobar.com/?a=1%26b=2&other=2');
213213
})
214214
});
215215

0 commit comments

Comments
 (0)