From 3c947e205a6c1d987b876a4bcdced6577481b248 Mon Sep 17 00:00:00 2001 From: Joel Denning Date: Mon, 15 Mar 2021 11:27:12 -0600 Subject: [PATCH] Send 404 when env is invalid. Don't use default importmap file. Resolves #96. (#97) --- src/cache-control.js | 3 ++- src/config.js | 2 -- src/io-methods/default.js | 22 +++++++--------- src/io-methods/filesystem.js | 46 ++++++--------------------------- src/io-methods/s3.js | 13 +++++----- src/modify.js | 2 +- src/trusted-urls.js | 3 ++- src/web-server.js | 27 ++++++++----------- test/import-map.test.js | 50 ++++++++++++++++++++++++++++++++++++ 9 files changed, 90 insertions(+), 78 deletions(-) diff --git a/src/cache-control.js b/src/cache-control.js index e077951..9a7d50e 100644 --- a/src/cache-control.js +++ b/src/cache-control.js @@ -1,6 +1,7 @@ -const { config } = require("./config"); +const { getConfig } = require("./config"); exports.getCacheControl = (hostSpecificCacheControl) => { + const config = getConfig(); if (config.cacheControl) { return config.cacheControl; } diff --git a/src/config.js b/src/config.js index 0628d54..a022d95 100644 --- a/src/config.js +++ b/src/config.js @@ -20,7 +20,5 @@ if (argv._.length === 0) { } } -exports.config = config; - exports.setConfig = (newConfig) => (config = newConfig); exports.getConfig = () => config; diff --git a/src/io-methods/default.js b/src/io-methods/default.js index 380b9a9..ee5846f 100644 --- a/src/io-methods/default.js +++ b/src/io-methods/default.js @@ -1,30 +1,26 @@ "use strict"; const _ = require("lodash"); -const fs = require("./filesystem"); +const filesystem = require("./filesystem"); const s3 = require("./s3"); const azure = require("./azure"); const google = require("./google-cloud-storage"); -const config = require("../config").config; +const { getConfig } = require("../config"); const memory = require("./memory"); -const defaultFilePath = - config && config.manifestFormat === "importmap" - ? "import-map.json" - : "sofe-manifest.json"; - function getFilePath(env) { + const config = getConfig(); if (_.has(config, ["locations", env])) { return config.locations[env]; } else if (_.has(config, ["locations", "default"])) { return config.locations.default; - } else if (!_.isEmpty(config.locations)) { + } else if (!env && !_.isEmpty(config.locations)) { return config.locations[Object.keys(config.locations)[0]]; } else { - return defaultFilePath; + throw Error(`No such environment '${env}'. Check your configuration file.`); } } -exports.readManifest = function (env) { +exports.readManifest = async function (env) { var filePath = getFilePath(env); if (usesAzure(filePath)) { //uses azure @@ -38,11 +34,11 @@ exports.readManifest = function (env) { return memory.readManifest(filePath); } else { //use local file - return fs.readManifest(filePath); + return filesystem.readManifest(filePath); } }; -exports.writeManifest = function (data, env) { +exports.writeManifest = async function (data, env) { var filePath = getFilePath(env); if (usesAzure(filePath)) { @@ -57,7 +53,7 @@ exports.writeManifest = function (data, env) { return memory.writeManifest(filePath, data); } else { //use local file - return fs.writeManifest(filePath, data); + return filesystem.writeManifest(filePath, data); } }; diff --git a/src/io-methods/filesystem.js b/src/io-methods/filesystem.js index 0b78e7d..73b2973 100644 --- a/src/io-methods/filesystem.js +++ b/src/io-methods/filesystem.js @@ -1,47 +1,17 @@ "use strict"; -const fs = require("fs"); -const config = require("../config").config; +const fs = require("fs/promises"); +const { getConfig } = require("../config"); const jsHelpers = require("./js-file-helpers.js"); exports.readManifest = function (filePath) { - return new Promise((resolve, reject) => { - //create file if not already created - fs.open(filePath, "a", function (err, fd) { - if (err) reject(`Could not open file ${filePath}`); - else { - fs.readFile(filePath, "utf8", function (err2, data) { - if (err2) { - console.error(err2); - reject(`Could not read file ${filePath}`); - } else resolve(data); - }); - } - }); - }); + return fs.readFile(filePath, "utf-8"); }; exports.writeManifest = function (filePath, data) { - const jsonPromise = new Promise((resolve, reject) => { - fs.writeFile(filePath, data, function (err) { - if (err) reject(`Could not write file ${filePath}`); - else resolve(); - }); - }); + const config = getConfig(); + const useJsHelpers = config && config.writeJsFile; + const finalFilePath = useJsHelpers ? jsHelpers.getJsPath(filePath) : filePath; + const finalData = useJsHelpers ? jsHelpers.createJsString(data) : data; - const jsPromise = new Promise((resolve, reject) => { - if (!config || !config.writeJsFile) { - resolve(); - } else { - fs.writeFile( - jsHelpers.getJsPath(filePath), - jsHelpers.createJsString(data), - function (err) { - if (err) reject(`Could not write file ${filePath}`); - else resolve(); - } - ); - } - }); - - return Promise.all([jsonPromise, jsPromise]); + return fs.writeFile(finalFilePath, finalData, "utf-8"); }; diff --git a/src/io-methods/s3.js b/src/io-methods/s3.js index 74d87bf..2f735c3 100644 --- a/src/io-methods/s3.js +++ b/src/io-methods/s3.js @@ -1,11 +1,11 @@ "use strict"; const aws = require("aws-sdk"), - config = require("../config").config, + getConfig = require("../config").getConfig, jsHelpers = require("./js-file-helpers.js"); -if (config && config.region) { - aws.config.update({ region: config.region }); +if (getConfig() && getConfig().region) { + aws.config.update({ region: getConfig().region }); } const { getCacheControl } = require("../cache-control"); @@ -24,8 +24,8 @@ function parseFilePath(filePath) { let s3PutObjectCacheControl; let s3PutObjectConfigSansCacheControl = {}; -if (config && config.s3 && config.s3.putObject) { - const { CacheControl, ...rest } = config.s3.putObject; +if (getConfig() && getConfig().s3 && getConfig().s3.putObject) { + const { CacheControl, ...rest } = getConfig().s3.putObject; s3PutObjectCacheControl = CacheControl; s3PutObjectConfigSansCacheControl = { ...rest }; } @@ -33,7 +33,7 @@ if (config && config.s3 && config.s3.putObject) { const cacheControl = getCacheControl(s3PutObjectCacheControl); const s3 = new aws.S3({ - endpoint: config.s3Endpoint, + endpoint: getConfig().s3Endpoint, }); exports.readManifest = function (filePath) { @@ -56,6 +56,7 @@ exports.readManifest = function (filePath) { }; exports.writeManifest = function (filePath, data) { + const config = getConfig(); const jsonPromise = new Promise(function (resolve, reject) { const file = parseFilePath(filePath); s3.putObject( diff --git a/src/modify.js b/src/modify.js index c3a23b1..41cfb74 100644 --- a/src/modify.js +++ b/src/modify.js @@ -2,7 +2,7 @@ // File editing const lock = new (require("rwlock"))(); const ioOperations = require("./io-operations.js"); -const getConfig = require("./config").getConfig; +const { getConfig } = require("./config"); const isImportMap = () => { const format = getConfig().manifestFormat; diff --git a/src/trusted-urls.js b/src/trusted-urls.js index 03ee637..2729c65 100644 --- a/src/trusted-urls.js +++ b/src/trusted-urls.js @@ -1,6 +1,7 @@ -const { config } = require("./config"); +const { getConfig } = require("./config"); exports.checkUrlUnsafe = (urlStr) => { + const config = getConfig(); if (!config.urlSafeList) { return null; } else { diff --git a/src/web-server.js b/src/web-server.js index a7d5797..0e6e215 100644 --- a/src/web-server.js +++ b/src/web-server.js @@ -58,11 +58,13 @@ app.use(auth); app.use(express.static(__dirname + "/public")); function getEnv(req) { - if (req.query.env === undefined) { - return "default"; - } else { - return req.query.env; - } + return req.query.env; +} + +function sendError(res, ex, prefix) { + const status = + ex && ex.message && /No such environment/.test(ex.message) ? 404 : 500; + res.status(status).send(`${prefix} -- ${ex.toString()}`); } // --------- // @@ -119,7 +121,7 @@ function handleGetManifest(req, res) { }) .catch((ex) => { console.error(ex); - res.status(500).send(`Could not read manifest file -- ${ex.toString()}`); + sendError(res, ex, "Could not read import map"); }); } @@ -206,8 +208,7 @@ app.patch("/import-map.json", (req, res) => { res.status(200).send(newImportMap); }) .catch((err) => { - console.error(err); - res.status(500).send(`Could not update import map`); + sendError(res, err, "Could not update import map"); }); }) .catch((err) => { @@ -266,10 +267,7 @@ app.patch("/services", function (req, res) { res.send(json); }) .catch((ex) => { - console.error(ex); - res - .status(500) - .send(`Could not write manifest file -- ${ex.toString()}`); + sendError(res, ex, "Could not patch service"); }); }) .catch((err) => { @@ -285,10 +283,7 @@ app.delete("/services/:serviceName", function (req, res) { res.send(data); }) .catch((ex) => { - console.error(ex); - res - .status(500) - .send(`Could not delete service ${req.params.serviceName}`); + sendError(res, ex, "Could not delete service"); }); }); diff --git a/test/import-map.test.js b/test/import-map.test.js index 5f82a95..55f7b07 100644 --- a/test/import-map.test.js +++ b/test/import-map.test.js @@ -188,4 +188,54 @@ describe(`/import-map.json`, () => { expect(response.body.imports.b).not.toBeDefined(); expect(response.body.imports["b/"]).not.toBeDefined(); }); + + it(`returns a 404 when you try to retrieve an import map for an environment that doesn't exist`, async () => { + await request(app) + .get("/import-map.json") + .query({ + env: "envThatDoesntExist", + }) + .expect(404); + }); + + it(`returns a 404 when you attempt to patch an import map environment that doesn't exist`, async () => { + const response = await request(app) + .patch("/import-map.json") + .query({ + skip_url_check: true, + env: "envThatDoesntExist", + }) + .set("accept", "json") + .send({ + imports: { + a: "/a-1.mjs", + }, + }) + .expect(404); + }); + + it(`returns a 404 when you attempt to patch a service for an environment that doesn't exist`, async () => { + await request(app) + .patch("/services") + .query({ + skip_url_check: true, + env: "envThatDoesntExist", + }) + .set("accept", "json") + .send({ + service: "a", + url: "/a-1-updated.mjs", + }) + .expect(404); + }); + + it(`returns a 404 when you attempt to delete a service for an environment that doesn't exist`, async () => { + await request(app) + .delete("/services/b") + .query({ + env: "envThatDoesntExist", + }) + .set("accept", "json") + .expect(404); + }); });