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

Require braces to be present for environment variable expansion #1304

Merged
merged 1 commit into from Apr 5, 2016

Conversation

andrewkroh
Copy link
Member

The docs were updated in #1290.

// ${} is all ASCII, so bytes are fine for this operation.
i := 0
for j := 0; j < len(s); j++ {
if s[j] == '$' && j+2 < len(s) && s[j+1] == '{' {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only line that was changed from the os.Expand function.

@andrewkroh andrewkroh changed the title Require braces to be present for environment variable expansion. Require braces to be present for environment variable expansion Apr 4, 2016
{"x${Z:D}", "xD"},
{"x${Z:A B C D}", "xA B C D"}, // Spaces are allowed in the default.
{"x${Z:}", "x"},

// Defaults don't work unless braces are used.
{"x$y:D", "xy:D"},
// Un-matched braces are swallowed by the Go os.Expand function.
Copy link
Contributor

Choose a reason for hiding this comment

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

This unfortunately increases the chance of it showing up accidentally, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it does. I'll see about modify their code to remove this behavior. I didn't realize it did this until I started reading the code.

@tsg
Copy link
Contributor

tsg commented Apr 5, 2016

Merging this as is, to have it in alpha1. Remaining comments can be addressed in other PRs.

@tsg tsg merged commit 8d0a983 into elastic:master Apr 5, 2016
andrewkroh added a commit to andrewkroh/beats that referenced this pull request Apr 13, 2016
- Variable expressions cannot be split across newlines.
- Unterminated variable expressions cause errors rather than silently swallowing characters.
- Added an escape sequence for using a literal '${' value.

Follow up to issue elastic#1304
ruflin pushed a commit that referenced this pull request Apr 15, 2016
- Variable expressions cannot be split across newlines.
- Unterminated variable expressions cause errors rather than silently swallowing characters.
- Added an escape sequence for using a literal '${' value.

Follow up to issue #1304
@andrewkroh andrewkroh deleted the feature/env-expansion-braces branch April 19, 2016 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants