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

redir syntax and examples seem invalid in some cases #6517

Open
Jiehong opened this issue Aug 13, 2024 · 2 comments
Open

redir syntax and examples seem invalid in some cases #6517

Jiehong opened this issue Aug 13, 2024 · 2 comments
Labels
discussion 💬 The right solution needs to be found

Comments

@Jiehong
Copy link

Jiehong commented Aug 13, 2024

Looking at https://caddyserver.com/docs/caddyfile/directives/redir#syntax, it seems as if the matcher is optional according to the examples.

Yet, trying to use it without a matcher does not work (both caddy run and caddy validate complain).

This works:

...
handle_errors {
		@404 {
			expression `{err.status_code} == 404`
		}
		handle @404 {
			redir * /subdir/index.html
		}

		# Any other kind of error should be passed through
		handle {
			respond "Internal http server error: {err.trace}"
		}
	}

changing the redir line to redir /subdir/index.html leads to the following issue:

Error: adapting config using caddyfile: parsing caddyfile tokens for 'handle_errors': parsing caddyfile tokens for 'handle': parsing caddyfile tokens for 'redir': wrong argument count or unexpected line ending after 'redir', at Caddyfile:41, at Caddyfile:42, at Caddyfile:48

Yet, the doc shows 3 out of 4 examples without a matcher:

image

I guess the syntax doc should explain that a bit better, shouldn't it?

FYI:

  • caddy version: v2.8.4 h1:q3pe0wpBj1OcHFZ3n/1nl4V4bxBrYoSoab7rL9BMYNk=
@francislavoie
Copy link
Member

francislavoie commented Aug 13, 2024

It's because if the first argument starts with a slash, it gets parsed as being the matcher for the directive, so you must use * to disambiguate. See https://caddyserver.com/docs/caddyfile/matchers#syntax

We did recently make a change to root and rewrite to be "argument length aware" so if only one argument follows the directive name then it allows the matcher to be omitted. We could do the same for redir because if there's only a single argument, the user probably meant it to be the redir target and not the matcher. But only if there's a single argument and not if they try to change the status code as well, the matcher would then be required otherwise it would redirect to the status code number as the target instead.

@Jiehong
Copy link
Author

Jiehong commented Aug 13, 2024

The error message reported could be more explicit and tell us that perhaps?

For example:

redir was provided was only 1 argument, treated as a path due it starting with '/'. A second argument is required.

If not very possible, perhaps an example in redir about this specific case (so that the doc of matchers is not duplicated)?

@mholt mholt added the discussion 💬 The right solution needs to be found label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion 💬 The right solution needs to be found
Projects
None yet
Development

No branches or pull requests

3 participants