Allow %, &, and ? in spec file name. (#6539)

* Allow %, & in file name.

* Extract escapeFilenameInUrl

* Fix tests.

* Add test for ? (non-Windows).

* Remove platform condition. And handle when special chars are in the dir.

* Fix failure.

* Fix failure.

* add e2e test for specs with special characters

* minor refactor

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>

packages/server/lib/controllers/files.coffee

@@ -6,6 +6,9 @@ glob        = require("../util/glob")
 specsUtil   = require("../util/specs")
 pathHelpers = require("../util/path_helpers")
 CacheBuster = require("../util/cache_buster")
+{ escapeFilenameInUrl } = require('../util/escape_filename')
+
+SPEC_URL_PREFIX = "/__cypress/tests?p"
 
 module.exports = {
   handleFiles: (req, res, config) ->
@@ -22,6 +25,12 @@ module.exports = {
 
     @getSpecs(test, config)
     .then (specs) =>
+      specs = specs.map((fileName) =>
+        fileName = fileName.replace(SPEC_URL_PREFIX, "__CYPRESS_SPEC_URL_PREFIX__")
+
+        return escapeFilenameInUrl(fileName).replace("__CYPRESS_SPEC_URL_PREFIX__", SPEC_URL_PREFIX)
+      )
+
       @getJavascripts(config)
       .then (js) =>
         res.render iframePath, {
@@ -63,7 +72,7 @@ module.exports = {
 
   getTestUrl: (file) ->
     file += CacheBuster.get()
-    "/__cypress/tests?p=#{file}"
+    "#{SPEC_URL_PREFIX}=#{file}"
 
   getTitle: (test) ->
     if test is "__all" then "All Tests" else test

packages/server/lib/project.js

@@ -28,6 +28,7 @@ const fs = require('./util/fs')
 const keys = require('./util/keys')
 const settings = require('./util/settings')
 const specsUtil = require('./util/specs')
+const { escapeFilenameInUrl } = require('./util/escape_filename')
 
 const localCwd = cwd()
 
@@ -522,7 +523,7 @@ class Project extends EE {
     return [
       browserUrl,
       '#/tests',
-      specUrl,
+      escapeFilenameInUrl(specUrl),
     ].join('/').replace(multipleForwardSlashesRe, replacer)
   }
 

packages/server/lib/util/escape_filename.ts

@@ -0,0 +1,11 @@
+const ampersandRe = /&/g
+const percentRe = /%/g
+const questionRe = /\?/g
+
+export function escapeFilenameInUrl (url: string) {
+  // escape valid file name characters that cannot be used in URL
+  return url
+  .replace(percentRe, '%25') // %
+  .replace(ampersandRe, '%26') // &
+  .replace(questionRe, '%3F') // ? -> it's only valid in Linux
+}

packages/server/test/e2e/2_controllers_spec.coffee

@@ -1,7 +1,11 @@
+fs = require("fs-extra")
+path = require("path")
+
 e2e = require("../support/helpers/e2e")
 Fixtures = require("../support/helpers/fixtures")
 
 nonExistentSpec = Fixtures.projectPath("non-existent-spec")
+e2eProject = Fixtures.projectPath("e2e")
 
 describe "e2e plugins", ->
   e2e.setup()
@@ -14,3 +18,21 @@ describe "e2e plugins", ->
       snapshot: true
       expectedExitCode: 1
     })
+
+  it "handles specs with $, &, ? in file name", ->
+    relativeSpecPath = path.join("d?ir&1%", "%di?r2&", "s%pec&?.js")
+    ## windows doesn't support ? in file names
+    if process.platform is "win32"
+      relativeSpecPath = specPath.replace(/\?/, "")
+    specPath = path.join(e2eProject, "cypress", "integration", relativeSpecPath)
+
+    fs.outputFile(specPath, "it('passes', () => {})")
+    .then =>
+      e2e.exec(@, {
+        spec: specPath
+        sanitizeScreenshotDimensions: true
+      })
+    .then ({ stdout }) ->
+      expect(stdout).to.include("1 found (#{relativeSpecPath})")
+      expect(stdout).to.include("Running:  #{relativeSpecPath}")
+      expect(stdout).to.include("Finished processing: /XXX/XXX/XXX/cypress/videos/#{relativeSpecPath}.mp4")

packages/server/test/integration/http_requests_spec.coffee

@@ -356,14 +356,18 @@ describe "Routes", ->
 
               body = res.body
 
-              expect(body.integration).to.have.length(3)
+              expect(body.integration).to.have.length(4)
 
               ## remove the absolute path key
               body.integration = _.map body.integration, (obj) ->
                 _.pick(obj, "name", "relative")
 
               expect(res.body).to.deep.eq({
                 integration: [
+                  {
+                    "name": "sub/a&b%c.js"
+                    "relative": "tests/sub/a&b%c.js"
+                  }
                   {
                     name: "sub/sub_test.coffee"
                     relative: "tests/sub/sub_test.coffee"

packages/server/test/support/fixtures/projects/todos/tests/sub/a&b%c.js

@@ -0,0 +1,4 @@
+/* eslint-disable mocha/no-global-tests */
+it('is truthy', function () {
+  expect(true).to.be.true
+})

packages/server/test/support/fixtures/server/expected_todos_all_tests_iframe.html

@@ -18,6 +18,7 @@
     </script>
     <script type="text/javascript" src="/__cypress/tests?p=tests/_support/spec_helper.js-123"></script>
     <script type="text/javascript" src="/__cypress/tests?p=tests/etc/etc.js-123"></script>
+    <script type="text/javascript" src="/__cypress/tests?p=tests/sub/a%26b%25c.js-123"></script>
     <script type="text/javascript" src="/__cypress/tests?p=tests/sub/sub_test.coffee-123"></script>
     <script type="text/javascript" src="/__cypress/tests?p=tests/test1.js-123"></script>
     <script type="text/javascript" src="/__cypress/tests?p=tests/test2.coffee-123"></script>

packages/server/test/unit/project_spec.js

@@ -698,6 +698,35 @@ This option will not have an effect in Some-other-name. Tests that rely on web s
       })
     })
 
+    it('escapses %, &', function () {
+      const todosSpec = path.join(this.todosPath, 'tests/sub/a&b%c.js')
+
+      return this.project.getSpecUrl(todosSpec)
+      .then((str) => {
+        expect(str).to.eq('http://localhost:8888/__/#/tests/integration/sub/a%26b%25c.js')
+      })
+    })
+
+    // ? is invalid in Windows, but it can be tested here
+    // because it's a unit test and doesn't check the existence of files
+    it('escapes ?', function () {
+      const todosSpec = path.join(this.todosPath, 'tests/sub/a?.spec.js')
+
+      return this.project.getSpecUrl(todosSpec)
+      .then((str) => {
+        expect(str).to.eq('http://localhost:8888/__/#/tests/integration/sub/a%3F.spec.js')
+      })
+    })
+
+    it('escapes %, &, ? in the url dir', function () {
+      const todosSpec = path.join(this.todosPath, 'tests/s%&?ub/a.spec.js')
+
+      return this.project.getSpecUrl(todosSpec)
+      .then((str) => {
+        expect(str).to.eq('http://localhost:8888/__/#/tests/integration/s%25%26%3Fub/a.spec.js')
+      })
+    })
+
     it('returns __all spec url', function () {
       return this.project.getSpecUrl()
       .then((str) => {