Merge pull request #199 from alphagov/sengi/cwe-20

Fix regex misuse.

spec/javascripts/environment.spec.js

@@ -2,7 +2,7 @@ describe('Popup.environment', function () {
   function createEnvironmentForUrl (location) {
     var a = document.createElement('a')
     a.href = location
-    return Popup.environment(a.href, a.host, a.origin).allEnvironments
+    return Popup.environment(a.href, a.hostname, a.origin).allEnvironments
   }
 
   it('returns the correct environment links when the user is on production', function () {

src/fetch-page-data.js

@@ -5,7 +5,7 @@
 chrome.runtime.sendMessage({
   action: 'populatePopup',
   currentLocation: window.location.href,
-  currentHost: window.location.host,
+  currentHost: window.location.hostname,
   currentOrigin: window.location.origin,
   currentPathname: window.location.pathname,
   renderingApplication: getMetatag('govuk:rendering-application'),

src/popup/content_links.js

@@ -6,12 +6,13 @@ Popup.generateContentLinks = function (location, origin, pathname, currentEnviro
 
   // If no path can be found (which means we're probably in a publishing app)
   // Similarly if we're on GOVUK account, not many of the links are relevant
-  if (!path || origin.match(/www.account/)) {
+  if (!path || origin.includes('www.account')) {
     return []
   }
 
-  // This is 'https://www.gov.uk' or 'https://www-origin.integration.publishing.service.gov.uk/', etc.
-  if (origin === 'http://webarchive.nationalarchives.gov.uk' || origin.match(/draft-origin/) || origin.match(/content-data/) || origin.match(/support/)) {
+  // Origin looks like 'https://www.gov.uk' or 'https://www-origin.integration.publishing.service.gov.uk/' or similar.
+  if (origin === 'http://webarchive.nationalarchives.gov.uk' ||
+      /draft-origin|content-data|support/.test(origin)) {
     origin = 'https://www.gov.uk'
   }
 

src/popup/environment.js

@@ -1,18 +1,16 @@
 var Popup = Popup || {}
 
-// Given a location, host and origin, generate URLs for all GOV.UK environments.
+// Given a location, hostname and origin, generate URLs for all GOV.UK environments.
 //
 // Returns a hash with envs, including one with `class: "current"` to show
 // the current environment.
 Popup.environment = function (location, host, origin) {
   function isPartOfGOVUK () {
-    return host.match(/www.gov.uk/) ||
-        host.match(/publishing.service.gov.uk/) ||
-        host.match(/dev.gov.uk/)
+    return /^(www|.*\.publishing\.service|(www\.)?dev)\.gov\.uk$/.test(host)
   }
 
   function isGOVUKAccount () {
-    return host.match(/www.account/) || host.match(/login.service.dev/)
+    return /^(www\.account.*\.service\.gov\.uk|login\.service\.dev)$/.test(host)
   }
 
   if (!isPartOfGOVUK()) {
@@ -28,7 +26,6 @@ Popup.environment = function (location, host, origin) {
         host: 'https://www.gov.uk',
         origin: origin
       }
-
     }
   }
 
@@ -64,7 +61,7 @@ Popup.environment = function (location, host, origin) {
   ]
 
   var application = isGOVUKAccount() ? host.split('.')[1] : host.split('.')[0]
-  var inFrontend = application.match(/www/) && !isGOVUKAccount()
+  var inFrontend = application.includes('www') && !isGOVUKAccount()
   var environments = ENVIRONMENTS
 
   var currentEnvironment

src/popup/extract_path.js

@@ -3,25 +3,26 @@ var Popup = Popup || {}
 // Extract the relevant path from a location, such as `/foo` from URLs like
 // `www.gov.uk/foo` and `www.gov.uk/api/content/foo`.
 Popup.extractPath = function (location, pathname, renderingApplication) {
+  const url = new URL(location)
   var extractedPath
 
-  if (location.match(/api\/content/)) {
+  if (location.includes('api/content')) {
     extractedPath = pathname.replace('api/content/', '')
-  } else if (location.match(/anonymous_feedback/)) {
+  } else if (location.includes('anonymous_feedback')) {
     extractedPath = extractQueryParameter(location, 'path')
-  } else if (location.match(/content-data/)) {
+  } else if (location.includes('content-data')) {
     extractedPath = pathname.replace('metrics/', '')
-  } else if (location.match(/nationalarchives.gov.uk/)) {
+  } else if (/\.?nationalarchives\.gov\.uk$/.test(url.hostname)) {
     extractedPath = pathname.split('https://www.gov.uk')[1]
-  } else if (location.match(/api\/search.json/)) {
+  } else if (location.includes('api/search.json')) {
     extractedPath = extractQueryParameter(location, 'filter_link')
-  } else if (location.match(/info/)) {
+  } else if (location.includes('info')) {
     extractedPath = pathname.replace('info/', '')
-  } else if (location.match(/api.*\.json/)) {
+  } else if (/api.*\.json/.test(location)) {
     extractedPath = pathname.replace('api/', '').replace('.json', '')
-  } else if (location.match(/visualise/)) {
+  } else if (location.includes('visualise')) {
     extractedPath = pathname.replace('/y/visualise', '')
-  } else if (location.match(/www/) || location.match(/draft-origin/)) {
+  } else if (/www|draft-origin/.test(location)) {
     extractedPath = pathname
   }
 

src/popup/popup.js

@@ -71,9 +71,9 @@ var Popup = Popup || {};
   })
 
   // Render the popup.
-  function renderPopup (location, host, origin, pathname, renderingApplication, windowHeight, abTestBuckets) {
+  function renderPopup (location, hostname, origin, pathname, renderingApplication, windowHeight, abTestBuckets) {
     // Creates a view object with the data and render a template with it.
-    var view = createView(location, host, origin, pathname, renderingApplication, abTestBuckets)
+    var view = createView(location, hostname, origin, pathname, renderingApplication, abTestBuckets)
 
     var contentStore = view.contentLinks.find(function (el) { return el.name === 'Content item (JSON)' })
 
@@ -162,7 +162,7 @@ var Popup = Popup || {};
         // TODO: we're not actually re-rendering the popup correctly here, because
         // we don't have access to the DOM here. This is a temporary solution to
         // make most functionality work after the user clicks a button in the popup.
-        renderPopup(location.href, location.host, location.origin, location.pathname, {})
+        renderPopup(location.href, location.hostname, location.origin, location.pathname, {})
       })
     })
 
@@ -212,11 +212,11 @@ var Popup = Popup || {};
     })
   }
 
-  // This is the view object. It takes a location, host, origin, the name of the
+  // This is the view object. It takes a location, hostname, origin, the name of the
   // rendering app and a list of A/B test buckets and creates an object with all
   // URLs and other view data to render the popup.
-  function createView (location, host, origin, pathname, renderingApplication, abTestBuckets) {
-    var environment = Popup.environment(location, host, origin)
+  function createView (location, hostname, origin, pathname, renderingApplication, abTestBuckets) {
+    var environment = Popup.environment(location, hostname, origin)
     var contentLinks = Popup.generateContentLinks(location, origin, pathname, environment.currentEnvironment, renderingApplication)
     // var abTests = Popup.findActiveAbTests(abTestBuckets)