diff --git a/packages/https-proxy/lib/server.coffee b/packages/https-proxy/lib/server.coffee index 52433a3462..a409774600 100644 --- a/packages/https-proxy/lib/server.coffee +++ b/packages/https-proxy/lib/server.coffee @@ -98,13 +98,18 @@ class Server res.end() .pipe(res) - _getProxyForUrl: (url) -> - if url == "https://localhost:#{@_sniPort}" + _getProxyForUrl: (urlStr) -> + port = Number(_.get(url.parse(urlStr), 'port')) + + debug('getting proxy URL %o', { port, serverPort: @_port, sniPort: @_sniPort, url: urlStr }) + + if [@_sniPort, @_port].includes(port) ## https://github.com/cypress-io/cypress/issues/4257 - ## this is a tunnel to the SNI server, it should never go through a proxy + ## this is a tunnel to the SNI server or to the main server, + ## it should never go through a proxy return undefined - getProxyForUrl(url) + getProxyForUrl(urlStr) _makeDirectConnection: (req, browserSocket, head) -> { port, hostname } = url.parse("https://#{req.url}") @@ -167,7 +172,7 @@ class Server return makeConnection(@_port) ## else spin up the SNI server - { hostname } = url.parse("http://#{req.url}") + { hostname } = url.parse("https://#{req.url}") if sslServer = sslServers[hostname] return makeConnection(sslServer.port) diff --git a/packages/server/.gitignore b/packages/server/.gitignore index 6c30b65556..47fb47fd69 100644 --- a/packages/server/.gitignore +++ b/packages/server/.gitignore @@ -1,2 +1,3 @@ lib/util/ensure-url.js lib/util/proxy.js +.http-mitm-proxy diff --git a/packages/server/__snapshots__/8_network_error_handling_spec.coffee.js b/packages/server/__snapshots__/8_network_error_handling_spec.coffee.js index 5d59bf569b..fd0d47e674 100644 --- a/packages/server/__snapshots__/8_network_error_handling_spec.coffee.js +++ b/packages/server/__snapshots__/8_network_error_handling_spec.coffee.js @@ -140,3 +140,186 @@ exports['e2e network error handling Cypress does not connect to the upstream pro ` + +exports['e2e network error handling Cypress does not delay a 304 Not Modified in normal network conditions 1'] = ` + +==================================================================================================== + + (Run Starting) + + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ Cypress: 1.2.3 │ + │ Browser: FooBrowser 88 │ + │ Specs: 1 found (network_error_304_handling_spec.js) │ + │ Searched: cypress/integration/network_error_304_handling_spec.js │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + + +──────────────────────────────────────────────────────────────────────────────────────────────────── + + Running: network_error_304_handling_spec.js... (1 of 1) + + + network error 304 handling + ✓ does not retry on 304 not modified + + + 1 passing + + + (Results) + + ┌──────────────────────────────────────────────────┐ + │ Tests: 1 │ + │ Passing: 1 │ + │ Failing: 0 │ + │ Pending: 0 │ + │ Skipped: 0 │ + │ Screenshots: 0 │ + │ Video: true │ + │ Duration: X seconds │ + │ Spec Ran: network_error_304_handling_spec.js │ + └──────────────────────────────────────────────────┘ + + + (Video) + + - Started processing: Compressing to 32 CRF + - Finished processing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds) + + +==================================================================================================== + + (Run Finished) + + + Spec Tests Passing Failing Pending Skipped + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ ✔ network_error_304_handling_spec.js XX:XX 1 1 - - - │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + All specs passed! XX:XX 1 1 - - - + + +` + +exports['e2e network error handling Cypress does not delay a 304 Not Modified behind a proxy 1'] = ` + +==================================================================================================== + + (Run Starting) + + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ Cypress: 1.2.3 │ + │ Browser: FooBrowser 88 │ + │ Specs: 1 found (network_error_304_handling_spec.js) │ + │ Searched: cypress/integration/network_error_304_handling_spec.js │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + + +──────────────────────────────────────────────────────────────────────────────────────────────────── + + Running: network_error_304_handling_spec.js... (1 of 1) + + + network error 304 handling + ✓ does not retry on 304 not modified + + + 1 passing + + + (Results) + + ┌──────────────────────────────────────────────────┐ + │ Tests: 1 │ + │ Passing: 1 │ + │ Failing: 0 │ + │ Pending: 0 │ + │ Skipped: 0 │ + │ Screenshots: 0 │ + │ Video: true │ + │ Duration: X seconds │ + │ Spec Ran: network_error_304_handling_spec.js │ + └──────────────────────────────────────────────────┘ + + + (Video) + + - Started processing: Compressing to 32 CRF + - Finished processing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds) + + +==================================================================================================== + + (Run Finished) + + + Spec Tests Passing Failing Pending Skipped + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ ✔ network_error_304_handling_spec.js XX:XX 1 1 - - - │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + All specs passed! XX:XX 1 1 - - - + + +` + +exports['e2e network error handling Cypress does not delay a 304 Not Modified behind a proxy with transfer-encoding: chunked 1'] = ` + +==================================================================================================== + + (Run Starting) + + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ Cypress: 1.2.3 │ + │ Browser: FooBrowser 88 │ + │ Specs: 1 found (network_error_304_handling_spec.js) │ + │ Searched: cypress/integration/network_error_304_handling_spec.js │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + + +──────────────────────────────────────────────────────────────────────────────────────────────────── + + Running: network_error_304_handling_spec.js... (1 of 1) + + + network error 304 handling + ✓ does not retry on 304 not modified + + + 1 passing + + + (Results) + + ┌──────────────────────────────────────────────────┐ + │ Tests: 1 │ + │ Passing: 1 │ + │ Failing: 0 │ + │ Pending: 0 │ + │ Skipped: 0 │ + │ Screenshots: 0 │ + │ Video: true │ + │ Duration: X seconds │ + │ Spec Ran: network_error_304_handling_spec.js │ + └──────────────────────────────────────────────────┘ + + + (Video) + + - Started processing: Compressing to 32 CRF + - Finished processing: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds) + + +==================================================================================================== + + (Run Finished) + + + Spec Tests Passing Failing Pending Skipped + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ ✔ network_error_304_handling_spec.js XX:XX 1 1 - - - │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + All specs passed! XX:XX 1 1 - - - + + +` diff --git a/packages/server/lib/controllers/proxy.coffee b/packages/server/lib/controllers/proxy.coffee index 7e2d6c972a..8616c393f6 100644 --- a/packages/server/lib/controllers/proxy.coffee +++ b/packages/server/lib/controllers/proxy.coffee @@ -1,19 +1,20 @@ _ = require("lodash") zlib = require("zlib") concat = require("concat-stream") -through = require("through") Promise = require("bluebird") accept = require("http-accept") debug = require("debug")("cypress:server:proxy") cwd = require("../cwd") cors = require("../util/cors") +passthruStream = require("../util/passthru_stream").passthruStream buffers = require("../util/buffers") rewriter = require("../util/rewriter") blacklist = require("../util/blacklist") conditional = require("../util/conditional_stream") networkFailures = require("../util/network_failures") -redirectRe = /^30(1|2|3|7|8)$/ +REDIRECT_STATUS_CODES = [301, 302, 303, 307, 308] +NO_BODY_STATUS_CODES = [204, 304] zlib = Promise.promisifyAll(zlib) @@ -25,6 +26,16 @@ zlibOptions = { isGzipError = (err) -> Object.prototype.hasOwnProperty.call(zlib.constants, err.code) +## https://github.com/cypress-io/cypress/issues/4298 +## https://tools.ietf.org/html/rfc7230#section-3.3.3 +## HEAD, 1xx, 204, and 304 responses should never contain anything after headers +responseMustHaveEmptyBody = (method, statusCode) -> + _.some([ + _.includes(NO_BODY_STATUS_CODES, statusCode), + _.inRange(statusCode, 100, 200), + _.invoke(method, 'toLowerCase') == 'head', + ]) + setCookie = (res, key, val, domainName) -> ## cannot use res.clearCookie because domain ## is not sent correctly @@ -83,16 +94,7 @@ module.exports = { return res.status(503).end() - # if req.headers.accept is "text/event-stream" - # return nodeProxy.web(req, res, { - # secure: false - # ignorePath: true - # target: req.proxiedUrl - # timeout: 0 - # proxyTimeout: 0 - # }) - - thr = through (d) -> @queue(d) + thr = passthruStream() @getHttpContent(thr, req, res, remoteState, config, request) .pipe(res) @@ -164,6 +166,9 @@ module.exports = { wantsSecurityRemoved, }) + if responseMustHaveEmptyBody(req.method, statusCode) + return res.end() + ## if there is nothing to inject then just ## bypass the stream buffer and pipe this back if wantsInjection @@ -269,7 +274,7 @@ module.exports = { catch err ## noop - if redirectRe.test(statusCode) + if REDIRECT_STATUS_CODES.includes(statusCode) newUrl = headers.location ## set cookies to initial=true @@ -279,12 +284,12 @@ module.exports = { ## finally redirect our user agent back to our domain ## by making this an absolute-path-relative redirect - res.redirect(statusCode, newUrl) - else - if headers["x-cypress-file-server-error"] - wantsInjection or= "partial" + return res.redirect(statusCode, newUrl) - setBody(str, statusCode, headers) + if headers["x-cypress-file-server-error"] + wantsInjection or= "partial" + + setBody(str, statusCode, headers) if obj = buffers.take(remoteUrl) wantsInjection = "full" diff --git a/packages/server/lib/util/passthru_stream.js b/packages/server/lib/util/passthru_stream.js new file mode 100644 index 0000000000..dcfca425a8 --- /dev/null +++ b/packages/server/lib/util/passthru_stream.js @@ -0,0 +1,9 @@ +const through = require('through') + +module.exports = { + passthruStream () { + return through(function (chunk) { + this.queue(chunk) + }) + }, +} diff --git a/packages/server/package.json b/packages/server/package.json index 47f674d517..2f9a594d28 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -71,6 +71,7 @@ "eventsource": "1.0.7", "express-session": "1.16.1", "express-useragent": "1.0.12", + "http-mitm-proxy": "0.7.0", "https-proxy-agent": "1.0.0", "inquirer": "3.3.0", "istanbul": "0.4.5", diff --git a/packages/server/test/e2e/8_network_error_handling_spec.coffee b/packages/server/test/e2e/8_network_error_handling_spec.coffee index 76abab69b4..4cd2c57f09 100644 --- a/packages/server/test/e2e/8_network_error_handling_spec.coffee +++ b/packages/server/test/e2e/8_network_error_handling_spec.coffee @@ -1,10 +1,16 @@ _ = require("lodash") +express = require("express") +http = require("http") +https = require("https") path = require("path") net = require("net") -debug = require("debug")("network-error-handling-spec") +request = require("request") +stream = require("stream") +debug = require("debug")("cypress:server:network-error-handling-spec") Promise = require("bluebird") bodyParser = require("body-parser") DebugProxy = require("@cypress/debugging-proxy") +mitmProxy = require("http-mitm-proxy") launcher = require("@packages/launcher") chrome = require("../../lib/browsers/chrome") e2e = require("../support/helpers/e2e") @@ -122,6 +128,9 @@ controllers = { ## https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.5.4 ## "The implication is that this is a temporary condition which will be alleviated after some delay." res.sendStatus(503) + + load304: (req, res) -> + res.type('html').end('') } describe "e2e network error handling", -> @@ -145,6 +154,8 @@ describe "e2e network error handling", -> next() + app.use("/static", express.static(path.join(e2ePath, 'static'))) + app.use(bodyParser.urlencoded({ extended: true })) app.get "/immediate-reset", controllers.immediateReset @@ -154,6 +165,7 @@ describe "e2e network error handling", -> app.get "/works-third-time-else-500/:id", controllers.worksThirdTimeElse500 app.post "/print-body-third-time", controllers.printBodyThirdTime + app.get "/load-304.html", controllers.load304 app.get "/load-img-net-error.html", controllers.loadImgNetError app.get "/load-script-net-error.html", controllers.loadScriptNetError app.get "/print-body-third-time-form", controllers.printBodyThirdTimeForm @@ -278,11 +290,14 @@ describe "e2e network error handling", -> context "Cypress", -> beforeEach -> delete process.env.HTTP_PROXY + delete process.env.HTTPS_PROXY delete process.env.NO_PROXY afterEach -> if @debugProxy @debugProxy.stop() + .then => + @debugProxy = null it "baseurl check tries 5 times in run mode", -> e2e.exec(@, { @@ -432,3 +447,67 @@ describe "e2e network error handling", -> host: 'localhost' port: HTTPS_PORT }) + + ## https://github.com/cypress-io/cypress/issues/4298 + context "does not delay a 304 Not Modified", -> + it "in normal network conditions", -> + e2e.exec(@, { + spec: "network_error_304_handling_spec.js" + video: false + config: { + baseUrl: "http://localhost:#{PORT}" + pageLoadTimeout: 4000 + } + expectedExitCode: 0 + snapshot: true + }) + + it "behind a proxy", -> + @debugProxy = new DebugProxy() + + @debugProxy + .start(PROXY_PORT) + .then => + process.env.HTTP_PROXY = "http://localhost:#{PROXY_PORT}" + process.env.NO_PROXY = "" + .then => + e2e.exec(@, { + spec: "network_error_304_handling_spec.js" + video: false + config: { + baseUrl: "http://localhost:#{PORT}" + pageLoadTimeout: 4000 + } + expectedExitCode: 0 + snapshot: true + }) + + it "behind a proxy with transfer-encoding: chunked", -> + mitmProxy = mitmProxy() + + mitmProxy.onRequest (ctx, callback) -> + callback() + + mitmProxy.listen({ + host: '127.0.0.1' + port: PROXY_PORT + keepAlive: true + httpAgent: http.globalAgent + httpsAgent: https.globalAgent + forceSNI: false + forceChunkedRequest: true + }) + + process.env.HTTP_PROXY = "http://localhost:#{PROXY_PORT}" + process.env.NO_PROXY = "" + + e2e.exec(@, { + spec: "network_error_304_handling_spec.js" + video: false + config: { + baseUrl: "http://localhost:#{PORT}" + pageLoadTimeout: 4000 + } + expectedExitCode: 0 + snapshot: true + }) diff --git a/packages/server/test/integration/http_requests_spec.coffee b/packages/server/test/integration/http_requests_spec.coffee index 1a9b1a506b..90e191431b 100644 --- a/packages/server/test/integration/http_requests_spec.coffee +++ b/packages/server/test/integration/http_requests_spec.coffee @@ -6,6 +6,7 @@ rp = require("request-promise") dns = require("dns") http = require("http") path = require("path") +url = require("url") zlib = require("zlib") str = require("underscore.string") browserify = require("browserify") @@ -93,11 +94,11 @@ describe "Routes", -> ## options including our proxy @rp = (options = {}) => if _.isString(options) - url = options + targetUrl = options options = {} _.defaults options, { - url, + url: targetUrl, proxy: @proxy, jar, simple: false, @@ -3061,21 +3062,51 @@ describe "Routes", -> es.onmessage = (m) => expect(m.data).to.eq("hey") es.close() - # - # it "handles errors when the event source connection cannot be made", (done) -> - # ## it should call req.socket.destroy() when receiving error - # @server.onRequest (req, res) => - # sinon.spy(@server._request, "create") - # sinon.spy(req.socket, "destroy") - # - # es.onerror = (e) => - # expect(@server._request.create).to.be.calledWithMatch({timeout: null}) - # expect(req.socket.destroy).to.be.calledOnce - # done() - # - # es = new EventSource("http://localhost:7777/sse", { - # proxy: @proxy - # }) + + context "when body should be empty", -> + @timeout(1000) + + beforeEach (done) -> + @httpSrv = http.createServer (req, res) -> + { query } = url.parse(req.url, true) + + if _.has(query, 'chunked') + res.setHeader('tranfer-encoding', 'chunked') + else + res.setHeader('content-length', '0') + + res.writeHead(Number(query.status), { + 'x-foo': 'bar' + }) + res.end() + + @httpSrv.listen => + @port = @httpSrv.address().port + + @setup("http://localhost:#{@port}") + .then(_.ary(done, 0)) + + afterEach -> + @httpSrv.close() + + [204, 304, 101, 102, 103].forEach (status) -> + it "passes through a #{status} response immediately", -> + @rp({ + url: "http://localhost:#{@port}/?status=#{status}" + timeout: 100 + }) + .then (res) -> + expect(res.headers['x-foo']).to.eq('bar') + expect(res.statusCode).to.eq(status) + + it "passes through a #{status} response with chunked encoding immediately", -> + @rp({ + url: "http://localhost:#{@port}/?status=#{status}&chunked" + timeout: 100 + }) + .then (res) -> + expect(res.headers['x-foo']).to.eq('bar') + expect(res.statusCode).to.eq(status) context "POST *", -> beforeEach -> diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/network_error_304_handling_spec.js b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/network_error_304_handling_spec.js new file mode 100644 index 0000000000..e2229fa4d8 --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/network_error_304_handling_spec.js @@ -0,0 +1,6 @@ +describe('network error 304 handling', function () { + it('does not retry on 304 not modified', function () { + cy.visit('/load-304.html') + cy.visit('/load-304.html') + }) +})