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

caddyfile: Prevent bad block opening tokens #4655

Merged
merged 2 commits into from Mar 23, 2022
Merged

Conversation

francislavoie
Copy link
Member

@francislavoie francislavoie commented Mar 23, 2022

Fix #4501

Basically, we want to prevent two cases where bad block openings can happen.

localhost
reverse_proxy { to abc:123 }

☝️ this one used to result in the upstreams list containing } (the last token on the line) because since no newline happens, to reads all the remaining tokens on the same line, consuming both abc:123 and }, but never closing the block.

This bug is being fixed by checking if after we see a { token, whether the next token is on a new line; if not, err. This could break "legitimate" situations where some hypothetical expression-like directive wants to consume all the tokens on the line where { is some syntactical token of that "language", but I think this is unlikely, and can be worked around by quoting the tokens like "{" and "}" (or with backticks).

localhost
reverse_proxy abc:123 {}

☝️ this one ends up in an upstreams list with abc:123 and {}; I don't think there's ever a case where there'd be a placeholder with no name, and having it at the end of line seems like almost always a mistake (where the user is trying to put an empty block in their config).

Same idea, we're checking if when we see {} whether the next token is on a new line, and if so, err. This can also be worked around by quoting with "{}"

Worth noting that #4643 made the quote workaround possible in this implementation, because now we know whether a particular token was quoted. We would only know the token value previously, without context of quoting. 🎉

@francislavoie francislavoie added the bug 🐞 Something isn't working label Mar 23, 2022
@francislavoie francislavoie added this to the v2.5.0 milestone Mar 23, 2022
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks Francis, this is cool. We can give this a try!

@mholt mholt merged commit 134b805 into master Mar 23, 2022
@mholt mholt deleted the caddyfile-bad-blocks branch March 23, 2022 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reverse proxy sporadically 502 when empty curly braces are written on the same line in Caddyfile
3 participants