Skip to content

funcs: Don't panic if templatefile path is sensitive#35180

Merged
apparentlymart merged 1 commit intomainfrom
b-templatefile-sensitive-path-crash
May 20, 2024
Merged

funcs: Don't panic if templatefile path is sensitive#35180
apparentlymart merged 1 commit intomainfrom
b-templatefile-sensitive-path-crash

Conversation

@apparentlymart
Copy link
Copy Markdown
Contributor

Previously we were partially propagating any marks from the path, but not going all the way so we still ran into trouble when trying to use the string containing the file contents.

Now we'll have loadTmpl also return the marks it had to read through to actually parse the template, and then we'll use those (instead of the original path marks) to mark the result. In practice the pathMarks and the tmplMarks should always match today, but this is intentionally structured to make the data flow clearer -- the marks always travel along with whatever they related to -- so we're less likely to break this accidentally under future maintenance.

This fixes #31119.

Previously we were partially propagating any marks from the path, but not
going all the way so we still ran into trouble when trying to use the
string containing the file contents.

Now we'll have loadTmpl also return the marks it had to read through to
actually parse the template, and then we'll use those (instead of the
original path marks) to mark the result. In practice the pathMarks and
the tmplMarks should always match today, but this is intentionally
structured to make the data flow clearer -- the marks always travel along
with whatever they related to -- so we're less likely to break this
accidentally under future maintenence.
@kmoe
Copy link
Copy Markdown
Member

kmoe commented May 20, 2024

@apparentlymart do we want to backport this into v1.8?

@apparentlymart
Copy link
Copy Markdown
Contributor Author

This situation seems rare enough that backporting would not be very impactful vs. just waiting for v1.9, but I'm happy to do if you disagree and think it's worth it!

@apparentlymart apparentlymart merged commit 9dd28fc into main May 20, 2024
@apparentlymart apparentlymart deleted the b-templatefile-sensitive-path-crash branch May 20, 2024 15:34
@github-actions
Copy link
Copy Markdown
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link
Copy Markdown
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1.1.9: templatefile + sensitive value: go panic: value is marked, so must be unmarked first

2 participants