Skip to content

caddyfile: Add parse error on site address with trailing {#4163

Merged
mholt merged 2 commits into2.4from
caddyfile-parse-address-brace
May 12, 2021
Merged

caddyfile: Add parse error on site address with trailing {#4163
mholt merged 2 commits into2.4from
caddyfile-parse-address-brace

Conversation

@francislavoie
Copy link
Member

@francislavoie francislavoie commented May 12, 2021

Honestly, we should've done this a long time ago, but better late than never.

This is an incredibly common mistake made by users, so we should catch it earlier in the parser and give a more friendly message. Often it ends up adapting but with mistakes, or erroring out later due to other site addresses being read as directives.

There's not really ever a situation where a lone { is valid at the end of a site address, so this should be fine. But I suppose there are edgecases where the user wants to use a path matcher where it ends specifically in {, but... why?

@francislavoie francislavoie added the bug 🐞 Something isn't working label May 12, 2021
@francislavoie francislavoie added this to the v2.4.1 milestone May 12, 2021
@francislavoie francislavoie requested a review from mholt May 12, 2021 01:55
@francislavoie francislavoie changed the title caddyfile: Add parse error on site address in { caddyfile: Add parse error on site address with trailing { May 12, 2021
@francislavoie francislavoie force-pushed the caddyfile-parse-address-brace branch from 4e4b548 to 8bfa681 Compare May 12, 2021 04:35
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.

Doesn't caddy fmt fix this? 🤔

@francislavoie francislavoie requested a review from mholt May 12, 2021 20:58
This is an incredibly common mistake made by users, so we should catch it earlier in the parser and give a more friendly message. Often it ends up adapting but with mistakes, or erroring out later due to other site addresses being read as directives.

There's not really ever a situation where a lone '{' is valid at the end of a site address (but I suppose there are edgecases where the user wants to use a path matcher where it ends specifically in `{`, but... why?), so this should be fine.
@francislavoie francislavoie force-pushed the caddyfile-parse-address-brace branch from 68ffbbf to 3566591 Compare May 12, 2021 20:59
@mholt mholt merged commit b82db99 into 2.4 May 12, 2021
@mholt mholt deleted the caddyfile-parse-address-brace branch May 12, 2021 22:18
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.

2 participants