Skip to content

Commit

Permalink
fix: consistent and more intuitive response for the serveHttp function
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicolasdao
nicolasdao committed Aug 30, 2017
1 parent 226e4cb commit 80e6d8d
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 43 deletions.
22 changes: 22 additions & 0 deletions src/routing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
*/

/**
* Converts a route string template to a route detail object with specific info about that route.
* From 'users/{username}/account/{id}' to { "name": "/users/{username}/account/{id}/", "params": [ "username", "id"], "regex": {} }
*
* @param {String} route Uri path
* @return {Object} e.g. { "name": "/users/{username}/account/{id}/", "params": [ "username", "id"], "regex": {} }
*/
Expand All @@ -27,6 +29,26 @@ const getRouteDetails = route => {
}
}

/**
* Matches a URI path against a route object (probably generated by the 'getRouteDetails' function). If it matches,
* it returns an object containing the part that matches, as well as the map of arguments that matches. Example:
* const route = getRouteDetails('users/{username}/account/{accountId}')
* result = matchRoute('/users/nic/account/1/blabla', route)
* // {
* // "match": "/users/nic/account/1/",
* // "route": "/users/nic/account/1/hu",
* // "parameters": {
* // "username": "nic",
* // "accountId": "1"
* // }
* // }
*
* @param {String} reqPath URI path (e.g. '/users/nic/account/1/blabla')
* @param {Object} route Route details
* @param {Array} route.params Array of variable names (e.g. ['username', 'accountId'])
* @param {Regex} route.regex Regex
* @return {Object} null if there is no match, Object as described in the example above if it does.
*/
const matchRoute = (reqPath, { params, regex }) => {
if (!reqPath)
return null
Expand Down
45 changes: 17 additions & 28 deletions src/webfunc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,37 +84,24 @@ const handleHttpRequest = (req, res, appconfig) => Promise.resolve(appconfig ||
const sameOrigin = referer.indexOf(origin) == 0

if (noConfig) {
if (!sameOrigin) {
return setResponseHeaders(res, appConfig).then(res => {
res.status(403).send(`Forbidden - CORS issue. Origin '${origin}' is not allowed.`)
return res
})
}
if (method != 'head' && method != 'get' && method != 'options' && method != 'post') {
return setResponseHeaders(res, appConfig).then(res => {
res.status(403).send(`Forbidden - CORS issue. Method '${method.toUpperCase()}' is not allowed.`)
return res
})
}
if (!sameOrigin)
return setResponseHeaders(res, appConfig).then(res => res.status(403).send(`Forbidden - CORS issue. Origin '${origin}' is not allowed.`))

if (method != 'head' && method != 'get' && method != 'options' && method != 'post')
return setResponseHeaders(res, appConfig).then(res => res.status(403).send(`Forbidden - CORS issue. Method '${method.toUpperCase()}' is not allowed.`))
}
// Check CORS

if (!origins['*'] && Object.keys(origins).length != 0 && !(origin in origins)) {
return setResponseHeaders(res, appConfig).then(res => {
res.status(403).send(`Forbidden - CORS issue. Origin '${origin}' is not allowed.`)
return res
})
}
if (Object.keys(methods).length != 0 && method != 'get' && method != 'head' && !(method in methods)) {
return setResponseHeaders(res, appConfig).then(res => {
res.status(403).send(`Forbidden - CORS issue. Method '${method.toUpperCase()}' is not allowed.`)
return res
})
}
if (!origins['*'] && Object.keys(origins).length != 0 && !(origin in origins))
return setResponseHeaders(res, appConfig).then(res => res.status(403).send(`Forbidden - CORS issue. Origin '${origin}' is not allowed.`))

if (Object.keys(methods).length != 0 && method != 'get' && method != 'head' && !(method in methods))
return setResponseHeaders(res, appConfig).then(res => res.status(403).send(`Forbidden - CORS issue. Method '${method.toUpperCase()}' is not allowed.`))

if (method == 'head' || method == 'options')
return setResponseHeaders(res, appConfig).then(res => res.status(200).send())
})
.then(() => ({ req, res }))

/**
* Returns a function (req, res) => ... that the Google Cloud Function expects.
Expand Down Expand Up @@ -171,7 +158,7 @@ const serveHttp = (arg1, arg2, arg3) => {
if (!r) {
return setResponseHeaders(res, _appconfig).then(res => {
res.status(404).send(`Endpoint '${httpEndpoint}' not found.`)
return res
return { req, res }
})
}
else
Expand All @@ -182,6 +169,7 @@ const serveHttp = (arg1, arg2, arg3) => {
.then(() => !res.headersSent
? setResponseHeaders(res, _appconfig).then(res => httpNextRequest(req, res, Object.assign(parameters, getRequestParameters(req))))
: res)
.then(() => ({ req, res }))
}

const firebaseHosting = _appconfig.hosting == 'firebase'
Expand Down Expand Up @@ -234,19 +222,20 @@ const serveHttpEndpoints = (endpoints, appconfig) => {
.filter(e => e.endpoint.route.name != '/' && e.route && (e.endpoint.method == httpMethod || !e.endpoint.method))
.sort((a, b) => b.route.match.length - a.route.match.length)[0] || {}).endpoint

if (!endpoint)
return res.send(404, `Endpoint '${httpEndpoint}' for method ${httpMethod} not found.`)
if (!endpoint)
res.send(404, `Endpoint '${httpEndpoint}' for method ${httpMethod} not found.`)

const next = endpoint.next || (() => Promise.resolve(null))
if (typeof(next) != 'function')
return res.send(500, `Wrong argument exception. Endpoint '${httpEndpoint}' for method ${httpMethod} defines a 'next' argument that is not a function similar to '(req, res, params) => ...'.`)
res.send(500, `Wrong argument exception. Endpoint '${httpEndpoint}' for method ${httpMethod} defines a 'next' argument that is not a function similar to '(req, res, params) => ...'.`)

const parameters = getRequestParameters(req)
const requestParameters = matchRoute(httpEndpoint, endpoint.route).parameters

return next(req, res, Object.assign(parameters, requestParameters))
})
: res)
.then(() => ({ req, res }))

const firebaseHosting = _appconfig.hosting == 'firebase'
return firebaseHosting ? functions.https.onRequest(cloudFunction) : cloudFunction
Expand Down
45 changes: 30 additions & 15 deletions test/webfunc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ describe('webfunc', () =>
})
const res = httpMocks.createResponse()
const appconfig = {}
return handleHttpRequest(req, res, appconfig).then(res => {
return handleHttpRequest(req, res, appconfig).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Origin \'http://localhost:8080\' is not allowed.')
})
Expand Down Expand Up @@ -220,7 +221,8 @@ describe('webfunc', () =>
'Access-Control-Max-Age': '1296000'
}
}
return handleHttpRequest(req, res, appconfig).then(res => {
return handleHttpRequest(req, res, appconfig).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Origin \'http://localhost:8080\' is not allowed.')
})
Expand Down Expand Up @@ -273,7 +275,8 @@ describe('webfunc', () =>
'Access-Control-Max-Age': '1296000'
}
}
return handleHttpRequest(req, res, appconfig).then(res => {
return handleHttpRequest(req, res, appconfig).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Method \'POST\' is not allowed.')
})
Expand Down Expand Up @@ -319,7 +322,8 @@ describe('webfunc', () =>
})
const res = httpMocks.createResponse()
const appconfig = {}
return handleHttpRequest(req, res, appconfig).then(res => {
return handleHttpRequest(req, res, appconfig).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Method \'PUT\' is not allowed.')
})
Expand All @@ -343,7 +347,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res._getData(),'Hello World')
})
})))
Expand All @@ -366,7 +371,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Origin \'http://localhost:8080\' is not allowed.')
})
Expand Down Expand Up @@ -397,7 +403,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res._getData(),'Hello World')
})
})))
Expand Down Expand Up @@ -427,7 +434,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res._getData(),'Hello World')
})
})))
Expand Down Expand Up @@ -457,7 +465,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Origin \'http://localhost:8080\' is not allowed.')
})
Expand Down Expand Up @@ -488,7 +497,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res._getData(),'Hello World')
})
})))
Expand Down Expand Up @@ -518,7 +528,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Method \'POST\' is not allowed.')
})
Expand Down Expand Up @@ -549,7 +560,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res._getData(),'Hello World')
})
})))
Expand All @@ -572,7 +584,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 403)
assert.equal(res._getData(), 'Forbidden - CORS issue. Method \'PUT\' is not allowed.')
})
Expand Down Expand Up @@ -603,7 +616,8 @@ describe('webfunc', () =>
res.status(200).send('Hello World')
return res
}, appconfig)
return fn(req, res).then(res => {
return fn(req, res).then(({req, res}) => {
assert.isOk(req)
assert.equal(res._getData(),'Hello World')
const headers = res._getHeaders()
assert.isOk(headers)
Expand Down Expand Up @@ -793,7 +807,8 @@ describe('webfunc', () =>
assert.equal(headers['Access-Control-Allow-Origin'], 'http://boris.com, http://localhost:8080')
assert.equal(headers['Access-Control-Max-Age'], '1296000')
})
const result_03 = fn(req_03, res_03).then(res => {
const result_03 = fn(req_03, res_03).then(({req, res}) => {
assert.isOk(req)
assert.equal(res.statusCode, 404)
assert.equal(res._getData(), 'Endpoint \'/\' not found.')
})
Expand Down

0 comments on commit 80e6d8d

Please sign in to comment.