diff --git a/api/api.js b/api/api.js index dd11a6c23f0..2b250e1e94b 100644 --- a/api/api.js +++ b/api/api.js @@ -18,10 +18,15 @@ const versionInfo = require('../frontend/express/version.info.js'); const moment = require("moment"); const tracker = require('./parts/mgmt/tracker.js'); const { RateLimiterMemory } = require("rate-limiter-flexible"); +const uploadTemp = require('./utils/upload-temp.js'); var t = ["countly:", "api"]; common.processRequest = processRequest; +// Core endpoints that read params.files. Plugins declare their own. +plugins.uploadPaths.push({path: "/i/apps/create"}); +plugins.uploadPaths.push({path: "/i/apps/update"}); + if (cluster.isMaster) { console.log("Starting Countly", "version", versionInfo.version, "package", pack.version); if (!common.checkDatabaseConfigMatch(countlyConfig.mongodb, frontendConfig.mongodb)) { @@ -482,6 +487,27 @@ function handleRequest(req, res) { if (countlyConfig.api.maxUploadFileSize) { formidableOptions.maxFileSize = countlyConfig.api.maxUploadFileSize; } + // The body is parsed before the request is routed or authorized, so + // uploads are refused unless a plugin declared that this exact path + // reads params.files + Object.assign(formidableOptions, uploadTemp.parseOptions(req.url, common.config && common.config.path, plugins.uploadPaths)); + + // Whatever did get written is removed once the response is done with. + // 'close' matters as well as 'finish': an upload aborted mid-stream + // leaves a partial file that 'finish' never sees. Handlers that consume + // an upload unlink it themselves; this only catches what they did not, + // which is every request rejected before reaching a handler at all. + /** + * Remove any upload temp file the handlers did not consume + * @returns {void} + */ + const discardOnce = () => { + res.removeListener('finish', discardOnce); + res.removeListener('close', discardOnce); + uploadTemp.discardUploads(params); + }; + res.on('finish', discardOnce); + res.on('close', discardOnce); const form = new formidable.IncomingForm(formidableOptions); if (/crash_symbols\/(add_symbol|upload_symbol)/.test(req.url)) { @@ -517,6 +543,9 @@ function handleRequest(req, res) { } } params.files = files; + //snapshot the paths formidable produced, before a handler can + //repoint params.files[x].path at something that must not be removed + uploadTemp.trackUploads(params, files); if (multiFormData) { let formDataUrl = []; for (const i in fields) { diff --git a/api/parts/mgmt/apps.js b/api/parts/mgmt/apps.js index 04a2be31992..783387860fb 100644 --- a/api/parts/mgmt/apps.js +++ b/api/parts/mgmt/apps.js @@ -320,8 +320,15 @@ appsApi.createApp = async function(params) { appId: app.ops[0]._id, data: newApp }); - iconUpload(Object.assign({}, params, {app_id: app.ops[0]._id})); - common.returnOutput(params, newApp); + //respond only once the icon has been processed, so the uploaded + //temp file is not removed while jimp is still reading it + iconUpload(Object.assign({}, params, {app_id: app.ops[0]._id})) + .catch(function() { + //iconUpload logs its own failures, the app was created either way + }) + .then(function() { + common.returnOutput(params, newApp); + }); } else { common.returnMessage(params, 500, "Error creating App: " + err); @@ -497,8 +504,15 @@ appsApi.updateApp = function(params) { update: updatedApp } }); - iconUpload(params); - common.returnOutput(params, updatedApp); + //respond only once the icon has been processed, so the + //uploaded temp file is not removed while jimp is reading it + iconUpload(params) + .catch(function() { + //iconUpload logs its own failures, the app was updated either way + }) + .then(function() { + common.returnOutput(params, updatedApp); + }); }); } else { diff --git a/api/utils/upload-temp.js b/api/utils/upload-temp.js new file mode 100644 index 00000000000..44e7f8ffefe --- /dev/null +++ b/api/utils/upload-temp.js @@ -0,0 +1,158 @@ +/** + * @module api/utils/upload-temp + * @description Bounds what the formidable parse in api/api.js is allowed to + * write to disk, and removes whatever it wrote that no handler consumed. + * + * api/api.js parses every POST body with formidable, before the request is + * routed or authorized. formidable writes multipart parts and raw + * application/octet-stream bodies to disk, so without a gate any POST carrying + * such a body leaves a file behind - including requests to paths that no + * handler serves. This module provides: + * + * - parseOptions: formidable options that allow a file to be written only + * where a plugin declared that the path reads params.files + * - discardUploads: removal of the files a request left behind + */ + +'use strict'; + +const fs = require('fs'); + +/** + * Strip the query string and the installation subpath from a request url. + * requestProcessor strips the subpath before routing, so it has to be stripped + * here too or uploads would be refused on subdirectory installs. + * @param {string} url - request url, query string included + * @param {string} [installPath] - installation subpath (common.config.path) + * @returns {string} the path as requestProcessor will see it + */ +function normalizePath(url, installPath) { + let reqPath = (url || '').split('?')[0]; + + if (installPath && installPath !== '/' + && (reqPath === installPath || reqPath.indexOf(installPath + '/') === 0)) { + reqPath = reqPath.substring(installPath.length) || '/'; + } + + //so that /i and /i/ are the same path + if (reqPath.length > 1 && reqPath.charAt(reqPath.length - 1) === '/') { + reqPath = reqPath.substring(0, reqPath.length - 1); + } + + return reqPath; +} + +/** + * Find the declaration a plugin registered for this path, if any. + * + * Matching is exact: a declaration for /i must not let /i/anything through, + * otherwise declaring the SDK write endpoint would reopen the whole /i tree. + * @param {Array} uploadPaths - plugins.uploadPaths + * @param {string} reqPath - normalized request path + * @returns {object|null} the declaration, or null when uploads are not allowed + */ +function findDeclaration(uploadPaths, reqPath) { + if (!Array.isArray(uploadPaths) || !reqPath) { + return null; + } + + for (let i = 0; i < uploadPaths.length; i++) { + const entry = uploadPaths[i]; + const declared = (typeof entry === 'string') ? entry : (entry && entry.path); + if (declared && declared === reqPath) { + return (typeof entry === 'string') ? {path: entry} : entry; + } + } + + return null; +} + +/** + * formidable options restricting what this request may write to disk. + * + * Returns the options to merge into the IncomingForm options: an empty object + * where uploads are expected, and otherwise the entries that suppress them. + * Both suppressions are needed and cover different parsers - `filter` is only + * consulted for multipart parts, while formidable's octetstream plugin creates + * its file directly via _newFile() and never calls `filter`, so the only way to + * stop a raw body reaching disk is to leave that plugin out. + * + * The remaining parsers are kept enabled either way, so urlencoded and json + * bodies still populate params.qstring exactly as before. + * @param {string} url - request url, query string included + * @param {string} [installPath] - installation subpath (common.config.path) + * @param {Array} [uploadPaths] - plugins.uploadPaths, the declared upload paths + * @returns {object} options to merge into the IncomingForm options + */ +function parseOptions(url, installPath, uploadPaths) { + const declaration = findDeclaration(uploadPaths, normalizePath(url, installPath)); + const options = {}; + + if (!declaration) { + options.filter = function() { + return false; + }; + } + + if (!declaration || !declaration.raw) { + options.enabledPlugins = ['querystring', 'multipart', 'json']; + } + + return options; +} + +/** + * Record the paths formidable produced for this request, so they can be removed + * later without trusting params.files. + * + * The snapshot is taken before any handler runs, because a handler may repoint + * params.files[x].path at something else entirely - crash_symbolication points + * it at files shipped with the plugin when serving populator data, which must + * never be deleted. + * @param {object} params - request params + * @param {object} files - the files formidable parsed + * @returns {void} + */ +function trackUploads(params, files) { + const tracked = []; + + Object.keys(files || {}).forEach((key) => { + const file = files[key]; + const target = file && (file.filepath || file.path); + if (target) { + tracked.push(target); + } + }); + + params.uploadTempPaths = tracked; +} + +/** + * Remove the files formidable wrote for this request. + * + * Handlers that consume an upload unlink it themselves, so in the normal case + * these are already gone and the unlink is a no-op. What is left is every + * request that was rejected before reaching a handler at all, which is not + * enumerable at the rejection sites - rights.js alone rejects in 63 places. + * + * Only the snapshot from trackUploads is touched, never params.files, so a + * handler substituting a path cannot redirect this at another file. + * @param {object} params - request params + * @returns {void} + */ +function discardUploads(params) { + if (!params || !params.uploadTempPaths || !params.uploadTempPaths.length) { + return; + } + + params.uploadTempPaths.forEach((target) => { + fs.unlink(target, function() {}); + }); + params.uploadTempPaths = []; +} + +module.exports = { + parseOptions, + trackUploads, + discardUploads +}; diff --git a/plugins/pluginManager.js b/plugins/pluginManager.js index e46a6df1a1f..a5b3c8f671b 100644 --- a/plugins/pluginManager.js +++ b/plugins/pluginManager.js @@ -79,6 +79,20 @@ var pluginManager = function pluginManager() { * @type {{collection: string, db: mongodb.Db, property: string, expireAfterSeconds: number}[]} */ this.ttlCollections = []; + /** + * Request paths that are allowed to carry a file upload. + * + * The API parses POST bodies before a request is routed or authorized, and + * formidable writes multipart parts and raw application/octet-stream bodies + * to disk. Uploads are therefore refused by default and a plugin has to + * declare the paths where it actually reads params.files, so a body sent + * anywhere else is never written to disk. Set raw when the endpoint reads an + * application/octet-stream body rather than a multipart part. + * + * Paths are matched exactly, after the installation subpath is stripped. + * @type {{path: string, raw: boolean=}[]} + */ + this.uploadPaths = []; /** * Custom configuration files for different databases for docker env */ diff --git a/plugins/star-rating/api/api.js b/plugins/star-rating/api/api.js index e350b275385..c3626c52fd4 100644 --- a/plugins/star-rating/api/api.js +++ b/plugins/star-rating/api/api.js @@ -488,6 +488,8 @@ function uploadFile(myfile, id, callback) { * "result": "Missing parameter "api_key" or "auth_token"" * } */ + //also declared by the surveys plugin, which serves this path when enabled + plugins.uploadPaths.push({path: "/i/feedback/upload"}); plugins.register("/i/feedback/upload", function(ob) { // do not respond if this isn't feedback fetch request // or surveys plugin enabled @@ -980,6 +982,7 @@ function uploadFile(myfile, id, callback) { * "result": "Missing parameter \"api_key\" or \"auth_token\""" * } */ + plugins.uploadPaths.push({path: "/i/feedback/logo"}); plugins.register("/i/feedback/logo", function(ob) { var params = ob.params; validateCreate(params, FEATURE_NAME, function() { diff --git a/plugins/star-rating/tests.js b/plugins/star-rating/tests.js index 2b6e92dcccb..5910b044545 100644 --- a/plugins/star-rating/tests.js +++ b/plugins/star-rating/tests.js @@ -738,4 +738,46 @@ describe('Testing Rating plugin', function() { }); }); }); -}); \ No newline at end of file +}); + +// Uploads are refused unless a plugin declares the request path, so a wrong +// declaration silently turns /i/feedback/upload into an endpoint that reports an +// invalid image. The control test establishes what that looks like, which is +// what makes the second assertion mean something rather than pass for any +// reason. This endpoint only serves when the surveys plugin is absent, which is +// the case in this repo. +describe('Feedback logo upload', function() { + var logo = require("path").resolve(__dirname, './../../frontend/express/public/images/favicon.png'); + + it('should reject when nothing is attached', function(done) { + API_KEY_ADMIN = testUtils.get("API_KEY_ADMIN"); + request + .post('/i/feedback/upload?api_key=' + API_KEY_ADMIN) + .end(function(err, res) { + if (err) { + return done(err); + } + // with no file there is nothing to store, and the endpoint says so + res.text.should.containEql('feedback.image'); + done(); + }); + }); + + it('should receive an attached logo', function(done) { + API_KEY_ADMIN = testUtils.get("API_KEY_ADMIN"); + request + .post('/i/feedback/upload?api_key=' + API_KEY_ADMIN) + .attach('feedback_logo', logo) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + // if /i/feedback/upload were not declared, the part would be + // dropped and this would be the invalid image response + var ob = JSON.parse(res.text); + ob.should.have.property('result', 'Success'); + done(); + }); + }); +}); diff --git a/test/3.api.write/12.app.image.upload.js b/test/3.api.write/12.app.image.upload.js new file mode 100644 index 00000000000..cf86ece4ef1 --- /dev/null +++ b/test/3.api.write/12.app.image.upload.js @@ -0,0 +1,194 @@ +var request = require('supertest'); +var should = require('should'); +var testUtils = require("../testUtils"); +var fs = require('fs'); +var path = require('path'); +request = request(testUtils.url); + +var API_KEY_ADMIN = ""; + +var DEFAULT_ICON = fs.readFileSync(path.resolve(__dirname, './../../frontend/express/public/images/default_app_icon.png')); +var ICON_A = path.resolve(__dirname, './../../frontend/express/public/images/favicon.png'); +var ICON_B = path.resolve(__dirname, './../../frontend/express/public/images/default_member_icon.png'); + +// Uploads are refused unless a plugin declares the request path, so a wrong +// declaration silently stops an endpoint from ever receiving its file. +// +// The apps endpoints cannot be checked on their response: iconUpload does +// nothing when params.files is empty, so they answer with the app either way. +// The icon is served by /appimages/.png, which also always answers 200 - +// with the stored icon, or with default_app_icon.png when there is none. So the +// check is on what gets served, with a freshly created app to give a control. + +/** + * Fetch the served icon, retrying while it is still the default. + * + * iconUpload resolves once jimp has produced the buffer, but hands that buffer + * to countlyFs.saveData without waiting for it, so the store completes after the + * response has gone out. Polling is how the test observes the result without + * production code having to change. + * @param {string} appId - the app whose icon to fetch + * @param {number} attempts - how many times to look before giving up + * @param {function} callback - called with (err, body) + * @returns {void} + */ +function fetchStoredIcon(appId, attempts, callback) { + request + .get('/appimages/' + appId + '.png') + .expect(200) + .end(function(err, res) { + if (err) { + return callback(err); + } + var body = res.body; + var stored = Buffer.isBuffer(body) && Buffer.compare(body, DEFAULT_ICON) !== 0; + if (stored || attempts <= 1) { + // hand back whatever we have; the caller asserts on it, so a + // genuinely refused upload still fails with a useful message + return callback(null, body); + } + setTimeout(function() { + fetchStoredIcon(appId, attempts - 1, callback); + }, 300); + }); +} + +describe('App icon upload on create', function() { + var createdId = null; + + it('should create an app with an attached icon', function(done) { + API_KEY_ADMIN = testUtils.get("API_KEY_ADMIN"); + request + .post('/i/apps/create?api_key=' + API_KEY_ADMIN + '&args=' + JSON.stringify({name: "Icon Upload Test App"})) + .attach('app_image', ICON_A) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('_id'); + createdId = ob._id; + done(); + }); + }); + + it('should serve the uploaded icon rather than the default', function(done) { + if (!createdId) { + return done(new Error("app was not created")); + } + fetchStoredIcon(createdId, 10, function(err, body) { + if (err) { + return done(err); + } + Buffer.isBuffer(body).should.equal(true); + body.length.should.be.above(0); + Buffer.compare(body, DEFAULT_ICON).should.not.equal(0, + "the default icon came back, so the uploaded file never reached the handler"); + done(); + }); + }); + + it('should remove the test app', function(done) { + if (!createdId) { + return done(new Error("app was not created")); + } + request + .get('/i/apps/delete?api_key=' + API_KEY_ADMIN + '&args=' + JSON.stringify({app_id: createdId})) + .expect(200) + .end(function(err) { + if (err) { + return done(err); + } + done(); + }); + }); +}); + +// The update path needs its own cover, and creating the app without an icon +// first gives a control: the default must come back before the upload, so the +// change afterwards cannot be explained by whatever the app already held. +describe('App icon upload on update', function() { + var createdId = null; + + it('should create an app with no icon', function(done) { + API_KEY_ADMIN = testUtils.get("API_KEY_ADMIN"); + request + .get('/i/apps/create?api_key=' + API_KEY_ADMIN + '&args=' + JSON.stringify({name: "Icon Update Test App"})) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('_id'); + createdId = ob._id; + done(); + }); + }); + + it('should serve the default icon before any upload', function(done) { + if (!createdId) { + return done(new Error("app was not created")); + } + request + .get('/appimages/' + createdId + '.png') + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + // the control that makes the assertion after the upload mean + // something: with nothing stored the route falls back + Buffer.compare(res.body, DEFAULT_ICON).should.equal(0); + done(); + }); + }); + + it('should accept an icon on update', function(done) { + if (!createdId) { + return done(new Error("app was not created")); + } + request + .post('/i/apps/update?api_key=' + API_KEY_ADMIN + '&args=' + JSON.stringify({app_id: createdId, name: "Icon Update Test App renamed"})) + .attach('app_image', ICON_B) + .expect(200) + .end(function(err) { + if (err) { + return done(err); + } + done(); + }); + }); + + it('should serve the uploaded icon after the update', function(done) { + if (!createdId) { + return done(new Error("app was not created")); + } + fetchStoredIcon(createdId, 10, function(err, body) { + if (err) { + return done(err); + } + Buffer.isBuffer(body).should.equal(true); + body.length.should.be.above(0); + Buffer.compare(body, DEFAULT_ICON).should.not.equal(0, + "the default icon still came back, so the uploaded file never reached the handler"); + done(); + }); + }); + + it('should remove the test app', function(done) { + if (!createdId) { + return done(new Error("app was not created")); + } + request + .get('/i/apps/delete?api_key=' + API_KEY_ADMIN + '&args=' + JSON.stringify({app_id: createdId})) + .expect(200) + .end(function(err) { + if (err) { + return done(err); + } + done(); + }); + }); +}); diff --git a/test/unit-tests/api.utils.upload-temp.js b/test/unit-tests/api.utils.upload-temp.js new file mode 100644 index 00000000000..233d89db248 --- /dev/null +++ b/test/unit-tests/api.utils.upload-temp.js @@ -0,0 +1,410 @@ +var should = require("should"); +var fs = require("fs"); +var os = require("os"); +var path = require("path"); +var uploadTemp = require("../../api/utils/upload-temp"); + +// These tests exercise pure path logic, a scan of this repo's own source, and a +// temp directory created under the OS temp directory. They touch no request +// handling and no database. +// +// The declarations are read out of the source rather than restated here. Loading +// the plugins instead would be more faithful, but a plugin api.js cannot be +// required in isolation - it pulls in common.js, the config and the database +// drivers, and would need a stub covering some 28 pluginManager members. What a +// scan cannot check is that every endpoint reading params.files has a +// declaration; only the integration suite, which boots real plugins, can. + +/** + * Every upload path this repo declares, read out of the source. + * @returns {Array} objects with the declared path and the file declaring it + */ +function declaredInSource() { + var repoRoot = path.resolve(__dirname, "..", ".."); + var files = [path.join(repoRoot, "api", "api.js")]; + var pluginsDir = path.join(repoRoot, "plugins"); + + fs.readdirSync(pluginsDir).forEach(function(name) { + var candidate = path.join(pluginsDir, name, "api", "api.js"); + if (fs.existsSync(candidate)) { + files.push(candidate); + } + }); + + var found = []; + files.forEach(function(file) { + var src = fs.readFileSync(file, "utf8"); + // matches both push("/i/x") and push({path: "/i/x", raw: true}) + var re = /uploadPaths\.push\(\s*(?:\{[^}]*?path:\s*)?["']([^"']+)["']/g; + var match = re.exec(src); + while (match !== null) { + found.push({path: match[1], file: path.relative(repoRoot, file)}); + match = re.exec(src); + } + }); + + return found; +} + +/** + * Every URL a plugin test actually attaches a file to. + * + * The window for each request stops at the next .post(, because a spec often + * uploads in one test and posts without a file in the next; a fixed size window + * attributes the upload to the wrong URL. + * @returns {object} url -> array of spec files posting to it with a file + */ +function urlsWithRealUploads() { + var repoRoot = path.resolve(__dirname, "..", ".."); + var pluginsDir = path.join(repoRoot, "plugins"); + var specs = []; + + fs.readdirSync(pluginsDir).forEach(function(name) { + var single = path.join(pluginsDir, name, "tests.js"); + if (fs.existsSync(single)) { + specs.push(single); + } + var dir = path.join(pluginsDir, name, "tests"); + if (fs.existsSync(dir)) { + fs.readdirSync(dir).forEach(function(entry) { + if (entry.endsWith(".js")) { + specs.push(path.join(dir, entry)); + } + }); + } + }); + + var out = {}; + specs.forEach(function(file) { + // commented out lines must not count: plugins/push/tests.js has a + // whole disabled upload suite that would otherwise look real + var src = fs.readFileSync(file, "utf8").replace(/^\s*\/\/.*$/gm, ""); + if (src.indexOf(".attach(") === -1) { + return; + } + var posts = []; + var re = /\.post\(\s*[`'"]([^`'"?]+)/g; + var match = re.exec(src); + while (match !== null) { + posts.push({at: match.index, after: re.lastIndex, url: match[1].replace(/\/$/, "")}); + match = re.exec(src); + } + posts.forEach(function(post, i) { + var end = (i + 1 < posts.length) ? posts[i + 1].at : src.length; + if (src.slice(post.after, end).indexOf(".attach(") !== -1) { + out[post.url] = out[post.url] || []; + out[post.url].push(path.relative(repoRoot, file)); + } + }); + }); + + return out; +} + +// A synthetic registry. These entries are deliberately not real endpoints: the +// matcher's behaviour does not depend on which paths happen to exist, and +// realistic looking data here previously read as if it were the real registry. +var FIXTURE = [ + {path: "/i"}, + {path: "/i/declared"}, + {path: "/i/nested/declared"}, + {path: "/i/raw/endpoint", raw: true}, + "/i/string/form" +]; + +describe("upload temp file handling", function() { + /** + * Whether these options allow a multipart part to be written to disk + * @param {object} opts - options returned by parseOptions + * @returns {boolean} true when multipart files are allowed + */ + function allowsMultipart(opts) { + return typeof opts.filter === "undefined"; + } + + /** + * Whether these options allow a raw octet-stream body to be written to disk + * @param {object} opts - options returned by parseOptions + * @returns {boolean} true when raw bodies are allowed + */ + function allowsRaw(opts) { + return typeof opts.enabledPlugins === "undefined"; + } + + var declared = declaredInSource(); + var registry = declared.map(function(entry) { + return entry.path; + }); + + describe("declarations in this repo", function() { + + // Update this list when an upload endpoint is added or removed. It is + // here so that a removal is a visible diff rather than an endpoint + // quietly starting to refuse the uploads it used to accept. + var EXPECTED = [ + "/i/apps/create", + "/i/apps/update", + "/i/feedback/logo", + "/i/feedback/upload" + ]; + + it("declares exactly the expected set", function() { + registry.slice().sort().should.eql(EXPECTED.slice().sort()); + }); + + it("declares only well formed API paths", function() { + declared.forEach(function(entry) { + var where = entry.path + " declared in " + entry.file; + // requestProcessor only routes /i and /o + entry.path.should.match(/^\/[io](\/|$)/, where + " must be under /i or /o"); + entry.path.should.not.match(/[?*\s]/, where + " must be a bare path"); + entry.path.should.not.match(/.\/$/, where + " must not end in a slash"); + }); + }); + + it("matches every declaration, so none of them is unreachable", function() { + // a declaration the matcher cannot match, a trailing slash say, + // would silently refuse the upload it was meant to permit + declared.forEach(function(entry) { + allowsMultipart(uploadTemp.parseOptions(entry.path, undefined, registry)) + .should.equal(true, entry.path + " in " + entry.file + " must be matched"); + }); + }); + + it("does not let a declaration open a deeper path", function() { + declared.forEach(function(entry) { + allowsMultipart(uploadTemp.parseOptions(entry.path + "/extra", undefined, registry)) + .should.equal(false, entry.path + " must not permit a deeper path"); + }); + }); + }); + + describe("upload tests and declarations agree", function() { + // Catches a declaration whose path shape is wrong - /i/surveys/create + // instead of /i/surveys/nps/create, say. Such a declaration matches + // nothing, so it refuses the upload it was meant to permit, and only a + // real request reveals it. + // + // This repo currently has no active upload test: the only .attach calls + // are commented out in plugins/push/tests.js, so /i/apps/* and + // /i/feedback/* have nothing exercising a real multipart POST. The check + // is here so that it starts guarding the moment one is added. + var uploaded = urlsWithRealUploads(); + + it("declares every endpoint a test uploads to", function() { + Object.keys(uploaded).sort().forEach(function(url) { + registry.indexOf(url).should.not.equal(-1, + url + " is uploaded to by " + uploaded[url].join(", ") + + " but is not declared, so the upload would be refused"); + }); + }); + }); + + describe("parseOptions - multipart", function() { + it("allows a declared path", function() { + allowsMultipart(uploadTemp.parseOptions("/i", undefined, FIXTURE)).should.equal(true); + allowsMultipart(uploadTemp.parseOptions("/i/declared", undefined, FIXTURE)).should.equal(true); + allowsMultipart(uploadTemp.parseOptions("/i/nested/declared", undefined, FIXTURE)).should.equal(true); + }); + + it("refuses an undeclared path under /i (deny by default)", function() { + [ + "/i/undeclared", + "/i/declared/deeper", + "/i/nested", + "/i/bulk", + "/i/apps/delete" + ].forEach(function(url) { + allowsMultipart(uploadTemp.parseOptions(url, undefined, FIXTURE)) + .should.equal(false, url + " is not declared and must be refused"); + }); + }); + + it("refuses read endpoints and paths no handler serves", function() { + [ + "/o", + "/o/export", + "/crowd/admin/uploadplugin.action", + "/", + "/admin", + "/.env", + "/wp-login.php" + ].forEach(function(url) { + allowsMultipart(uploadTemp.parseOptions(url, undefined, FIXTURE)) + .should.equal(false, url + " must be refused"); + }); + }); + + it("matches exactly, so declaring /i does not open the /i tree", function() { + allowsMultipart(uploadTemp.parseOptions("/i", undefined, FIXTURE)).should.equal(true); + allowsMultipart(uploadTemp.parseOptions("/i/", undefined, FIXTURE)).should.equal(true); + allowsMultipart(uploadTemp.parseOptions("/i/anything", undefined, FIXTURE)).should.equal(false); + }); + + it("does not match paths that merely start with i or o", function() { + allowsMultipart(uploadTemp.parseOptions("/iffy", undefined, FIXTURE)).should.equal(false); + allowsMultipart(uploadTemp.parseOptions("/index.php", undefined, FIXTURE)).should.equal(false); + allowsMultipart(uploadTemp.parseOptions("/output", undefined, FIXTURE)).should.equal(false); + }); + + it("ignores the query string", function() { + allowsMultipart(uploadTemp.parseOptions("/i?app_key=x&device_id=y", undefined, FIXTURE)).should.equal(true); + allowsMultipart(uploadTemp.parseOptions("/crowd/x?a=/i", undefined, FIXTURE)).should.equal(false); + }); + + it("honours a subdirectory installation path", function() { + allowsMultipart(uploadTemp.parseOptions("/countly/i/declared", "/countly", FIXTURE)).should.equal(true); + allowsMultipart(uploadTemp.parseOptions("/countly/i", "/countly", FIXTURE)).should.equal(true); + allowsMultipart(uploadTemp.parseOptions("/countly/crowd/admin", "/countly", FIXTURE)).should.equal(false); + allowsMultipart(uploadTemp.parseOptions("/countly", "/countly", FIXTURE)).should.equal(false); + // a prefix that only looks similar must not be stripped + allowsMultipart(uploadTemp.parseOptions("/countlyx/i/declared", "/countly", FIXTURE)).should.equal(false); + }); + + it("refuses everything when nothing is declared", function() { + allowsMultipart(uploadTemp.parseOptions("/i/declared", undefined, [])).should.equal(false); + allowsMultipart(uploadTemp.parseOptions("/i/declared", undefined, undefined)).should.equal(false); + }); + + it("accepts a bare string declaration as well as an object", function() { + allowsMultipart(uploadTemp.parseOptions("/i/string/form", undefined, FIXTURE)).should.equal(true); + }); + + it("handles a missing or empty url", function() { + allowsMultipart(uploadTemp.parseOptions(undefined, undefined, FIXTURE)).should.equal(false); + allowsMultipart(uploadTemp.parseOptions("", undefined, FIXTURE)).should.equal(false); + }); + + it("rejects every part when refusing uploads", function() { + var opts = uploadTemp.parseOptions("/crowd/admin/uploadplugin.action", undefined, FIXTURE); + opts.filter({name: "file_evil", originalFilename: "evil.jar"}).should.equal(false); + }); + }); + + describe("parseOptions - raw octet-stream", function() { + it("allows a raw body only where the declaration says raw", function() { + allowsRaw(uploadTemp.parseOptions("/i/raw/endpoint", undefined, FIXTURE)).should.equal(true); + }); + + it("refuses a raw body everywhere else, including other declared paths", function() { + [ + "/i", + "/i/declared", + "/i/nested/declared", + "/i/string/form", + "/i/undeclared", + "/o", + "/crowd/admin/uploadplugin.action" + ].forEach(function(url) { + allowsRaw(uploadTemp.parseOptions(url, undefined, FIXTURE)) + .should.equal(false, url + " must refuse a raw body"); + }); + }); + + it("drops only the octetstream parser, so fields still parse", function() { + var opts = uploadTemp.parseOptions("/i/declared", undefined, FIXTURE); + opts.enabledPlugins.should.not.containEql("octetstream"); + opts.enabledPlugins.should.containEql("querystring"); + opts.enabledPlugins.should.containEql("json"); + opts.enabledPlugins.should.containEql("multipart"); + }); + + it("honours a subdirectory installation path", function() { + allowsRaw(uploadTemp.parseOptions("/countly/i/raw/endpoint", "/countly", FIXTURE)).should.equal(true); + allowsRaw(uploadTemp.parseOptions("/countly/i/declared", "/countly", FIXTURE)).should.equal(false); + }); + }); + + describe("discardUploads", function() { + var dir; + + beforeEach(function() { + dir = path.join(os.tmpdir(), "countly-discard-test-" + process.pid); + fs.rmSync(dir, {recursive: true, force: true}); + fs.mkdirSync(dir, {recursive: true}); + }); + + afterEach(function() { + fs.rmSync(dir, {recursive: true, force: true}); + }); + + it("removes the files formidable produced", function(done) { + var a = path.join(dir, "aaa"); + var b = path.join(dir, "bbb"); + fs.writeFileSync(a, "one"); + fs.writeFileSync(b, "two"); + + var params = {}; + uploadTemp.trackUploads(params, {one: {filepath: a}, two: {filepath: b}}); + uploadTemp.discardUploads(params); + + setTimeout(function() { + fs.existsSync(a).should.equal(false); + fs.existsSync(b).should.equal(false); + done(); + }, 50); + }); + + it("accepts the formidable v1 path property", function(done) { + var target = path.join(dir, "ccc"); + fs.writeFileSync(target, "x"); + + var params = {}; + uploadTemp.trackUploads(params, {one: {path: target}}); + uploadTemp.discardUploads(params); + + setTimeout(function() { + fs.existsSync(target).should.equal(false); + done(); + }, 50); + }); + + it("ignores a path a handler substituted after parsing", function(done) { + // crash_symbolication repoints params.files.symbols.path at files + // shipped with the plugin when serving populator data + var tmp = path.join(dir, "ddd"); + var shipped = path.join(dir, "shipped-sample"); + fs.writeFileSync(tmp, "upload"); + fs.writeFileSync(shipped, "shipped asset"); + + var params = {}; + uploadTemp.trackUploads(params, {symbols: {filepath: tmp}}); + params.files = {symbols: {path: shipped}}; + uploadTemp.discardUploads(params); + + setTimeout(function() { + fs.existsSync(tmp).should.equal(false, "the upload must be removed"); + fs.existsSync(shipped).should.equal(true, "the shipped file must survive"); + done(); + }, 50); + }); + + it("only removes each file once", function(done) { + var target = path.join(dir, "eee"); + fs.writeFileSync(target, "x"); + + var params = {}; + uploadTemp.trackUploads(params, {one: {filepath: target}}); + uploadTemp.discardUploads(params); + params.uploadTempPaths.should.have.length(0); + uploadTemp.discardUploads(params); + + setTimeout(function() { + fs.existsSync(target).should.equal(false); + done(); + }, 50); + }); + + it("tolerates requests with no uploads", function() { + should.doesNotThrow(function() { + uploadTemp.discardUploads({}); + uploadTemp.discardUploads(undefined); + var params = {}; + uploadTemp.trackUploads(params, undefined); + uploadTemp.discardUploads(params); + uploadTemp.trackUploads(params, {broken: {}}); + uploadTemp.discardUploads(params); + }); + }); + }); +});