From 9a388d016823d70a672d3c8f94be2f59e120322c Mon Sep 17 00:00:00 2001 From: Romain Prieto Date: Tue, 23 May 2017 12:53:17 +1000 Subject: [PATCH] Pre-process origin matchers for better performance (regexes created once only) --- src/actual.js | 5 +++-- src/origin-matcher.js | 24 ++++++++++++++++++++++++ src/origin.js | 16 ---------------- src/preflight.js | 5 +++-- test/origin.spec.js | 27 ++++++++++++++++++--------- 5 files changed, 48 insertions(+), 29 deletions(-) create mode 100644 src/origin-matcher.js delete mode 100644 src/origin.js diff --git a/src/actual.js b/src/actual.js index 1c2337f..d6d4d30 100644 --- a/src/actual.js +++ b/src/actual.js @@ -1,13 +1,14 @@ -var origin = require('./origin.js') +var originMatcher = require('./origin-matcher.js') var constants = require('./constants.js') exports.handler = function (options) { + var matcher = originMatcher.create(options.origins) return function (req, res, next) { var originHeader = req.headers['origin'] // If either no origin was set, or the origin isn't supported, continue // without setting any headers - if (!originHeader || !origin.allowed(options.origins || [], originHeader)) { + if (!originHeader || !matcher(originHeader)) { return next() } diff --git a/src/origin-matcher.js b/src/origin-matcher.js new file mode 100644 index 0000000..ddfdf3e --- /dev/null +++ b/src/origin-matcher.js @@ -0,0 +1,24 @@ + +exports.create = function (allowedOrigins) { + // pre-compile list of matchers, so regexes are only built once + var matchers = allowedOrigins.map(createMatcher) + // does a given request Origin match the list? + return function (requestOrigin) { + if (requestOrigin) { + return matchers.some(matcher => matcher(requestOrigin)) + } else { + return false + } + } +} + +function createMatcher (allowedOrigin) { + if (allowedOrigin.indexOf('*') === -1) { + // simple string comparison + return requestOrigin => requestOrigin === allowedOrigin + } else { + // need to build a regex + var regex = '^' + allowedOrigin.replace('.', '\\.').replace('*', '.*') + '$' + return requestOrigin => requestOrigin.match(regex) + } +} diff --git a/src/origin.js b/src/origin.js deleted file mode 100644 index 1d88cab..0000000 --- a/src/origin.js +++ /dev/null @@ -1,16 +0,0 @@ - -exports.allowed = function (list, requestOrigin) { - function match (origin) { - if (origin.indexOf('*') !== -1) { - var regex = '^' + origin.replace('.', '\\.').replace('*', '.*') + '$' - return requestOrigin.match(regex) - } else { - return requestOrigin === origin - } - } - if (requestOrigin) { - return list.some(match) - } else { - return false - } -} diff --git a/src/preflight.js b/src/preflight.js index a00d37e..3edeb0a 100644 --- a/src/preflight.js +++ b/src/preflight.js @@ -1,13 +1,14 @@ -var origin = require('./origin') +var originMatcher = require('./origin-matcher') var constants = require('./constants.js') exports.handler = function (options) { + var matcher = originMatcher.create(options.origins) return function (req, res, next) { if (req.method !== 'OPTIONS') return next() // 6.2.1 and 6.2.2 var originHeader = req.headers['origin'] - if (origin.allowed(options.origins, originHeader) === false) return next() + if (!matcher(originHeader)) return next() // 6.2.3 var requestedMethod = req.headers[constants['AC_REQ_METHOD']] diff --git a/test/origin.spec.js b/test/origin.spec.js index 57f494a..547a9bb 100644 --- a/test/origin.spec.js +++ b/test/origin.spec.js @@ -1,46 +1,55 @@ /* eslint-env mocha */ require('should') -var origin = require('../src/origin') +var originMatcher = require('../src/origin-matcher') describe('Origin list', function () { it('returns false if the request has no origin', function () { var list = ['http://api.myapp.com', 'http://www.myapp.com'] - origin.allowed(list, null).should.eql(false) + var matcher = originMatcher.create(list) + matcher(null).should.eql(false) + matcher('').should.eql(false) }) it('returns false if the origin is not in the list', function () { var list = ['http://api.myapp.com', 'http://www.myapp.com'] - origin.allowed(list, 'http://random-website.com').should.eql(false) + var matcher = originMatcher.create(list) + matcher('http://random-website.com').should.eql(false) }) it('returns true if the origin matched', function () { var list = ['http://api.myapp.com', 'http://www.myapp.com'] - origin.allowed(list, 'http://api.myapp.com').should.eql(true) + var matcher = originMatcher.create(list) + matcher('http://api.myapp.com').should.eql(true) }) it('does not do partial matches by default', function () { var list = ['http://api.myapp.com', 'http://www.myapp.com'] - origin.allowed(list, 'api.myapp.com').should.eql(false) + var matcher = originMatcher.create(list) + matcher('api.myapp.com').should.eql(false) }) it('always matches if the list contains *', function () { var list = ['*'] - origin.allowed(list, 'http://random-website.com').should.eql(true) + var matcher = originMatcher.create(list) + matcher('http://random-website.com').should.eql(true) }) it('supports * for partial matches', function () { var list = ['http://*.myapp.com', 'http://other-website.com'] - origin.allowed(list, 'http://api.myapp.com').should.eql(true) + var matcher = originMatcher.create(list) + matcher('http://api.myapp.com').should.eql(true) }) it('escapes the partial regex properly', function () { // the "." should be a real dot, not mean "[any character]myapp" var list = ['http://*.myapp.com', 'http://other-website.com'] - origin.allowed(list, 'http://xmyapp.com').should.eql(false) + var matcher = originMatcher.create(list) + matcher('http://xmyapp.com').should.eql(false) }) it('returns false if there was no partial match', function () { var list = ['http://*.myapp.com'] - origin.allowed(list, 'http://random-website.com').should.eql(false) + var matcher = originMatcher.create(list) + matcher('http://random-website.com').should.eql(false) }) })