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

fix(cli) Allow env variables containing equals #179

Merged
merged 2 commits into from
Mar 13, 2020
Merged

fix(cli) Allow env variables containing equals #179

merged 2 commits into from
Mar 13, 2020

Conversation

robwa10
Copy link
Contributor

@robwa10 robwa10 commented Mar 12, 2020

fix(cli): Correctly parse out env variables that contain an equal sign

Fixes a bug where we don't correctly parse env variables that contain an equal sign as part of the variable. An equal sign is a valid special character and occurs reasonably often in things such as CLIENT_SECRETS, etc. This seems to be a regression as the previous env command didn't parse those incorrectly.

@robwa10 robwa10 added bug Something isn't working cli Relates to `zapier-platform-cli` labels Mar 12, 2020
@robwa10 robwa10 changed the title Allow env vars containing equals Allow env variables containing equals Mar 12, 2020
Copy link
Contributor

@xavdid xavdid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

I tested a little bit and found that this wouldn't work for multiline values (such as public keys). Rather than chase down every possible valid value with a regex, I'd suggest the following:

const [key, ...valueParts] = kvPair.split('=')
const value = valueParts.join('=')

When run with your test, we get

> key
'SECURE_KEY'
> valueParts
[ '8n*e9!', '92g' ]

This'll work with any number of equals in the value and avoids the regex and the unusual .split call. Also plays nice with whatever is otherwise valid in the value. How does that sound?

@robwa10
Copy link
Contributor Author

robwa10 commented Mar 13, 2020

Ah, yes, spread syntax. I knew there was a more concise way forward here! Also, never considered multi-line inputs like public keys so a great catch there. 👍

@xavdid
Copy link
Contributor

xavdid commented Mar 13, 2020

cool! feel free to merge at your leisure.

@robwa10
Copy link
Contributor Author

robwa10 commented Mar 13, 2020

@xavdid Github doesn't trust me...to be fair I've given them some good reasons early on in my Github experiences... 😂

@xavdid
Copy link
Contributor

xavdid commented Mar 13, 2020

odd! I'll merge it and take a closer look at why you can't

@xavdid xavdid merged commit a0160bc into master Mar 13, 2020
@xavdid xavdid deleted the patch-37575 branch March 13, 2020 23:31
@xavdid xavdid changed the title Allow env variables containing equals fix(cli) Allow env variables containing equals Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Relates to `zapier-platform-cli`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants