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

strconv: Fix escaped backslashes \\ in braces ${} #125

Merged
merged 1 commit into from
Jun 7, 2016
Merged

Conversation

phinze
Copy link
Contributor

@phinze phinze commented Jun 6, 2016

Fixes hashicorp/terraform#6359 and several
related issues by unescaping a double backslash within braces to a
single backslash.

This bumps into the bit of HIL that still hangs out in HCL - for values
that aren't interpolated, like Terraform's variable defaults - users
were unable to get a backslash to show up within ${}.

That's because ${} is handled specially to allow for, e.g., double
quotes inside of braces.

Here, we add \\ as a special cased escape (along with \") so users
can get backslashes in these scenarios by doubling them up.

@sethvargo
Copy link
Contributor

👍

@@ -70,6 +71,9 @@ func Unquote(s string) (t string, err error) {
if q, _ := utf8.DecodeRuneInString(s); q == '"' {
continue
}
if q, _ := utf8.DecodeRuneInString(s); q == '\\' {
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The performance is unlikely to matter but decoding utf8 runes is not exactly cheap, so perhaps we should just turn this into a switch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair - I was just so punch drunk that the copy paste seemed too hilarious not to do 😄

Fixes hashicorp/terraform#6359 and several
related issues by unescaping a double backslash within braces to a
single backslash.

This bumps into the bit of HIL that still hangs out in HCL - for values
that aren't interpolated, like Terraform's variable defaults - users
were unable to get a backslash to show up within `${}`.

That's because `${}` is handled specially to allow for, e.g., double
quotes inside of braces.

Here, we add `\\` as a special cased escape (along with `\"`) so users
can get backslashes in these scenarios by doubling them up.
@phinze phinze changed the title Handle \\ within ${} strconv: Fix escaped backslashes \\ in braces ${} Jun 7, 2016
@phinze
Copy link
Contributor Author

phinze commented Jun 7, 2016

Now, with more switch!

@mitchellh
Copy link
Contributor

LGTM!

@phinze phinze merged commit d7400db into master Jun 7, 2016
@phinze phinze deleted the b-escapes branch June 7, 2016 00:19
phinze added a commit to hashicorp/terraform that referenced this pull request Jun 7, 2016
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