fileserver: Add disable_canonical_uris Caddyfile subdirective

[mritd]

add 'canonical_uris' parameter to caddyfile

reference #2741

Signed-off-by: mritd mritd@linux.com

[CLAassistant]

All committers have signed the CLA.

[francislavoie]

Ah, good catch! This was an oversight when Caddyfile support was added to file_server early on it seems.

I think the UX of this option isn't consistent with other boolean options. Since CanonicalURIs being true is the same as being unset/nil (since the default is to enable the canonicalization redirects), I think the Caddyfile option should be called disable_canonical_uris and the argument should be removed.

Also, please add a Caddyfile adapt test (or update an existing one) to cover this option. See https://github.com/caddyserver/caddy/tree/master/caddytest/integration/caddyfile_adapt

[mritd]

Thanks for your reply, I have modified the subdirective UX, but I still have a question:

Is it necessary to add strict parameter verification here?

https://github.com/mritd/caddy/blob/737ead4e3de9b27d64f0b3dbd3e654149719b16c/modules/caddyhttp/fileserver/caddyfile.go#L117

case "disable_canonical_uris":
	if h.NextArg() {
		return nil, h.ArgErr()
	}

[francislavoie]

Not necessary, but it's a nice to have to make sure invalid configs are rejected. If it's rejected, we know for sure nobody used an argument, so in the future if we want to add an argument for whatever reason, it wouldn't be a possibly breaking change.

[mritd]

Okay thank you 😁

[mholt]

Thanks, this'll probably come in handy.