Consolidate url parse/create in PackageURL module

server/middleware/PackageUtils.js

@@ -1,60 +0,0 @@
-const { parse: parseURL } = require('url')
-
-const URLFormat = /^\/((?:@[^\/@]+\/)?[^\/@]+)(?:@([^\/]+))?(\/.*)?$/
-
-const decodeParam = (param) =>
-  param ? decodeURIComponent(param) : ''
-
-const ValidQueryKeys = {
-  main: true,
-  meta: true,
-  json: true
-}
-
-const queryIsValid = (query) =>
-  Object.keys(query).every(key => ValidQueryKeys[key])
-
-const parsePackageURL = (url) => {
-  const { pathname, search, query } = parseURL(url, true)
-
-  if (!queryIsValid(query))
-    return null
-
-  const match = URLFormat.exec(pathname)
-
-  if (match == null)
-    return null
-
-  const packageName = match[1]
-  const packageVersion = decodeParam(match[2]) || 'latest'
-  const filename = decodeParam(match[3])
-
-  return {          // If the URL is /@scope/name@version/file.js?main=browser:
-    pathname,       // /@scope/name@version/path.js
-    search,         // ?main=browser
-    query,          // { main: 'browser' }
-    packageName,    // @scope/name
-    packageVersion, // version
-    filename        // /file.js
-  }
-}
-
-const createPackageURL = (packageName, version, filename, search) => {
-  let pathname = `/${packageName}`
-
-  if (version != null)
-    pathname += `@${version}`
-
-  if (filename)
-    pathname += filename
-
-  if (search)
-    pathname += search
-
-  return pathname
-}
-
-module.exports = {
-  parsePackageURL,
-  createPackageURL
-}

server/middleware/PackageUtils.test.js

@@ -1,88 +0,0 @@
-const { parsePackageURL } = require('./PackageUtils')
-
-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: ''
-    })
-  })
-
-  it('returns null for invalid pathnames', () => {
-    expect(parsePackageURL('history')).toBe(null)
-  })
-
-  it('returns null for invalid query parameters', () => {
-    expect(parsePackageURL('/query-string@5.0.0/index.js?invalid')).toBe(null)
-  })
-})

server/middleware/fetchPackage.js

@@ -1,7 +1,7 @@
 const { maxSatisfying: maxSatisfyingVersion } = require('semver')
 const PackageCache = require('../PackageCache')
 const PackageInfo = require('../PackageInfo')
-const { createPackageURL } = require('./PackageUtils')
+const PackageURL = require('../PackageURL')
 
 /**
  * Fetch the package from the registry and store a local copy on disk.
@@ -35,12 +35,12 @@ function fetchPackage(req, res, next) {
         }
       })
     } else if (req.packageVersion in tags) {
-      res.redirect(createPackageURL(req.packageName, tags[req.packageVersion], req.filename, req.search))
+      res.redirect(PackageURL.create(req.packageName, tags[req.packageVersion], req.filename, req.search))
     } else {
       const maxVersion = maxSatisfyingVersion(Object.keys(versions), req.packageVersion)
 
       if (maxVersion) {
-        res.redirect(createPackageURL(req.packageName, maxVersion, req.filename, req.search))
+        res.redirect(PackageURL.create(req.packageName, maxVersion, req.filename, req.search))
       } else {
         res.status(404).send(`Cannot find package ${req.packageSpec}`)
       }

server/middleware/parseURL.js

@@ -1,15 +1,10 @@
-const { parsePackageURL } = require('./PackageUtils')
+const PackageURL = require('../PackageURL')
 
 /**
  * Parse and validate the URL.
  */
 function parseURL(req, res, next) {
-  let url
-  try {
-    url = parsePackageURL(req.url)
-  } catch (error) {
-    return res.status(403).send(`Invalid URL: ${req.url}`)
-  }
+  const url = PackageURL.parse(req.url)
 
   if (url == null)
     return res.status(403).send(`Invalid URL: ${req.url}`)