From fd9e1c72f31e7b3c5249cb8ff3f1bd56135a9f72 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Wed, 20 Jul 2016 10:07:13 -0400 Subject: [PATCH 1/3] Use express-layer matching for batch routing, - Extract request logger for later use --- src/PromiseRouter.js | 122 ++++++++++++----------------------------- src/SensitiveLogger.js | 55 +++++++++++++++++++ 2 files changed, 90 insertions(+), 87 deletions(-) create mode 100644 src/SensitiveLogger.js diff --git a/src/PromiseRouter.js b/src/PromiseRouter.js index 1252b22747..a9987e61fd 100644 --- a/src/PromiseRouter.js +++ b/src/PromiseRouter.js @@ -10,6 +10,26 @@ import express from 'express'; import url from 'url'; import log from './logger'; import {inspect} from 'util'; +import { + logRequest, + logResponse +} from './SensitiveLogger'; + +const Layer = require('express/lib/router/layer'); + +function validateParameter(key, value) { + if (key == 'className') { + if (value.match(/_?[A-Za-z][A-Za-z_0-9]*/)) { + return value; + } + }else if (key == 'objectId') { + if (value.match(/[A-Za-z0-9]+/)) { + return value; + } + } else { + return value; + } +} export default class PromiseRouter { // Each entry should be an object with: @@ -65,7 +85,8 @@ export default class PromiseRouter { this.routes.push({ path: path, method: method, - handler: handler + handler: handler, + layer: new Layer(path, null, handler) }); }; @@ -78,30 +99,16 @@ export default class PromiseRouter { if (route.method != method) { continue; } - // NOTE: we can only route the specific wildcards :className and - // :objectId, and in that order. - // This is pretty hacky but I don't want to rebuild the entire - // express route matcher. Maybe there's a way to reuse its logic. - var pattern = '^' + route.path + '$'; - - pattern = pattern.replace(':className', - '(_?[A-Za-z][A-Za-z_0-9]*)'); - pattern = pattern.replace(':objectId', - '([A-Za-z0-9]+)'); - var re = new RegExp(pattern); - var m = path.match(re); - if (!m) { - continue; - } - var params = {}; - if (m[1]) { - params.className = m[1]; - } - if (m[2]) { - params.objectId = m[2]; - } - return {params: params, handler: route.handler}; + let layer = route.layer || new Layer(route.path, null, route.handler); + let match = layer.match(path); + if (match) { + let params = layer.params; + Object.keys(params).forEach((key) => { + params[key] = validateParameter(key, params[key]); + }); + return {params: params, handler: route.handler}; + } } }; @@ -129,24 +136,7 @@ export default class PromiseRouter { expressApp() { var expressApp = express(); - for (var route of this.routes) { - switch(route.method) { - case 'POST': - expressApp.post(route.path, makeExpressHandler(this.appId, route.handler)); - break; - case 'GET': - expressApp.get(route.path, makeExpressHandler(this.appId, route.handler)); - break; - case 'PUT': - expressApp.put(route.path, makeExpressHandler(this.appId, route.handler)); - break; - case 'DELETE': - expressApp.delete(route.path, makeExpressHandler(this.appId, route.handler)); - break; - default: - throw 'unexpected code branch'; - } - } + this.mountOnto(expressApp); return expressApp; } } @@ -159,26 +149,15 @@ function makeExpressHandler(appId, promiseHandler) { let config = AppCache.get(appId); return function(req, res, next) { try { - let url = maskSensitiveUrl(req); - let body = maskSensitiveBody(req); - let stringifiedBody = JSON.stringify(body, null, 2); - log.verbose(`REQUEST for [${req.method}] ${url}: ${stringifiedBody}`, { - method: req.method, - url: url, - headers: req.headers, - body: body - }); + logRequest(req.originalUrl, req.method, req.body, req.headers); + promiseHandler(req).then((result) => { if (!result.response && !result.location && !result.text) { log.error('the handler did not include a "response" or a "location" field'); throw 'control should not get here'; } - let stringifiedResponse = JSON.stringify(result, null, 2); - log.verbose( - `RESPONSE from [${req.method}] ${url}: ${stringifiedResponse}`, - {result: result} - ); + logResponse(req.originalUrl, req.method, result); var status = result.status || 200; res.status(status); @@ -214,34 +193,3 @@ function makeExpressHandler(appId, promiseHandler) { } } } - -function maskSensitiveBody(req) { - let maskBody = Object.assign({}, req.body); - let shouldMaskBody = (req.method === 'POST' && req.originalUrl.endsWith('/users') - && !req.originalUrl.includes('classes')) || - (req.method === 'PUT' && /users\/\w+$/.test(req.originalUrl) - && !req.originalUrl.includes('classes')) || - (req.originalUrl.includes('classes/_User')); - if (shouldMaskBody) { - for (let key of Object.keys(maskBody)) { - if (key == 'password') { - maskBody[key] = '********'; - break; - } - } - } - return maskBody; -} - -function maskSensitiveUrl(req) { - let maskUrl = req.originalUrl.toString(); - let shouldMaskUrl = req.method === 'GET' && req.originalUrl.includes('/login') - && !req.originalUrl.includes('classes'); - if (shouldMaskUrl) { - let password = url.parse(req.originalUrl, true).query.password; - if (password) { - maskUrl = maskUrl.replace('password=' + password, 'password=********') - } - } - return maskUrl; -} diff --git a/src/SensitiveLogger.js b/src/SensitiveLogger.js new file mode 100644 index 0000000000..469e7441e5 --- /dev/null +++ b/src/SensitiveLogger.js @@ -0,0 +1,55 @@ +import log from './logger'; +const URL = require('url'); + +function maskSensitiveBody(url, method, body) { + let maskBody = Object.assign({}, body); + let shouldMaskBody = (method === 'POST' && url.endsWith('/users') + && !url.includes('classes')) || + (method === 'PUT' && /users\/\w+$/.test(url) + && !url.includes('classes')) || + (url.includes('classes/_User')); + if (shouldMaskBody) { + for (let key of Object.keys(maskBody)) { + if (key == 'password') { + maskBody[key] = '********'; + break; + } + } + } + return maskBody; +} + +function maskSensitiveUrl(url, method, body) { + let maskUrl = url.toString(); + let shouldMaskUrl = method === 'GET' && url.includes('/login') + && !url.includes('classes'); + if (shouldMaskUrl) { + let password = URL.parse(url, true).query.password; + if (password) { + maskUrl = maskUrl.replace('password=' + password, 'password=********') + } + } + return maskUrl; +} + +export function logRequest(url, method, body, headers) { + url = maskSensitiveUrl(url, method, body); + body = maskSensitiveBody(url, method, body); + let stringifiedBody = JSON.stringify(body, null, 2); + log.verbose(`REQUEST for [${method}] ${url}: ${stringifiedBody}`, { + method: method, + url: url, + headers: headers, + body: body + }); +} + +export function logResponse(url, method, body) { + url = maskSensitiveUrl(url, method, body); + body = maskSensitiveBody(url, method, body); + let stringifiedResponse = JSON.stringify(body, null, 2); + log.verbose( + `RESPONSE from [${method}] ${url}: ${stringifiedResponse}`, + {result: body} + ); +} From e6f10cc7b5bd6d08480b15cb656d710ec205a270 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Wed, 20 Jul 2016 10:14:14 -0400 Subject: [PATCH 2/3] Refactors batch router --- src/PromiseRouter.js | 13 +++++++++++++ src/batch.js | 25 ++++++------------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/PromiseRouter.js b/src/PromiseRouter.js index a9987e61fd..bfd0acf82a 100644 --- a/src/PromiseRouter.js +++ b/src/PromiseRouter.js @@ -139,6 +139,19 @@ export default class PromiseRouter { this.mountOnto(expressApp); return expressApp; } + + tryRouteRequest(method, path, request) { + var match = this.match(method, path); + if (!match) { + throw new Parse.Error( + Parse.Error.INVALID_JSON, + 'cannot route ' + method + ' ' + path); + } + request.params = match.params; + return new Promise((resolve, reject) => { + match.handler(request).then(resolve, reject); + }); + } } // A helper function to make an express handler out of a a promise diff --git a/src/batch.js b/src/batch.js index bf61b9e999..0f2d1da5be 100644 --- a/src/batch.js +++ b/src/batch.js @@ -29,8 +29,7 @@ function handleBatch(router, req) { var apiPrefixLength = req.originalUrl.length - batchPath.length; var apiPrefix = req.originalUrl.slice(0, apiPrefixLength); - var promises = []; - for (var restRequest of req.body.requests) { + let promises = req.body.requests.map((restRequest) => { // The routablePath is the path minus the api prefix if (restRequest.path.slice(0, apiPrefixLength) != apiPrefix) { throw new Parse.Error( @@ -38,30 +37,18 @@ function handleBatch(router, req) { 'cannot route batch path ' + restRequest.path); } var routablePath = restRequest.path.slice(apiPrefixLength); - - // Use the router to figure out what handler to use - var match = router.match(restRequest.method, routablePath); - if (!match) { - throw new Parse.Error( - Parse.Error.INVALID_JSON, - 'cannot route ' + restRequest.method + ' ' + routablePath); - } - - // Construct a request that we can send to a handler - var request = { + let request = { body: restRequest.body, - params: match.params, config: req.config, auth: req.auth, info: req.info - }; - - promises.push(match.handler(request).then((response) => { + } + return router.tryRouteRequest(restRequest.method, routablePath, request).then((response) => { return {success: response.response}; }, (error) => { return {error: {code: error.code, error: error.message}}; - })); - } + }); + }); return Promise.all(promises).then((results) => { return {response: results}; From b4426f97c241848124d3318e1a5687ea7931645a Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Wed, 20 Jul 2016 11:55:24 -0400 Subject: [PATCH 3/3] Lowercase sensitive --- src/PromiseRouter.js | 2 +- src/{SensitiveLogger.js => sensitiveLogger.js} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/{SensitiveLogger.js => sensitiveLogger.js} (100%) diff --git a/src/PromiseRouter.js b/src/PromiseRouter.js index bfd0acf82a..e8f236003f 100644 --- a/src/PromiseRouter.js +++ b/src/PromiseRouter.js @@ -13,7 +13,7 @@ import {inspect} from 'util'; import { logRequest, logResponse -} from './SensitiveLogger'; +} from './sensitiveLogger'; const Layer = require('express/lib/router/layer'); diff --git a/src/SensitiveLogger.js b/src/sensitiveLogger.js similarity index 100% rename from src/SensitiveLogger.js rename to src/sensitiveLogger.js