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

hclsyntax: Impose an upper limit on a refined prefix in TemplateExpr #617

Merged

Conversation

wata727
Copy link
Contributor

@wata727 wata727 commented Jul 1, 2023

Fixes #616

There is no limit to the length of string prefixes produced by template expressions, so in rare cases they may return a refined unknown string has too long a prefix.

The cty's msgpack decoder limits the size of an acceptable refinements to 1 kiB, so such a value cannot be handled and an error occurs.

This change limits the length of prefixes to 128 B, so overly long prefixes are no longer an issue in most cases.

There is no limit to the length of string prefixes produced by template
expressions, so in rare cases they may return a refined unknown string
has too long a prefix.

The cty's msgpack decoder limits the size of an acceptable refinements
to 1 kiB, so such a value cannot be handled and an error occurs.

This change limits the length of prefixes to 128 B, so overly long
prefixes are no longer an issue in most cases.
@apparentlymart
Copy link
Member

apparentlymart commented Jul 7, 2023

Thanks for this fix, @wata727!

I'm going to merge this now, but I expect we'll wait to tag a new HCL release and upgrade Terraform's dependency for now because the first Terraform release including refinements is not due for a while yet and we'll probably find some other quirks of this new functionality we'll also want to patch before release, and so hopefully we can bundle them together into a single HCL release if so.

I have also spent a little time thinking about how to solve this more generally in cty but my current work priorities are elsewhere so I've not yet had a chance to try out the different ideas. I'll aim to spend some time on that before the Terraform v1.6.0 release freeze, but I think it's still reasonable to have this extra limit in HCL templates in particular because string prefix refinements are primarily intended for dealing with short, predictable prefixes (like ami- on EC2 machine image IDs) and it seems unlikely to me that making decisions based on very long prefixes will be significantly more profitable than decisions based on short prefixes.

(Based on my research so far, the most significant benefits I anticipated with this feature were making var.foo != "" be able to return true or false based on a known prefix, and for Terraform's startswith function being able to answer startswith(var.foo, "ami-") even if the full ID isn't known, assuming that the value came from a source that knows it's generating an AMI ID.)

@apparentlymart apparentlymart merged commit 527ec31 into hashicorp:main Jul 7, 2023
4 checks passed
@wata727 wata727 deleted the truncate_string_prefix_refinements branch July 8, 2023 12:42
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.

hclsyntax: Template expression produces too large value refinements
2 participants