From d1767da26de2a7d690f6621264d3af90164a714e Mon Sep 17 00:00:00 2001 From: Naomi Seyfer Date: Mon, 2 Dec 2013 14:18:53 -0800 Subject: [PATCH 01/14] Short socket timeout while no pending request. Long timeout with pending req --- packages/livedata/stream_server.js | 7 ++++ packages/webapp/webapp_server.js | 51 +++++++++++++++--------------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/packages/livedata/stream_server.js b/packages/livedata/stream_server.js index c675cb9e57..8e90ae49fb 100644 --- a/packages/livedata/stream_server.js +++ b/packages/livedata/stream_server.js @@ -46,7 +46,14 @@ StreamServer = function () { if (!Package.webapp) { throw new Error("Cannot create a DDP server without the webapp package"); } + // Install the sockjs handlers, but we want to keep around our own particular + // request handler that adjusts idle timeouts while we have an outstanding + // request. This compensates for the fact that sockjs removes all listeners + // for "request" to add its own. + Package.webapp.WebApp.httpServer.removeListener('request', Package.webapp.WebApp._timeoutAdjustmentRequestCallback); self.server.installHandlers(Package.webapp.WebApp.httpServer); + Package.webapp.WebApp.httpServer.addListener('request', Package.webapp.WebApp._timeoutAdjustmentRequestCallback); + Package.webapp.WebApp.httpServer.on('closing', function () { _.each(self.open_sockets, function (socket) { socket.end(); diff --git a/packages/webapp/webapp_server.js b/packages/webapp/webapp_server.js index 1dbfb9d7ab..b4d22ce5de 100644 --- a/packages/webapp/webapp_server.js +++ b/packages/webapp/webapp_server.js @@ -12,6 +12,9 @@ var optimist = Npm.require('optimist'); var useragent = Npm.require('useragent'); var send = Npm.require('send'); +var SHORT_SOCKET_TIMEOUT = 3*1000; +var LONG_SOCKET_TIMEOUT = 120*1000; + WebApp = {}; WebAppInternals = {}; @@ -196,6 +199,23 @@ Meteor.startup(function () { }); + +// When we have a request pending, we want the socket timeout to be long, to +// give ourselves a while to serve it, and to allow sockjs long polls to +// complete. On the other hand, we want to close idle sockets relatively +// quickly, so that we can shut down relatively promptly but cleanly, without +// cutting off anyone's response. +WebApp._timeoutAdjustmentRequestCallback = function (req, res) { + req.setTimeout(LONG_SOCKET_TIMEOUT); // this is really just req.socket.setTimeout(LONG_AMOUNT); + // Insert our new finish listener to run BEFORE the existing one which removes the response from the socket. + var finishListeners = res.listeners('finish'); + res.removeAllListeners('finish'); + res.on('finish', function () { + res.setTimeout(SHORT_SOCKET_TIMEOUT); // again, basically just res.socket.setTimeout + }); + _.each(finishListeners, function (l) { res.on('finish', l); }); +}; + var runWebAppServer = function () { var shuttingDown = false; // read the control for the client we'll be serving up @@ -417,42 +437,21 @@ var runWebAppServer = function () { var httpServer = http.createServer(app); var onListeningCallbacks = []; - var longPollingSockets = {}; - // After 5 seconds of a socket being open, assume it is a long-polling // connection that we have to keep track of to shut down when we're shutting // down the server overall. - httpServer.setTimeout(5000, Meteor.bindEnvironment(function (socket) { - if (shuttingDown) { - socket.end(); - } else { - socket._meteorLongPollingId = Random.id(); - longPollingSockets[socket._meteorLongPollingId] = socket; - // give the socket another minute to live. - var destroy = Meteor.setTimeout(function () { - delete longPollingSockets[socket._meteorLongPollingId]; - socket.removeListener('close', onClose); - socket.destroy(); - }, 60*1000); + httpServer.setTimeout(SHORT_SOCKET_TIMEOUT); - var onClose = function () { - delete longPollingSockets[socket._meteorLongPollingId]; - Meteor.clearTimeout(destroy); - }; - socket.on('close', onClose); - } - }, function (err) { - console.log(err); - })); + // Do this here, and then also in livedata/stream_server.js, because + // stream_server.js kills all the current request handlers when installing its + // own. + httpServer.on('request', WebApp._timeoutAdjustmentRequestCallback); // For now, handle SIGHUP here. Later, this should be in some centralized // Meteor shutdown code. process.on('SIGHUP', Meteor.bindEnvironment(function () { shuttingDown = true; - _.each(longPollingSockets, function (socket, id) { - socket.end(); - }); // tell others with websockets open that we plan to close this. httpServer.emit('closing'); httpServer.close( function () { From bc3b42a941e9aca1e1b627694e432ce4378ddd16 Mon Sep 17 00:00:00 2001 From: Naomi Seyfer Date: Mon, 2 Dec 2013 15:15:06 -0800 Subject: [PATCH 02/14] glasser comments --- packages/webapp/webapp_server.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/webapp/webapp_server.js b/packages/webapp/webapp_server.js index b4d22ce5de..853bec01a9 100644 --- a/packages/webapp/webapp_server.js +++ b/packages/webapp/webapp_server.js @@ -12,7 +12,7 @@ var optimist = Npm.require('optimist'); var useragent = Npm.require('useragent'); var send = Npm.require('send'); -var SHORT_SOCKET_TIMEOUT = 3*1000; +var SHORT_SOCKET_TIMEOUT = 5*1000; var LONG_SOCKET_TIMEOUT = 120*1000; WebApp = {}; @@ -206,12 +206,16 @@ Meteor.startup(function () { // quickly, so that we can shut down relatively promptly but cleanly, without // cutting off anyone's response. WebApp._timeoutAdjustmentRequestCallback = function (req, res) { - req.setTimeout(LONG_SOCKET_TIMEOUT); // this is really just req.socket.setTimeout(LONG_AMOUNT); - // Insert our new finish listener to run BEFORE the existing one which removes the response from the socket. + // this is really just req.socket.setTimeout(LONG_SOCKET_TIMEOUT); + req.setTimeout(LONG_SOCKET_TIMEOUT); + // Insert our new finish listener to run BEFORE the existing one which removes + // the response from the socket. var finishListeners = res.listeners('finish'); + // XXX Apparently in Node 0.12 this event is now called 'prefinish'. + // https://github.com/joyent/node/commit/7c9b6070 res.removeAllListeners('finish'); res.on('finish', function () { - res.setTimeout(SHORT_SOCKET_TIMEOUT); // again, basically just res.socket.setTimeout + res.setTimeout(SHORT_SOCKET_TIMEOUT); }); _.each(finishListeners, function (l) { res.on('finish', l); }); }; @@ -437,9 +441,9 @@ var runWebAppServer = function () { var httpServer = http.createServer(app); var onListeningCallbacks = []; - // After 5 seconds of a socket being open, assume it is a long-polling - // connection that we have to keep track of to shut down when we're shutting - // down the server overall. + // After 5 seconds w/o data on a socket, kill it. On the other hand, if + // there's an outstanding request, give it a higher timeout instead (to avoid + // killing long-polling requests) httpServer.setTimeout(SHORT_SOCKET_TIMEOUT); // Do this here, and then also in livedata/stream_server.js, because @@ -453,6 +457,8 @@ var runWebAppServer = function () { process.on('SIGHUP', Meteor.bindEnvironment(function () { shuttingDown = true; // tell others with websockets open that we plan to close this. + // XXX: Eventually, this should be done with a standard meteor shut-down + // logic path. httpServer.emit('closing'); httpServer.close( function () { process.exit(0); From b1cc446fc846330ba2a34c98bfba8361cec99b61 Mon Sep 17 00:00:00 2001 From: Naomi Seyfer Date: Mon, 2 Dec 2013 16:25:40 -0800 Subject: [PATCH 03/14] rename event with a meteor prefix --- packages/livedata/stream_server.js | 2 +- packages/webapp/webapp_server.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/livedata/stream_server.js b/packages/livedata/stream_server.js index 8e90ae49fb..436d891dd3 100644 --- a/packages/livedata/stream_server.js +++ b/packages/livedata/stream_server.js @@ -54,7 +54,7 @@ StreamServer = function () { self.server.installHandlers(Package.webapp.WebApp.httpServer); Package.webapp.WebApp.httpServer.addListener('request', Package.webapp.WebApp._timeoutAdjustmentRequestCallback); - Package.webapp.WebApp.httpServer.on('closing', function () { + Package.webapp.WebApp.httpServer.on('meteor-closing', function () { _.each(self.open_sockets, function (socket) { socket.end(); }); diff --git a/packages/webapp/webapp_server.js b/packages/webapp/webapp_server.js index 853bec01a9..22f7a2e269 100644 --- a/packages/webapp/webapp_server.js +++ b/packages/webapp/webapp_server.js @@ -459,7 +459,7 @@ var runWebAppServer = function () { // tell others with websockets open that we plan to close this. // XXX: Eventually, this should be done with a standard meteor shut-down // logic path. - httpServer.emit('closing'); + httpServer.emit('meteor-closing'); httpServer.close( function () { process.exit(0); }); From 0a4663bafd408dd4ea68633fbffd7e4a7724d8a5 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 2 Dec 2013 16:49:09 -0800 Subject: [PATCH 04/14] Upgrade websocket-driver from 0.3.0 to 0.3.1 This fixes a DoS we found: https://github.com/faye/faye-websocket-node/issues/26 --- packages/livedata/.npm/package/npm-shrinkwrap.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/livedata/.npm/package/npm-shrinkwrap.json b/packages/livedata/.npm/package/npm-shrinkwrap.json index ffe3f903e9..7e31b07e80 100644 --- a/packages/livedata/.npm/package/npm-shrinkwrap.json +++ b/packages/livedata/.npm/package/npm-shrinkwrap.json @@ -10,7 +10,7 @@ "version": "0.7.0", "dependencies": { "websocket-driver": { - "version": "0.3.0" + "version": "0.3.1" } } } From c066b90e1cd6e9fbe14b022d99988a6c7c816bbf Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 2 Dec 2013 16:59:50 -0800 Subject: [PATCH 05/14] Add comment referencing #1648 --- tools/meteor_npm.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/meteor_npm.js b/tools/meteor_npm.js index b70a6c3ca2..b959b5ec4c 100644 --- a/tools/meteor_npm.js +++ b/tools/meteor_npm.js @@ -210,6 +210,10 @@ _.extend(exports, { var installedDependencies = self._installedDependencies(packageNpmDir); // If we already have the right things installed, life is good. + // XXX this check is wrong: what if we just pulled a commit that changes + // a sub-module in npm-shrinkwrap.json? See #1648 + // But while it might be "correct" to just drop this check we should + // be careful not to make the common case of no changes too slow. if (_.isEqual(installedDependencies, npmDependencies)) return; From 8ab66ff2557a34eb0eeeef5a12e9f444ac6032bb Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 2 Dec 2013 15:57:09 -0800 Subject: [PATCH 06/14] Stop using handlebars in the bundler. The dev bundle contains a copy of the handlebars NPM module solely for creating app.html. This is separate from the NPM module used by the handlebars NPM package. On the shark branch, we no longer use the handlebars NPM module for Meteor template (it is being replaced by Spacebars), so in preparation for that, we'll remove this barely-used build-time dependency on handlebars. A subsequent commit will remove it from the dev bundle. Once the Spacebars API has fully settled (eg, it has been merged to devel), we should get rid of this ad hoc templating and replace it with Spacebars, either in webapp_server (driven entirely by program.json) or by using unipackage.load in bundler. --- tools/app.html.in | 17 ----------------- tools/bundler.js | 40 ++++++++++++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 27 deletions(-) delete mode 100644 tools/app.html.in diff --git a/tools/app.html.in b/tools/app.html.in deleted file mode 100644 index e2bc250d0f..0000000000 --- a/tools/app.html.in +++ /dev/null @@ -1,17 +0,0 @@ - - - -{{#each stylesheets}} -{{/each}} - -##RUNTIME_CONFIG## - -{{#each scripts}} -{{/each}} - -{{{head_extra}}} - - -{{{body_extra}}} - - diff --git a/tools/bundler.js b/tools/bundler.js index 9c45d1c6f4..54c54b735e 100644 --- a/tools/bundler.js +++ b/tools/bundler.js @@ -769,19 +769,39 @@ _.extend(ClientTarget.prototype, { self.css[0].setUrlToHash(".css"); }, + // XXX Instead of packaging the boilerplate in the client program, the + // template should be part of WebApp, and we should make sure that all + // information that it needs is in the manifest (ie, make sure to include head + // and body). Then it will just need to do one level of templating instead + // of two. Alternatively, use spacebars with unipackage.load here. generateHtmlBoilerplate: function () { var self = this; - var templatePath = path.join(__dirname, "app.html.in"); - var template = watch.readAndWatchFile(self.watchSet, templatePath); - - var f = require('handlebars').compile(template.toString()); - return new Buffer(f({ - scripts: _.pluck(self.js, 'url'), - stylesheets: _.pluck(self.css, 'url'), - head_extra: self.head.join('\n'), - body_extra: self.body.join('\n') - }), 'utf8'); + var html = []; + html.push('\n' + + '\n' + + '\n'); + _.each(self.css, function (css) { + html.push(' \n'); + }); + html.push('\n\n##RUNTIME_CONFIG##\n\n'); + _.each(self.js, function (js) { + html.push(' \n'); + }); + html.push('\n\n'); + html.push(self.head.join('\n')); // unescaped! + html.push('\n' + + '\n' + + '\n'); + html.push(self.body.join('\n')); // unescaped! + html.push('\n' + + '\n' + + '\n'); + return new Buffer(html.join(''), 'utf8'); }, // Output the finished target to disk From cd873c0e4771ae82266fa8336c9c6920ce00d7e0 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 2 Dec 2013 16:10:18 -0800 Subject: [PATCH 07/14] Drop handlebars from the dev bundle. --- meteor | 2 +- scripts/generate-dev-bundle.sh | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/meteor b/meteor index e9742f6c37..f9d4362686 100755 --- a/meteor +++ b/meteor @@ -1,6 +1,6 @@ #!/bin/bash -BUNDLE_VERSION=0.3.24 +BUNDLE_VERSION=0.3.25 # OS Check. Put here because here is where we download the precompiled # bundles that are arch specific. diff --git a/scripts/generate-dev-bundle.sh b/scripts/generate-dev-bundle.sh index 8d58e1b8bb..e91efa7a65 100755 --- a/scripts/generate-dev-bundle.sh +++ b/scripts/generate-dev-bundle.sh @@ -100,7 +100,6 @@ which npm cd "$DIR/lib/node_modules" npm install optimist@0.6.0 npm install semver@2.1.0 -npm install handlebars@1.0.7 npm install request@2.27.0 npm install keypress@0.2.1 npm install underscore@1.5.2 From e11228a3f8c8abf810b46406c58d22ddcf489668 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 2 Dec 2013 17:50:30 -0800 Subject: [PATCH 08/14] Clean up package dirs containing only ".build" These directories are often left around when switching from another branch; git does not delete the gitignored .build directory (but it will show up annoyingly in 'git status'). --- tools/library.js | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/tools/library.js b/tools/library.js index 3b2f9fe857..ac68100444 100644 --- a/tools/library.js +++ b/tools/library.js @@ -25,15 +25,7 @@ var Library = function (options) { // Trim down localPackageDirs to just those that actually exist (and // that are actually directories) - self.localPackageDirs = _.filter(options.localPackageDirs, function (dir) { - try { - // use stat rather than lstat since symlink to dir is OK - var stats = fs.statSync(dir); - } catch (e) { - return false; - } - return stats.isDirectory(); - }); + self.localPackageDirs = _.filter(options.localPackageDirs, isDirectory); self.overrides = {}; // package name to package directory @@ -95,6 +87,9 @@ _.extend(Library.prototype, { // called from Package initialization code. Intended primarily for comparison // to the packageDirForBuildInfo field on a Package object; also used // internally to implement 'get'. + // + // If it finds a directory named name inside one of the localPackageDirs which + // contains nothing but ".build", it deletes that directory. findPackageDirectory: function (name) { var self = this; @@ -110,6 +105,9 @@ _.extend(Library.prototype, { for (var i = 0; i < self.localPackageDirs.length; ++i) { var packageDir = path.join(self.localPackageDirs[i], name); + if (!isDirectory(packageDir)) + continue; + // A directory is a package if it either contains 'package.js' (a package // source tree) or 'unipackage.json' (a compiled unipackage). (Actually, // for now, unipackages contain a dummy package.js too.) @@ -129,6 +127,13 @@ _.extend(Library.prototype, { fs.existsSync(path.join(packageDir, 'unipackage.json'))) { return packageDir; } + + // Does this package directory just contain a ".build" subdirectory and + // nothing else? Most likely, this package was created on another branch + // of meteor, and when you checked this branch out it left around the + // gitignored .build directory. Clean it up. + if (_.isEqual(fs.readdirSync(packageDir), ['.build'])) + files.rm_recursive(packageDir); } // Try the Meteor distribution, if we have one. @@ -464,3 +469,13 @@ _.extend(exports, { return out; } }); + +var isDirectory = function (dir) { + try { + // use stat rather than lstat since symlink to dir is OK + var stats = fs.statSync(dir); + } catch (e) { + return false; + } + return stats.isDirectory(); +}; From 4ea191e452bfc104d5a9805a622bf37ac9a2c405 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 2 Dec 2013 19:12:09 -0800 Subject: [PATCH 09/14] Drop uglify dependency from handlebars module This reduces the npm download done when updating the handlebars package by 2M. (Uglify is only used by bin/handlebars, not the handlebars API.) --- packages/handlebars/.npm/package/npm-shrinkwrap.json | 5 +---- packages/handlebars/package.js | 6 +++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/handlebars/.npm/package/npm-shrinkwrap.json b/packages/handlebars/.npm/package/npm-shrinkwrap.json index c786edadd8..6f6d7d42d0 100644 --- a/packages/handlebars/.npm/package/npm-shrinkwrap.json +++ b/packages/handlebars/.npm/package/npm-shrinkwrap.json @@ -1,7 +1,7 @@ { "dependencies": { "handlebars": { - "version": "1.0.7", + "from": "https://github.com/meteor/handlebars.js/tarball/543ec6689cf663cfda2d8f26c3c27de40aad7bd5", "dependencies": { "optimist": { "version": "0.3.7", @@ -10,9 +10,6 @@ "version": "0.0.2" } } - }, - "uglify-js": { - "version": "1.2.6" } } } diff --git a/packages/handlebars/package.js b/packages/handlebars/package.js index dfbecc219f..3fb3fdc032 100644 --- a/packages/handlebars/package.js +++ b/packages/handlebars/package.js @@ -3,7 +3,11 @@ Package.describe({ internal: true }); -Npm.depends({handlebars: '1.0.7'}); +Npm.depends({ + // Fork of 1.0.7 dropping a used-only-by-bin/handlebars dependency on the very + // large uglify-js 1.2.6. + handlebars: 'https://github.com/meteor/handlebars.js/tarball/543ec6689cf663cfda2d8f26c3c27de40aad7bd5' +}); Package.on_use(function (api) { api.use('underscore'); From df95f1e91473b95c017145c71eb67fd033e3ae5f Mon Sep 17 00:00:00 2001 From: Slava Kim Date: Tue, 3 Dec 2013 13:39:50 -0800 Subject: [PATCH 10/14] Properly handle projections where '_id' is the only rule. + Tests. Fixes #1651 --- packages/minimongo/minimongo.js | 9 ++++++++- packages/minimongo/minimongo_tests.js | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 2ddb61b84d..46ce091b15 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -1099,7 +1099,14 @@ LocalCollection._compileProjection = function (fields) { // Find the non-_id keys (_id is handled specially because it is included unless // explicitly excluded). Sort the keys, so that our code to detect overlaps // like 'foo' and 'foo.bar' can assume that 'foo' comes first. - var fieldsKeys = _.reject(_.keys(fields).sort(), function (key) { return key === '_id'; }); + var fieldsKeys = _.keys(fields).sort(); + + // If there are other rules other than '_id', treat '_id' differently in a + // separate case. If '_id' is the only rule, use it to understand if it is + // including/excluding projection. + if (fieldsKeys.length > 0 && !(fieldsKeys.length === 1 && fieldsKeys[0] === '_id')) + fieldsKeys = _.reject(fieldsKeys, function (key) { return key === '_id'; }); + var including = null; // Unknown var projectionRulesTree = {}; // Tree represented as nested objects diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 3a2d2260eb..a8948e0983 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -992,6 +992,30 @@ Tinytest.add("minimongo - projection_compiler", function (test) { "blacklist nested - path not found in doc"] ]); + testProjection({ _id: 1 }, [ + [{ _id: 42, x: 1, y: { z: "2" } }, + { _id: 42 }, + "_id whitelisted"], + [{ _id: 33 }, + { _id: 33 }, + "_id whitelisted, _id only"], + [{ x: 1 }, + {}, + "_id whitelisted, no _id"] + ]); + + testProjection({ _id: 0 }, [ + [{ _id: 42, x: 1, y: { z: "2" } }, + { x: 1, y: { z: "2" } }, + "_id blacklisted"], + [{ _id: 33 }, + {}, + "_id blacklisted, _id only"], + [{ x: 1 }, + { x: 1 }, + "_id blacklisted, no _id"] + ]); + test.throws(function () { testProjection({ 'inc': 1, 'excl': 0 }, [ [ { inc: 42, excl: 42 }, { inc: 42 }, "Can't combine incl/excl rules" ] From 6582a8860815a10af79cb888be96b58792f4c5e4 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 3 Dec 2013 11:52:16 -0800 Subject: [PATCH 11/14] Fix 0.6.6 regression in setting MAIL_URL Also: - ensure that AppConfig callbacks are always called at least once - don't start a new AppConfig Fiber every time you call Email.send Fixes #1649. --- packages/application-configuration/config.js | 14 +++---- packages/email/email.js | 42 +++++++++++--------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/packages/application-configuration/config.js b/packages/application-configuration/config.js index f7054ab3ac..75ee285d8b 100644 --- a/packages/application-configuration/config.js +++ b/packages/application-configuration/config.js @@ -58,9 +58,6 @@ try { packages: { 'mongo-livedata': { url: process.env.MONGO_URL - }, - 'email': { - url: process.env.MAIL_URL } } }; @@ -83,10 +80,13 @@ AppConfig.getAppConfig = function () { AppConfig.configurePackage = function (packageName, configure) { var appConfig = AppConfig.getAppConfig(); // Will either be based in the env var, // or wait for galaxy to connect. - var lastConfig = appConfig && appConfig.packages && appConfig.packages[packageName]; - if (lastConfig) { - configure(lastConfig); - } + var lastConfig = + (appConfig && appConfig.packages && appConfig.packages[packageName]) || {}; + // Always call the configure callback "soon" even if the initial configuration + // is empty (synchronously, though deferred would be OK). + // XXX make sure that all callers of configurePackage deal well with multiple + // callback invocations! eg, email does not + configure(lastConfig); var configureIfDifferent = function (app) { if (!EJSON.equals(app.config && app.config.packages && app.config.packages[packageName], lastConfig)) { diff --git a/packages/email/email.js b/packages/email/email.js index 6490f4ea65..8fce809930 100644 --- a/packages/email/email.js +++ b/packages/email/email.js @@ -33,19 +33,26 @@ var makePool = function (mailUrlString) { // We construct smtpPool at the first call to Email.send, so that // Meteor.startup code can set $MAIL_URL. -var smtpPool = null; -var maybeMakePool = function () { - // We check MAIL_URL in case someone else set it in Meteor.startup code. - var poolFuture = new Future(); - AppConfig.configurePackage('email', function (config) { - // TODO: allow reconfiguration. - if (!smtpPool && (config.url || process.env.MAIL_URL)) { - smtpPool = makePool(config.url || process.env.MAIL_URL); - } - poolFuture.return(); - }); +var smtpPoolFuture = new Future;; +var configured = false; - poolFuture.wait(); +var getPool = function () { + // We check MAIL_URL in case someone else set it in Meteor.startup code. + if (!configured) { + configured = true; + AppConfig.configurePackage('email', function (config) { + // XXX allow reconfiguration when the app config changes + if (smtpPoolFuture.isResolved()) + return; + var url = config.url || process.env.MAIL_URL; + var pool = null; + if (url) + pool = makePool(url); + smtpPoolFuture.return(pool); + }); + } + + return smtpPoolFuture.wait(); }; var next_devmode_mail_id = 0; @@ -81,8 +88,8 @@ var devModeSend = function (mc) { future.wait(); }; -var smtpSend = function (mc) { - smtpPool._future_wrapped_sendMail(mc).wait(); +var smtpSend = function (pool, mc) { + pool._future_wrapped_sendMail(mc).wait(); }; /** @@ -141,10 +148,9 @@ Email.send = function (options) { mc.addHeader(name, value); }); - maybeMakePool(); - - if (smtpPool) { - smtpSend(mc); + var pool = getPool(); + if (pool) { + smtpSend(pool, mc); } else { devModeSend(mc); } From 18704dccaaf233392772f2a2c7575b1cea2f57d8 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 4 Dec 2013 10:40:25 -0800 Subject: [PATCH 12/14] Note in the docs that BrowserPolicy can only be used in server code --- docs/client/packages/browser-policy.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/client/packages/browser-policy.html b/docs/client/packages/browser-policy.html index d89437e46e..cc46e01f3a 100644 --- a/docs/client/packages/browser-policy.html +++ b/docs/client/packages/browser-policy.html @@ -40,8 +40,8 @@ cross-site scripting attacks by disabling all scripts except those loaded from a `script src` attribute. Meteor determines the browser policy when the server starts up, so you should -call `BrowserPolicy` functions in top-level application code or in -`Meteor.startup`. +call `BrowserPolicy` functions on the server in top-level application code or in +`Meteor.startup`. `BrowserPolicy` functions cannot be used in client code. #### Frame options From a3abdfe8f9faed12536f6a02286a9be48056a9ff Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 4 Dec 2013 21:27:06 -0800 Subject: [PATCH 13/14] Get rid of last vestiges of options for makeElectorTries --- packages/follower-livedata/follower.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/follower-livedata/follower.js b/packages/follower-livedata/follower.js index f457ab5734..39799b491f 100644 --- a/packages/follower-livedata/follower.js +++ b/packages/follower-livedata/follower.js @@ -30,7 +30,6 @@ Follower = { }, options); // start each elector as untried/assumed connectable. - // for options.priority, low-priority things are tried first. var makeElectorTries = function (urlSet) { electorTries = {}; @@ -197,7 +196,7 @@ Follower = { if (!intervalHandle) intervalHandle = monitorConnection(); if (arguments[0] && arguments[0].url) { - makeElectorTries(arguments[0].url, {reset: true}); + makeElectorTries(arguments[0].url); tryElector(); } else { prevReconnect.apply(conn, arguments); From 43648de7e54d557ad032f3f4eb4ec3dbf1c4bc93 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 4 Dec 2013 22:05:28 -0800 Subject: [PATCH 14/14] Use array of args when `apply`ing reconnect --- packages/follower-livedata/follower.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/follower-livedata/follower.js b/packages/follower-livedata/follower.js index 39799b491f..0e4737522a 100644 --- a/packages/follower-livedata/follower.js +++ b/packages/follower-livedata/follower.js @@ -100,9 +100,9 @@ Follower = { } if (conn) { - prevReconnect.apply(conn, { + prevReconnect.apply(conn, [{ url: url - }); + }]); } else { conn = DDP.connect(url); prevReconnect = conn.reconnect;