From 8ddd05c7f133aa5e074124c6935851b797837a88 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Thu, 24 Oct 2019 15:06:41 -0400 Subject: [PATCH 1/2] Keep track of correct URL during request redirects --- packages/server/lib/request.coffee | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/server/lib/request.coffee b/packages/server/lib/request.coffee index a0d0d1b58d11..4b31948d8985 100644 --- a/packages/server/lib/request.coffee +++ b/packages/server/lib/request.coffee @@ -419,7 +419,7 @@ module.exports = (options = {}) -> setRequestCookieHeader: (req, reqUrl, automationFn) -> automationFn('get:cookies', { url: reqUrl }) .then (cookies) -> - debug('getting cookies from browser %o', { reqUrl, cookies }) + debug('got cookies from browser %o', { reqUrl, cookies }) header = cookies.map (cookie) -> "#{cookie.name}=#{cookie.value}" .join("; ") || undefined @@ -475,10 +475,12 @@ module.exports = (options = {}) -> followRedirect = options.followRedirect + currentUrl = options.url + options.followRedirect = (incomingRes) -> req = @ - newUrl = url.resolve(options.url, incomingRes.headers.location) + newUrl = url.resolve(currentUrl, incomingRes.headers.location) ## and when we know we should follow the redirect ## we need to override the init method and @@ -486,10 +488,11 @@ module.exports = (options = {}) -> ## and then grab the cookies for the new url req.init = _.wrap req.init, (orig, opts) => options.onBeforeReqInit -> - self.setCookiesOnBrowser(incomingRes, options.url, automationFn) + self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn) .then (cookies) -> self.setRequestCookieHeader(req, newUrl, automationFn) .then (cookieHeader) -> + currentUrl = newUrl orig.call(req, opts) followRedirect.call(req, incomingRes) @@ -552,8 +555,10 @@ module.exports = (options = {}) -> requestResponses.push(pick(response)) if options.followRedirect + currentUrl = options.url + options.followRedirect = (incomingRes) -> - newUrl = url.resolve(options.url, incomingRes.headers.location) + newUrl = url.resolve(currentUrl, incomingRes.headers.location) ## normalize the url redirects.push([incomingRes.statusCode, newUrl].join(": ")) @@ -567,10 +572,11 @@ module.exports = (options = {}) -> ## first set the new cookies on the browser ## and then grab the cookies for the new url req.init = _.wrap req.init, (orig, opts) => - self.setCookiesOnBrowser(incomingRes, options.url, automationFn) + self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn) .then -> self.setRequestCookieHeader(req, newUrl, automationFn) .then -> + currentUrl = newUrl orig.call(req, opts) ## cause the redirect to happen From 9dc98bad588d9380fc761b6e6aa0f871dbebbd27 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Thu, 24 Oct 2019 15:06:54 -0400 Subject: [PATCH 2/2] Add snapshots for URLs during redirects --- .../__snapshots__/request_spec.coffee.js | 65 +++++++++++++++++++ packages/server/test/unit/request_spec.coffee | 54 +++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 packages/server/__snapshots__/request_spec.coffee.js diff --git a/packages/server/__snapshots__/request_spec.coffee.js b/packages/server/__snapshots__/request_spec.coffee.js new file mode 100644 index 000000000000..0843aa20b760 --- /dev/null +++ b/packages/server/__snapshots__/request_spec.coffee.js @@ -0,0 +1,65 @@ +exports['lib/request #sendPromise followRedirect gets + attaches the cookies at each redirect 1'] = { + "setCalls": [ + { + "currentUrl": "http://localhost:1234/", + "setCookie": "foo=bar" + }, + { + "currentUrl": "http://localhost:1234/B", + "setCookie": "bar=baz" + }, + { + "currentUrl": "http://localhost:1234/B", + "setCookie": "foo=bar" + }, + { + "currentUrl": "http://localhost:1234/", + "setCookie": "quuz=quux" + } + ], + "getCalls": [ + { + "newUrl": "http://localhost:1234/" + }, + { + "newUrl": "http://localhost:1234/B" + }, + { + "newUrl": "http://localhost:1234/B" + }, + { + "newUrl": "http://localhost:1234/B" + } + ] +} + +exports['lib/request #sendStream gets + attaches the cookies at each redirect 1'] = { + "setCalls": [ + { + "currentUrl": "http://localhost:1234/", + "setCookie": "foo=bar" + }, + { + "currentUrl": "http://localhost:1234/B", + "setCookie": "bar=baz" + }, + { + "currentUrl": "http://localhost:1234/B", + "setCookie": "foo=bar" + } + ], + "getCalls": [ + { + "newUrl": "http://localhost:1234/" + }, + { + "newUrl": "http://localhost:1234/B" + }, + { + "newUrl": "http://localhost:1234/B" + }, + { + "newUrl": "http://localhost:1234/B" + } + ] +} diff --git a/packages/server/test/unit/request_spec.coffee b/packages/server/test/unit/request_spec.coffee index f98680fb357e..4460a503f8b9 100644 --- a/packages/server/test/unit/request_spec.coffee +++ b/packages/server/test/unit/request_spec.coffee @@ -3,9 +3,44 @@ require("../spec_helper") _ = require("lodash") http = require("http") Request = require("#{root}lib/request") +snapshot = require("snap-shot-it") request = Request({timeout: 100}) +testAttachingCookiesWith = (fn) -> + set = sinon.spy(request, 'setCookiesOnBrowser') + get = sinon.spy(request, 'setRequestCookieHeader') + + nock("http://localhost:1234") + .get("/") + .reply(302, "", { + 'set-cookie': 'foo=bar' + location: "/B" + }) + .get("/B") + .reply(302, "", { + 'set-cookie': 'bar=baz' + location: "/B" + }) + .get("/B") + .reply(200, "", { + 'set-cookie': 'quuz=quux' + }) + + fn() + .then -> + snapshot({ + setCalls: set.getCalls().map (call) -> + { + currentUrl: call.args[1], + setCookie: call.args[0].headers['set-cookie'] + } + getCalls: get.getCalls().map (call) -> + { + newUrl: _.get(call, 'args.1') + } + }) + describe "lib/request", -> beforeEach -> @fn = sinon.stub() @@ -722,6 +757,12 @@ describe "lib/request", -> expect(resp.status).to.eq(200) expect(resp).not.to.have.property("redirectedToUrl") + it "gets + attaches the cookies at each redirect", -> + testAttachingCookiesWith => + request.sendPromise({}, @fn, { + url: "http://localhost:1234/" + }) + context "form=true", -> beforeEach -> nock("http://localhost:8080") @@ -843,3 +884,16 @@ describe "lib/request", -> beginFn() expect(request.create).to.be.calledOnce expect(request.create).to.be.calledWith(options) + + it "gets + attaches the cookies at each redirect", -> + testAttachingCookiesWith => + request.sendStream({}, @fn, { + url: "http://localhost:1234/" + followRedirect: _.stubTrue + }) + .then (fn) => + req = fn() + + new Promise (resolve, reject) => + req.on('response', resolve) + req.on('error', reject)