Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cypress fails to parse environment variables with empty values #3742

Closed
Ameobea opened this issue Mar 18, 2019 · 6 comments · Fixed by #3743
Closed

Cypress fails to parse environment variables with empty values #3742

Ameobea opened this issue Mar 18, 2019 · 6 comments · Fixed by #3743
Labels
pkg/server This is due to an issue in the packages/server directory topic: environment variables type: bug

Comments

@Ameobea
Copy link
Contributor

Ameobea commented Mar 18, 2019

Current behavior:

Cypress fails to parse command line environment variables when environment variables with empty values are supplied.

Desired behavior:

Should default to setting the values to undefined

Steps to reproduce: (app code and test code)

Run a command like cypress run --env="USERNAME=,PASSWORD="

Versions

3.2.0 and earlier


Stack trace:

A JavaScript error occurred in the main process
Uncaught Exception:
TypeError: Cannot read property 'split' of undefined
    at pipesToCommas (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/lib/util/args.js:95:13)
    at JSONOrCoerce (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/lib/util/args.js:115:9)
    at /root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:13402:38
    at /root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:4911:15
    at baseForOwn (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:2996:24)
    at Function.mapValues (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:13401:7)
    at /root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:4374:28
    at arrayReduce (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:683:21)
    at baseWrapperValue (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:4373:14)
    at LodashWrapper.wrapperValue (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/node_modules/lodash/lodash.js:9052:14)
    at sanitizeAndConvertNestedArgs (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/lib/util/args.js:150:4)
    at Object.toObject (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/lib/util/args.js:225:21)
    at Object.start (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/lib/cypress.js:70:40)
    at Object.<anonymous> (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/index.js:21:43)
    at Object.<anonymous> (/root/.cache/Cypress/3.2.0/Cypress/resources/app/packages/server/index.js:23:3)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/root/.cache/Cypress/3.2.0/Cypress/resources/app/index.js:2:1)
    at Object.<anonymous> (/root/.cache/Cypress/3.2.0/Cypress/resources/app/index.js:3:3)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Object.<anonymous> (/root/.cache/Cypress/3.2.0/Cypress/resources/electron.asar/browser/init.js:171:8)
    at Object.<anonymous> (/root/.cache/Cypress/3.2.0/Cypress/resources/electron.asar/browser/init.js:173:3)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Function.Module.runMain (module.js:605:10)
    at startup (bootstrap_node.js:167:16)
    at bootstrap_node.js:589:3
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review stage: work in progress and removed stage: needs review The PR code is done & tested, needs review labels Mar 19, 2019
@jennifer-shehane jennifer-shehane added type: bug pkg/server This is due to an issue in the packages/server directory labels Mar 19, 2019
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review stage: work in progress and removed stage: work in progress stage: needs review The PR code is done & tested, needs review labels Mar 20, 2019
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Mar 20, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 20, 2019

The code for this is done in cypress-io/cypress#3743, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

flotwig pushed a commit that referenced this issue Mar 20, 2019
Fixes #3742

 * When parsing `--env` arguments containing variables with empty values (a valid pattern), Cypress crashed due to trying to `.split()` on an `undefined`.  For example, `cypress run --env="USERNAME=,PASSWORD="` would crash the application.
 * This commit changes the regular expression used to parse environment variables to use a `.*` rather than a `.+`.  This will initialize environment variables that are supplied without values to an empty string.  Since empty strings are falsy in JavaScript, this should work fine with users defaulting to hardcoded values in the case that environment variables aren't supplied (the use case that I was trying to create when I encountered this bug)

Previous behavior:

```sh
"USERNAME=,PASSWORD=".split(',').map(pair => pair.split(/=(.+)/))
// [["USERNAME="], ["PASSWORD="]]
```

New behavior:

```sh
"USERNAME=,PASSWORD=".split(',').map(pair => pair.split(/=(.*)/))
// [["USERNAME=", "", ""], ["PASSWORD=", "", ""]]
```

<!--
Thanks for contributing!

Please explain what changes were made and also
reference any issues that were fixed with #[ISSUE]
-->
laurinenas pushed a commit to laurinenas/cypress that referenced this issue Apr 28, 2019
Fixes cypress-io#3742

 * When parsing `--env` arguments containing variables with empty values (a valid pattern), Cypress crashed due to trying to `.split()` on an `undefined`.  For example, `cypress run --env="USERNAME=,PASSWORD="` would crash the application.
 * This commit changes the regular expression used to parse environment variables to use a `.*` rather than a `.+`.  This will initialize environment variables that are supplied without values to an empty string.  Since empty strings are falsy in JavaScript, this should work fine with users defaulting to hardcoded values in the case that environment variables aren't supplied (the use case that I was trying to create when I encountered this bug)

Previous behavior:

```sh
"USERNAME=,PASSWORD=".split(',').map(pair => pair.split(/=(.+)/))
// [["USERNAME="], ["PASSWORD="]]
```

New behavior:

```sh
"USERNAME=,PASSWORD=".split(',').map(pair => pair.split(/=(.*)/))
// [["USERNAME=", "", ""], ["PASSWORD=", "", ""]]
```

<!--
Thanks for contributing!

Please explain what changes were made and also
reference any issues that were fixed with #[ISSUE]
-->
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 17, 2019

Released in 3.3.0.

@ajainarayanan
Copy link

Just an update in case anyone is still seeing this issue. I had a similar issue happen even after upgrading to 3.3.2 version of Cypress. The issue I had looks something like this,

 node_modules/cypress/bin/cypress run --browser chrome --env var1=value1,var2 =value2

The space before = in var2 was causing the same issue. This is not an issue in cypress per se but seems to have been a decision by minimist node module. Just an FYI if someone saw this and was wondering why they are still seeing it even after upgrading cypress.

@maxime1992
Copy link

In case it's useful for someone else, I was doing something pretty silly...

The cypress folder is not at the root of the project so I wanted to launch it with an argument specifying that.

Reading the doc, I found an argument config and I didn't bother reading further 🤦‍♂️

So I was doing:

yarn run cypress open --config ./public-site-e2e/cypress.json

This flag's actually not for that at all.

And I have to do instead:

yarn run cypress open --project ./public-site-e2e

The error I had with the first one was

TypeError: Cannot read property 'split' of undefined

So it took me about 20mn to figure out what was wrong 😅

@jennifer-shehane
Copy link
Member

@maxime1992 Understandable. We do have an issue to add a --configFile argument. #1369

@mholtzhausen
Copy link

@maxime1992 @jennifer-shehane
--config sets configuration files on the cli line

The correct way to load a different config:

yarn run cypress open --config-file ./yourconfig.json

@cypress-io cypress-io locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg/server This is due to an issue in the packages/server directory topic: environment variables type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants