Arlolra has uploaded a new change for review. https://gerrit.wikimedia.org/r/201090
Change subject: Separate request and timeout ids ...................................................................... Separate request and timeout ids * Closes the DOS vector described at, https://gerrit.wikimedia.org/r/#/c/200829/ * Also, passes the request id to bunyan for loggin. Change-Id: I6239686588659f500a6a6a6dc702d742f2ef6cea --- M api/ParsoidService.js M api/routes.js M api/server.js M lib/ParsoidLogger.js M lib/mediawiki.parser.environment.js 5 files changed, 24 insertions(+), 21 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid refs/changes/90/201090/1 diff --git a/api/ParsoidService.js b/api/ParsoidService.js index 1138291..967eead 100644 --- a/api/ParsoidService.js +++ b/api/ParsoidService.js @@ -48,15 +48,11 @@ // limit upload file size app.use(express.limit('15mb')); - // request ids + // timeout ids, used internally to track runaway processes var buf = new Buffer(16); app.use(function(req, res, next) { - if(req.headers && req.headers['x-request-id']) { - res.local('reqId', req.headers['x-request-id']); - } else { - uuid(null, buf); - res.local('reqId', buf.toString('hex')); - } + uuid(null, buf); + res.local('timeoutId', buf.toString('hex')); next(); }); diff --git a/api/routes.js b/api/routes.js index 3ff6e73..7533bb7 100644 --- a/api/routes.js +++ b/api/routes.js @@ -61,11 +61,11 @@ } var CPU_TIMEOUT = 5 * 60 * 1000; // 5 minutes -var makeDone = function( reqId ) { +var makeDone = function( timeoutId ) { // Create this function in an outer scope so that we don't inadvertently // keep a reference to the promise here. return function() { - process.send({ type: "timeout", done: true, reqId: reqId }); + process.send({ type: "timeout", done: true, timeoutId: timeoutId }); }; }; @@ -73,7 +73,7 @@ var sufficientNodeVersion = !/^v0\.[0-8]\./.test( process.version ); var cpuTimeout = function( p, res ) { - var reqId = res.local("reqId"); + var timeoutId = res.local("timeoutId"); var location = util.format( "[%s/%s%s]", res.local("iwp"), res.local("pageName"), (res.local("oldid") ? "?oldid=" + res.local("oldid") : "") @@ -87,10 +87,10 @@ process.send({ type: "timeout", timeout: CPU_TIMEOUT, - reqId: reqId, + timeoutId: timeoutId, location: location }); - var done = makeDone( reqId ); + var done = makeDone( timeoutId ); p.then( done, done ); p.then( resolve, reject ); }); @@ -551,11 +551,13 @@ } return Promise.resolve().nodify(callback); } - MWParserEnv.getParserEnv(parsoidConfig, null, { + var options = { prefix: res.local('iwp'), pageName: res.local('pageName'), - cookie: req.headers.cookie - }).then(function( env ) { + cookie: req.headers.cookie, + reqId: req.headers['x-request-id'] + }; + MWParserEnv.getParserEnv(parsoidConfig, null, options).then(function( env ) { env.logger.registerBackend(/fatal(\/.*)?/, errBack.bind(this, env)); if ( res.local('v2') && res.local('v2').format === "pagebundle" ) { env.storeDataParsoid = true; diff --git a/api/server.js b/api/server.js index 247d87b..263350d 100755 --- a/api/server.js +++ b/api/server.js @@ -116,12 +116,12 @@ } if ( msg.type !== "timeout" ) { return; } if ( msg.done ) { - clearTimeout( timeouts.get( msg.reqId ) ); - timeouts.delete( msg.reqId ); + clearTimeout( timeouts.get( msg.timeoutId ) ); + timeouts.delete( msg.timeoutId ); } else if ( msg.timeout ) { var pid = worker.process.pid; - timeouts.set(msg.reqId, setTimeout(function() { - timeouts.delete( msg.reqId ); + timeouts.set(msg.timeoutId, setTimeout(function() { + timeouts.delete( msg.timeoutId ); if ( worker.id in cluster.workers ) { logger.log( "warning", util.format( "Cpu timeout fetching: %s; killing worker %s.", diff --git a/lib/ParsoidLogger.js b/lib/ParsoidLogger.js index e835e58..b3ce1a5 100644 --- a/lib/ParsoidLogger.js +++ b/lib/ParsoidLogger.js @@ -6,11 +6,12 @@ coreutil = require('util'); -function LocationData( wiki, title, meta ) { +function LocationData( wiki, title, meta, reqId ) { this.wiki = wiki; this.title = title; this.oldId = ( meta && meta.revision && meta.revision.revid ) ? meta.revision.revid : null; + this.reqId = reqId || null; } LocationData.prototype.toString = function() { return coreutil.format( @@ -103,7 +104,8 @@ return new LocationData( this.env.conf.wiki.iwp, this.env.page.name, - this.env.page.meta + this.env.page.meta, + this.env.reqId ); }; diff --git a/lib/mediawiki.parser.environment.js b/lib/mediawiki.parser.environment.js index ccd9a26..f59e5d7 100644 --- a/lib/mediawiki.parser.environment.js +++ b/lib/mediawiki.parser.environment.js @@ -39,6 +39,9 @@ // A passed-in cookie, if any this.cookie = options.cookie || null; + // A passed-in request id, if any + this.reqId = options.reqId || null; + // Configuration this.conf = {}; -- To view, visit https://gerrit.wikimedia.org/r/201090 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6239686588659f500a6a6a6dc702d742f2ef6cea Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/services/parsoid Gerrit-Branch: master Gerrit-Owner: Arlolra <abrea...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
