Remove unneeded req params related to the URL

modules/createServer.js

@@ -17,9 +17,9 @@ import noQuery from './middleware/noQuery.js';
 import redirectLegacyURLs from './middleware/redirectLegacyURLs.js';
 import requestLog from './middleware/requestLog.js';
 import validateFilename from './middleware/validateFilename.js';
-import validatePackageURL from './middleware/validatePackageURL.js';
+import validatePackagePathname from './middleware/validatePackagePathname.js';
 import validatePackageName from './middleware/validatePackageName.js';
-import validateVersion from './middleware/validateVersion.js';
+import validatePackageVersion from './middleware/validatePackageVersion.js';
 
 function createApp(callback) {
   const app = express();
@@ -61,18 +61,18 @@ export default function createServer() {
         app.get(
           '*/',
           noQuery(),
-          validatePackageURL,
+          validatePackagePathname,
           validatePackageName,
-          validateVersion,
+          validatePackageVersion,
           serveDirectoryBrowser
         );
 
         app.get(
           '*',
           noQuery(),
-          validatePackageURL,
+          validatePackagePathname,
           validatePackageName,
-          validateVersion,
+          validatePackageVersion,
           serveFileBrowser
         );
       })
@@ -86,9 +86,9 @@ export default function createServer() {
       app.get(
         '*/',
         allowQuery('meta'),
-        validatePackageURL,
+        validatePackagePathname,
         validatePackageName,
-        validateVersion,
+        validatePackageVersion,
         validateFilename,
         serveDirectoryMetadata
       );
@@ -96,9 +96,9 @@ export default function createServer() {
       app.get(
         '*',
         allowQuery('meta'),
-        validatePackageURL,
+        validatePackagePathname,
         validatePackageName,
-        validateVersion,
+        validatePackageVersion,
         validateFilename,
         serveFileMetadata
       );
@@ -120,9 +120,9 @@ export default function createServer() {
       app.get(
         '*',
         allowQuery('module'),
-        validatePackageURL,
+        validatePackagePathname,
         validatePackageName,
-        validateVersion,
+        validatePackageVersion,
         validateFilename,
         findEntry,
         serveModule
@@ -145,9 +145,9 @@ export default function createServer() {
     app.get(
       '*',
       noQuery(),
-      validatePackageURL,
+      validatePackagePathname,
       validatePackageName,
-      validateVersion,
+      validatePackageVersion,
       validateFilename,
       findEntry,
       serveFile

modules/middleware/findEntry.js

@@ -23,7 +23,7 @@ function fileRedirect(req, res, entry) {
         req.packageName,
         req.packageVersion,
         entry.path,
-        createSearch(req.query)
+        req.query
       )
     );
 }
@@ -42,7 +42,7 @@ function indexRedirect(req, res, entry) {
         req.packageName,
         req.packageVersion,
         entry.path,
-        createSearch(req.query)
+        req.query
       )
     );
 }

modules/middleware/validateFilename.js

@@ -1,7 +1,4 @@
 import createPackageURL from '../utils/createPackageURL.js';
-import createSearch from '../utils/createSearch.js';
-
-const leadingSlashes = /^\/*/;
 
 function filenameRedirect(req, res) {
   let filename;
@@ -63,8 +60,8 @@ function filenameRedirect(req, res) {
       createPackageURL(
         req.packageName,
         req.packageVersion,
-        filename.replace(leadingSlashes, '/'),
-        createSearch(req.query)
+        filename.replace(/^\/*/, '/'),
+        req.query
       )
     );
 }

modules/middleware/validatePackagePathname.js

@@ -0,0 +1,19 @@
+import parsePackagePathname from '../utils/parsePackagePathname.js';
+
+/**
+ * Parse the pathname in the URL. Reject invalid URLs.
+ */
+export default function validatePackagePathname(req, res, next) {
+  const parsed = parsePackagePathname(req.path);
+
+  if (parsed == null) {
+    return res.status(403).send({ error: `Invalid URL: ${req.path}` });
+  }
+
+  req.packageName = parsed.packageName;
+  req.packageVersion = parsed.packageVersion;
+  req.packageSpec = parsed.packageSpec;
+  req.filename = parsed.filename;
+
+  next();
+}

modules/middleware/validatePackageURL.js

@@ -1,23 +0,0 @@
-import parsePackageURL from '../utils/parsePackageURL.js';
-
-/**
- * Parse the URL and add various properties to the request object to
- * do with the package/file being requested. Reject invalid URLs.
- */
-export default function validatePackageURL(req, res, next) {
-  const url = parsePackageURL(req.url);
-
-  if (url == null) {
-    return res.status(403).send({ error: `Invalid URL: ${req.url}` });
-  }
-
-  req.packageName = url.packageName;
-  req.packageVersion = url.packageVersion;
-  req.packageSpec = `${url.packageName}@${url.packageVersion}`;
-  req.pathname = url.pathname; // TODO: remove
-  req.filename = url.filename;
-  req.search = url.search;
-  req.query = url.query;
-
-  next();
-}

modules/middleware/validatePackageVersion.js

@@ -13,7 +13,7 @@ function semverRedirect(req, res, newVersion) {
     .redirect(
       302,
       req.baseUrl +
-        createPackageURL(req.packageName, newVersion, req.filename, req.search)
+        createPackageURL(req.packageName, newVersion, req.filename, req.query)
     );
 }
 
@@ -65,7 +65,6 @@ async function validateVersion(req, res, next) {
   );
 
   if (!req.packageConfig) {
-    // TODO: Log why.
     return res
       .status(500)
       .type('text')

modules/utils/__tests__/parsePackagePathname-test.js

@@ -0,0 +1,59 @@
+import parsePackagePathname from '../parsePackagePathname.js';
+
+describe('parsePackagePathname', () => {
+  it('parses plain packages', () => {
+    expect(parsePackagePathname('/history@1.0.0/umd/history.min.js')).toEqual({
+      packageName: 'history',
+      packageVersion: '1.0.0',
+      packageSpec: 'history@1.0.0',
+      filename: '/umd/history.min.js'
+    });
+  });
+
+  it('parses plain packages with a hyphen in the name', () => {
+    expect(parsePackagePathname('/query-string@5.0.0/index.js')).toEqual({
+      packageName: 'query-string',
+      packageVersion: '5.0.0',
+      packageSpec: 'query-string@5.0.0',
+      filename: '/index.js'
+    });
+  });
+
+  it('parses plain packages with no version specified', () => {
+    expect(parsePackagePathname('/query-string/index.js')).toEqual({
+      packageName: 'query-string',
+      packageVersion: 'latest',
+      packageSpec: 'query-string@latest',
+      filename: '/index.js'
+    });
+  });
+
+  it('parses plain packages with version spec', () => {
+    expect(parsePackagePathname('/query-string@>=4.0.0/index.js')).toEqual({
+      packageName: 'query-string',
+      packageVersion: '>=4.0.0',
+      packageSpec: 'query-string@>=4.0.0',
+      filename: '/index.js'
+    });
+  });
+
+  it('parses scoped packages', () => {
+    expect(
+      parsePackagePathname('/@angular/router@4.3.3/src/index.d.ts')
+    ).toEqual({
+      packageName: '@angular/router',
+      packageVersion: '4.3.3',
+      packageSpec: '@angular/router@4.3.3',
+      filename: '/src/index.d.ts'
+    });
+  });
+
+  it('parses package names with a period in them', () => {
+    expect(parsePackagePathname('/index.js')).toEqual({
+      packageName: 'index.js',
+      packageVersion: 'latest',
+      packageSpec: 'index.js@latest',
+      filename: ''
+    });
+  });
+});

modules/utils/__tests__/parsePackageURL-test.js

@@ -1,80 +0,0 @@
-import parsePackageURL from '../parsePackageURL.js';
-
-describe('parsePackageURL', () => {
-  it('parses plain packages', () => {
-    expect(parsePackageURL('/history@1.0.0/umd/history.min.js')).toEqual({
-      pathname: '/history@1.0.0/umd/history.min.js',
-      search: '',
-      query: {},
-      packageName: 'history',
-      packageVersion: '1.0.0',
-      filename: '/umd/history.min.js'
-    });
-  });
-
-  it('parses plain packages with a hyphen in the name', () => {
-    expect(parsePackageURL('/query-string@5.0.0/index.js')).toEqual({
-      pathname: '/query-string@5.0.0/index.js',
-      search: '',
-      query: {},
-      packageName: 'query-string',
-      packageVersion: '5.0.0',
-      filename: '/index.js'
-    });
-  });
-
-  it('parses plain packages with no version specified', () => {
-    expect(parsePackageURL('/query-string/index.js')).toEqual({
-      pathname: '/query-string/index.js',
-      search: '',
-      query: {},
-      packageName: 'query-string',
-      packageVersion: 'latest',
-      filename: '/index.js'
-    });
-  });
-
-  it('parses plain packages with version spec', () => {
-    expect(parsePackageURL('/query-string@>=4.0.0/index.js')).toEqual({
-      pathname: '/query-string@>=4.0.0/index.js',
-      search: '',
-      query: {},
-      packageName: 'query-string',
-      packageVersion: '>=4.0.0',
-      filename: '/index.js'
-    });
-  });
-
-  it('parses scoped packages', () => {
-    expect(parsePackageURL('/@angular/router@4.3.3/src/index.d.ts')).toEqual({
-      pathname: '/@angular/router@4.3.3/src/index.d.ts',
-      search: '',
-      query: {},
-      packageName: '@angular/router',
-      packageVersion: '4.3.3',
-      filename: '/src/index.d.ts'
-    });
-  });
-
-  it('parses package names with a period in them', () => {
-    expect(parsePackageURL('/index.js')).toEqual({
-      pathname: '/index.js',
-      search: '',
-      query: {},
-      packageName: 'index.js',
-      packageVersion: 'latest',
-      filename: ''
-    });
-  });
-
-  it('parses valid query parameters', () => {
-    expect(parsePackageURL('/history?main=browser')).toEqual({
-      pathname: '/history',
-      search: '?main=browser',
-      query: { main: 'browser' },
-      packageName: 'history',
-      packageVersion: 'latest',
-      filename: ''
-    });
-  });
-});

modules/utils/createPackageURL.js

@@ -1,14 +1,16 @@
+import createSearch from './createSearch.js';
+
 export default function createPackageURL(
   packageName,
-  version,
-  pathname,
-  search
+  packageVersion,
+  filename,
+  query
 ) {
   let url = `/${packageName}`;
 
-  if (version != null) url += `@${version}`;
-  if (pathname) url += pathname;
-  if (search) url += search;
+  if (packageVersion) url += `@${packageVersion}`;
+  if (filename) url += filename;
+  if (query) url += createSearch(query);
 
   return url;
 }

modules/utils/parsePackagePathname.js

@@ -0,0 +1,26 @@
+const packagePathnameFormat = /^\/((?:@[^/@]+\/)?[^/@]+)(?:@([^/]+))?(\/.*)?$/;
+
+export default function parsePackagePathname(pathname) {
+  try {
+    pathname = decodeURIComponent(pathname);
+  } catch (error) {
+    return null;
+  }
+
+  const match = packagePathnameFormat.exec(pathname);
+
+  // Disallow invalid pathnames.
+  if (match == null) return null;
+
+  const packageName = match[1];
+  const packageVersion = match[2] || 'latest';
+  const filename = (match[3] || '').replace(/\/\/+/g, '/');
+
+  return {
+    // If the pathname is /@scope/name@version/file.js:
+    packageName, // @scope/name
+    packageVersion, // version
+    packageSpec: `${packageName}@${packageVersion}`, // @scope/name@version
+    filename // /file.js
+  };
+}

modules/utils/parsePackageURL.js

@@ -1,33 +0,0 @@
-import url from 'url';
-
-const packageURLFormat = /^\/((?:@[^/@]+\/)?[^/@]+)(?:@([^/]+))?(\/.*)?$/;
-
-export default function parsePackageURL(originalURL) {
-  let { pathname, search, query } = url.parse(originalURL, true);
-  try {
-    pathname = decodeURIComponent(pathname);
-  } catch (error) {
-    return null;
-  }
-
-  const match = packageURLFormat.exec(pathname);
-
-  // Disallow invalid URL formats.
-  if (match == null) {
-    return null;
-  }
-
-  const packageName = match[1];
-  const packageVersion = match[2] || 'latest';
-  const filename = (match[3] || '').replace(/\/\/+/g, '/');
-
-  return {
-    // If the URL is /@scope/name@version/file.js?main=browser:
-    pathname, // /@scope/name@version/path.js
-    search: search || '', // ?main=browser
-    query, // { main: 'browser' }
-    packageName, // @scope/name
-    packageVersion, // version
-    filename // /file.js
-  };
-}