Skip to content

Commit

Permalink
fix(middleware): simplify stripHost.
Browse files Browse the repository at this point in the history
The stripHost middleware only adds a modified url to the request.
That modified url is only used one place. By converting the middleware
to a module, the code is simpler and beforeMiddleware modules can reuse
karma middleware. (One alternative considered was to move the stripHost
in the chain before the beforeMiddleware, but this form seems better).
  • Loading branch information
johnjbarton committed Aug 16, 2018
1 parent eeadcf2 commit f094a74
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 63 deletions.
7 changes: 6 additions & 1 deletion lib/middleware/karma.js
Expand Up @@ -17,6 +17,7 @@ var url = require('url')
var _ = require('lodash')

var log = require('../logger').create('middleware:karma')
var stripHost = require('./strip_host').stripHost

function urlparse (urlStr) {
var urlObj = url.parse(urlStr, true)
Expand Down Expand Up @@ -90,7 +91,11 @@ function createKarmaMiddleware (
var customClientContextFile = injector.get('config.customClientContextFile')
var includeCrossOriginAttribute = injector.get('config.crossOriginAttribute')

var requestUrl = request.normalizedUrl.replace(/\?.*/, '')
var normalizedUrl = stripHost(request.url) || request.url
// For backwards compatibility in middleware plugins, remove in v4.
request.normalizedUrl = normalizedUrl

var requestUrl = normalizedUrl.replace(/\?.*/, '')
var requestedRangeHeader = request.headers['range']

// redirect /__karma__ to /__karma__ (trailing slash)
Expand Down
15 changes: 4 additions & 11 deletions lib/middleware/strip_host.js
@@ -1,18 +1,11 @@
/**
* Strip host middleware is responsible for stripping hostname from request path
* Strip hostname from request path
* This to handle requests that uses (normally over proxies) an absoluteURI as request path
*/

function createStripHostMiddleware () {
return function (request, response, next) {
function stripHostFromUrl (url) {
return url.replace(/^http[s]?:\/\/([a-z\-.:\d]+)\//, '/')
}

request.normalizedUrl = stripHostFromUrl(request.url) || request.url
next()
}
function stripHostFromUrl (url) {
return url.replace(/^http[s]?:\/\/([a-z\-.:\d]+)\//, '/')
}

// PUBLIC API
exports.create = createStripHostMiddleware
exports.stripHost = stripHostFromUrl
2 changes: 0 additions & 2 deletions lib/web-server.js
Expand Up @@ -10,7 +10,6 @@ const Promise = require('bluebird')
const common = require('./middleware/common')
const runnerMiddleware = require('./middleware/runner')
const stopperMiddleware = require('./middleware/stopper')
const stripHostMiddleware = require('./middleware/strip_host')
const karmaMiddleware = require('./middleware/karma')
const sourceFilesMiddleware = require('./middleware/source_files')
const proxyMiddleware = require('./middleware/proxy')
Expand Down Expand Up @@ -65,7 +64,6 @@ function createWebServer (injector, config) {

handler.use(injector.invoke(runnerMiddleware.create))
handler.use(injector.invoke(stopperMiddleware.create))
handler.use(injector.invoke(stripHostMiddleware.create))
handler.use(injector.invoke(karmaMiddleware.create))
handler.use(injector.invoke(sourceFilesMiddleware.create))
// TODO(vojta): extract the proxy into a plugin
Expand Down
65 changes: 16 additions & 49 deletions test/unit/middleware/strip_host.spec.js
@@ -1,61 +1,28 @@
var mocks = require('mocks')

describe('middleware.strip_host', function () {
var nextSpy
var HttpRequestMock = mocks.http.ServerRequest

var createStripHostMiddleware = require('../../../lib/middleware/strip_host').create

var handler = nextSpy = null

beforeEach(function () {
nextSpy = sinon.spy()
handler = createStripHostMiddleware(null, null, '/base/path')
return handler
})
const stripHost = require('../../../lib/middleware/strip_host').stripHost

it('should strip request with IP number', function (done) {
var request = new HttpRequestMock('http://192.12.31.100/base/a.js?123345')
handler(request, null, nextSpy)

expect(request.normalizedUrl).to.equal('/base/a.js?123345')
expect(nextSpy).to.have.been.called
return done()
it('should strip request with IP number', function () {
const normalizedUrl = stripHost('http://192.12.31.100/base/a.js?123345')
expect(normalizedUrl).to.equal('/base/a.js?123345')
})

it('should strip request with absoluteURI', function (done) {
var request = new HttpRequestMock('http://localhost/base/a.js?123345')
handler(request, null, nextSpy)

expect(request.normalizedUrl).to.equal('/base/a.js?123345')
expect(nextSpy).to.have.been.called
return done()
it('should strip request with absoluteURI', function () {
const normalizedUrl = stripHost('http://localhost/base/a.js?123345')
expect(normalizedUrl).to.equal('/base/a.js?123345')
})

it('should strip request with absoluteURI and port', function (done) {
var request = new HttpRequestMock('http://localhost:9876/base/a.js?123345')
handler(request, null, nextSpy)

expect(request.normalizedUrl).to.equal('/base/a.js?123345')
expect(nextSpy).to.have.been.called
return done()
it('should strip request with absoluteURI and port', function () {
const normalizedUrl = stripHost('http://localhost:9876/base/a.js?123345')
expect(normalizedUrl).to.equal('/base/a.js?123345')
})

it('should strip request with absoluteURI over HTTPS', function (done) {
var request = new HttpRequestMock('https://karma-runner.github.io/base/a.js?123345')
handler(request, null, nextSpy)

expect(request.normalizedUrl).to.equal('/base/a.js?123345')
expect(nextSpy).to.have.been.called
return done()
it('should strip request with absoluteURI over HTTPS', function () {
const normalizedUrl = stripHost('https://karma-runner.github.io/base/a.js?123345')
expect(normalizedUrl).to.equal('/base/a.js?123345')
})

return it('should return same url as passed one', function (done) {
var request = new HttpRequestMock('/base/b.js?123345')
handler(request, null, nextSpy)

expect(request.normalizedUrl).to.equal('/base/b.js?123345')
expect(nextSpy).to.have.been.called
return done()
it('should return same url as passed one', function () {
const normalizedUrl = stripHost('/base/b.js?123345')
expect(normalizedUrl).to.equal('/base/b.js?123345')
})
})

0 comments on commit f094a74

Please sign in to comment.