Break up parseURL middleware

server/__tests__/server-test.js

@@ -1,11 +1,13 @@
 const request = require("supertest");
+
 const createServer = require("../createServer");
+
 const clearBlacklist = require("./utils/clearBlacklist");
 const withBlacklist = require("./utils/withBlacklist");
 const withRevokedToken = require("./utils/withRevokedToken");
 const withToken = require("./utils/withToken");
 
-describe("The server", () => {
+describe("The production server", () => {
   let server;
   beforeEach(() => {
     server = createServer();
@@ -61,6 +63,17 @@ describe("The server", () => {
     });
   });
 
+  describe("GET /_publicKey", () => {
+    it("echoes the public key", done => {
+      request(server)
+        .get("/_publicKey")
+        .end((err, res) => {
+          expect(res.text).toMatch(/PUBLIC KEY/);
+          done();
+        });
+    });
+  });
+
   describe("POST /_auth", () => {
     it("creates a new auth token", done => {
       request(server)
@@ -114,17 +127,6 @@ describe("The server", () => {
     });
   });
 
-  describe("GET /_publicKey", () => {
-    it("echoes the public key", done => {
-      request(server)
-        .get("/_publicKey")
-        .end((err, res) => {
-          expect(res.text).toMatch(/PUBLIC KEY/);
-          done();
-        });
-    });
-  });
-
   describe("POST /_blacklist", () => {
     afterEach(clearBlacklist);
 

server/createRouter.js

@@ -2,9 +2,8 @@ const express = require("express");
 const bodyParser = require("body-parser");
 const cors = require("cors");
 
-const renderPage = require("./utils/renderPage");
-const requireAuth = require("./middleware/requireAuth");
 const MainPage = require("./components/MainPage");
+const renderPage = require("./utils/renderPage");
 
 function route(setup) {
   const app = express.Router();
@@ -43,17 +42,17 @@ function createRouter() {
     route(app => {
       app.post(
         "/",
-        requireAuth("blacklist.add"),
+        require("./middleware/requireAuth")("blacklist.add"),
         require("./actions/addToBlacklist")
       );
       app.get(
         "/",
-        requireAuth("blacklist.read"),
+        require("./middleware/requireAuth")("blacklist.read"),
         require("./actions/showBlacklist")
       );
       app.delete(
         "*",
-        requireAuth("blacklist.remove"),
+        require("./middleware/requireAuth")("blacklist.remove"),
         require("./middleware/validatePackageURL"),
         require("./actions/removeFromBlacklist")
       );
@@ -66,7 +65,10 @@ function createRouter() {
 
   app.get(
     "*",
-    require("./middleware/parseURL"),
+    require("./middleware/redirectLegacyURLs"),
+    require("./middleware/validatePackageURL"),
+    require("./middleware/validatePackageName"),
+    require("./middleware/validateQuery"),
     require("./middleware/checkBlacklist"),
     require("./middleware/fetchPackage"),
     require("./middleware/findFile"),

server/createServer.js

@@ -28,7 +28,7 @@ function createServer(publicDir, statsFile) {
   if (process.env.NODE_ENV !== "test") {
     app.use(
       morgan(
-        // Modified version of the Heroku router's log format
+        // Modified version of Heroku's log format
         // https://devcenter.heroku.com/articles/http-routing#heroku-router-log-format
         'method=:method path=":url" host=:req[host] request_id=:req[x-request-id] cf_ray=:req[cf-ray] fwd=:fwd status=:status bytes=:res[content-length]'
       )

server/middleware/parseURL.js

@@ -1,85 +0,0 @@
-const validateNpmPackageName = require("validate-npm-package-name");
-const parsePackageURL = require("../utils/parsePackageURL");
-const createSearch = require("./utils/createSearch");
-
-const knownQueryParams = {
-  main: true, // Deprecated, see #63
-  meta: true,
-  module: true
-};
-
-function isKnownQueryParam(param) {
-  return !!knownQueryParams[param];
-}
-
-function queryIsKnown(query) {
-  return Object.keys(query).every(isKnownQueryParam);
-}
-
-function sanitizeQuery(query) {
-  const saneQuery = {};
-
-  Object.keys(query).forEach(param => {
-    if (isKnownQueryParam(param)) saneQuery[param] = query[param];
-  });
-
-  return saneQuery;
-}
-
-/**
- * Parse and validate the URL.
- */
-function parseURL(req, res, next) {
-  // Permanently redirect /_meta/path to /path?meta.
-  if (req.path.match(/^\/_meta\//)) {
-    req.query.meta = "";
-    return res.redirect(301, req.path.substr(6) + createSearch(req.query));
-  }
-
-  // Permanently redirect /path?json => /path?meta
-  if (req.query.json != null) {
-    delete req.query.json;
-    req.query.meta = "";
-    return res.redirect(301, req.path + createSearch(req.query));
-  }
-
-  // Redirect requests with unknown query params to their equivalents
-  // with only known params so they can be served from the cache. This
-  // prevents people using random query params designed to bust the cache.
-  if (!queryIsKnown(req.query)) {
-    return res.redirect(302, req.path + createSearch(sanitizeQuery(req.query)));
-  }
-
-  const url = parsePackageURL(req.url);
-
-  // Disallow invalid URLs.
-  if (url == null) {
-    return res
-      .status(403)
-      .type("text")
-      .send(`Invalid URL: ${req.url}`);
-  }
-
-  const nameErrors = validateNpmPackageName(url.packageName).errors;
-
-  // Disallow invalid package names.
-  if (nameErrors) {
-    const reason = nameErrors.join(", ");
-    return res
-      .status(403)
-      .type("text")
-      .send(`Invalid package name "${url.packageName}" (${reason})`);
-  }
-
-  req.packageName = url.packageName;
-  req.packageVersion = url.packageVersion;
-  req.packageSpec = `${url.packageName}@${url.packageVersion}`;
-  req.pathname = url.pathname;
-  req.filename = url.filename;
-  req.search = url.search;
-  req.query = url.query;
-
-  next();
-}
-
-module.exports = parseURL;

server/middleware/redirectLegacyURLs.js

@@ -0,0 +1,23 @@
+const createSearch = require("./utils/createSearch");
+
+/**
+ * Redirect old URLs that we no longer support.
+ */
+function redirectLegacyURLs(req, res, next) {
+  // Permanently redirect /_meta/path to /path?meta.
+  if (req.path.match(/^\/_meta\//)) {
+    req.query.meta = "";
+    return res.redirect(301, req.path.substr(6) + createSearch(req.query));
+  }
+
+  // Permanently redirect /path?json => /path?meta
+  if (req.query.json != null) {
+    delete req.query.json;
+    req.query.meta = "";
+    return res.redirect(301, req.path + createSearch(req.query));
+  }
+
+  next();
+}
+
+module.exports = redirectLegacyURLs;

server/middleware/validatePackageName.js

@@ -0,0 +1,20 @@
+const validateNpmPackageName = require("validate-npm-package-name");
+
+/**
+ * Reject requests for invalid npm package names.
+ */
+function validatePackageName(req, res, next) {
+  const nameErrors = validateNpmPackageName(req.packageName).errors;
+
+  if (nameErrors) {
+    const reason = nameErrors.join(", ");
+    return res
+      .status(403)
+      .type("text")
+      .send(`Invalid package name "${req.packageName}" (${reason})`);
+  }
+
+  next();
+}
+
+module.exports = validatePackageName;

server/middleware/validatePackageURL.js

@@ -1,8 +1,8 @@
 const parsePackageURL = require("../utils/parsePackageURL");
 
 /**
- * Adds various properties to the request object to do with the
+ * Parse the URL and add various properties to the request object to
- * package/file being requested.
+ * do with the package/file being requested. Reject invalid URLs.
  */
 function validatePackageURL(req, res, next) {
   const url = parsePackageURL(req.url);
@@ -14,7 +14,7 @@ function validatePackageURL(req, res, next) {
   req.packageName = url.packageName;
   req.packageVersion = url.packageVersion;
   req.packageSpec = `${url.packageName}@${url.packageVersion}`;
-  req.pathname = url.pathname;
+  req.pathname = url.pathname; // TODO: remove
   req.filename = url.filename;
   req.search = url.search;
   req.query = url.query;

server/middleware/validateQuery.js

@@ -0,0 +1,34 @@
+const createSearch = require("./utils/createSearch");
+
+const knownQueryParams = {
+  main: true, // Deprecated, see #63
+  meta: true,
+  module: true
+};
+
+function isKnownQueryParam(param) {
+  return !!knownQueryParams[param];
+}
+
+function sanitizeQuery(originalQuery) {
+  const query = {};
+
+  Object.keys(originalQuery).forEach(param => {
+    if (isKnownQueryParam(param)) query[param] = originalQuery[param];
+  });
+
+  return query;
+}
+
+/**
+ * Reject URLs with invalid query parameters to increase cache hit rates.
+ */
+function validateQuery(req, res, next) {
+  if (!Object.keys(req.query).every(isKnownQueryParam)) {
+    return res.redirect(302, req.path + createSearch(sanitizeQuery(req.query)));
+  }
+
+  next();
+}
+
+module.exports = validateQuery;