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

vars not expanded in path directives #6387

Open
CRCinAU opened this issue Jun 10, 2024 · 6 comments
Open

vars not expanded in path directives #6387

CRCinAU opened this issue Jun 10, 2024 · 6 comments
Assignees

Comments

@CRCinAU
Copy link

CRCinAU commented Jun 10, 2024

I'm trying to simplify a bit of Caddyfile - and using a variable as part of the path name.

Part of the Caddyfile I'm using is as follows:

handle /xplane/charts/* {
        vars {
                cycle "2024-MAR-21"
        }
        @daps {
                not file
                not path /xplane/charts/DAPS-{vars.cycle}/
                path_regexp ^/xplane/charts/DAPS.+/(.*)$
        }
        @ersa {
                not file
                not path /xplane/charts/ERSA-{vars.cycle}/
                path_regexp ^/xplane/charts/ERSA.+/(.*)$
        }

        redir @daps /xplane/charts/DAPS-{vars.cycle}/{re.1} permanent
        redir @ersa /xplane/charts/ERSA-{vars.cycle}/{re.1} permanent

        file_server {
                browse
        }
}

It seems like {vars.cycle} does not get expanded in the not path lines, but does work correctly in the redir lines.

Currently, with {vars.cycle} present, this results in an endless loop of 301 redirects.

If I change the line to: not path /xplane/charts/DAPS-2024-MAR-21/, then everything works as expected.

Caddy running via caddy:latest - currently:

docker exec -ti caddy /bin/sh
/srv # caddy --version
v2.8.4 h1:q3pe0wpBj1OcHFZ3n/1nl4V4bxBrYoSoab7rL9BMYNk=
/srv # 
@francislavoie
Copy link
Member

francislavoie commented Jun 10, 2024

For ref, context from the forums https://caddy.community/t/rewriting-of-changing-url-paths/24426

The path matcher matches paths exactly, not as a prefix. Do you mean to match /xplane/charts/DAPS-{vars.cycle}/* instead (note the * at the end)?

@CRCinAU
Copy link
Author

CRCinAU commented Jun 10, 2024

Hi @francislavoie - the aim is to:

  1. Redirect a path, say: /xplane/charts/DAPS-2000-DEC-22/filename.pdf to /xplane/charts/DAPS-2024-MAR-21/filename.pdf

  2. The directory is browseable, but without the not path matcher to the directory listing page, a loop of 301's is entered.

  3. If I hard-set the not path to the value of vars.cycle instead of using the variable, then it works as expected.

For the sake of example, with the not path hard coded instead of using variables, the following redirects happen:

/xplane/charts/DAPS-2000-DEC-01/  -> /xplane/charts/DAPS-2024-MAR-21/ (can browse)
/xplane/charts/DAPS-2018-JAN-23/Melbourne (YMML).pdf  ->  /xplane/charts/DAPS-2024-MAR-21/Melbourne (YMML).pdf

With the not path set to include {vars.cycle}, the following redirects happen:

/xplane/charts/DAPS-2000-DEC-01/  -> /xplane/charts/DAPS-2024-MAR-21/ (Redirect loop to this URL)
/xplane/charts/DAPS-2018-JAN-23/Melbourne (YMML).pdf  ->  /xplane/charts/DAPS-2024-MAR-21/Melbourne (YMML).pdf

I don't believe that using a /* on the end would negate the infinite redirect loop for a directory listing served by the browse option to file_server.

The not path is there to prevent the infinite 301 redirects to the same page.

@francislavoie
Copy link
Member

francislavoie commented Jun 10, 2024

Oh right. The other details is the path matcher matches case-insensitively by converting the request path to lowercase, which means you need to write your matcher with lowercase characters as well to match it (including the variable contents). So if you change your variable to 2024-mar-21 it should work.

The tricky part is we lowercase the path during the provisioning phase (at server/config start) to avoid needing to do it on every request (saving a few nanoseconds at most on each request). This means that any dynamic parts of the path don't get lowercased since it happens before replacing the placeholders.

This also means that the placeholder itself must also be lowercase 😬 so you can't have uppercase characters in the placeholder/variable name 🤦‍♂️ I'd call this part a bug. Obviously the only fix is to move the lowercasing to after replacing any placeholders, which would be a slight performance hit for everyone even if they don't use placeholders. Or in provisioning we could do a smarter lowercase to only lowercase parts not within placeholders, but I dunno.

/cc @mholt a decision needs to be made here

@CRCinAU
Copy link
Author

CRCinAU commented Jun 10, 2024

The other details is the path matcher matches case-insensitively by converting the request path to lowercase, which means you need to write your matcher with lowercase characters as well to match it (including the variable contents). So if you change your variable to 2024-mar-21 it should work.

Hmmmm - this is interesting - as I'm guessing that would break the redir line using lower case as well - as it'll do filesystem matching on a lower case name - where the actual directory on disk is upper case?

EDIT: This does seem to work - although obviously not ideal:

handle /xplane/charts/* {
        vars {
                cycle "2024-mar-21"
                cycle2 "2024-MAR-21"
        }
        @daps {
                not file
                not path /xplane/charts/DAPS-{vars.cycle}/
                path_regexp ^/xplane/charts/DAPS.+/(.*)$
        }
        @ersa {
                not file
                not path /xplane/charts/ERSA-{vars.cycle}/
                path_regexp ^/xplane/charts/ERSA.+/(.*)$
        }

        redir @daps /xplane/charts/DAPS-{vars.cycle2}/{re.1} permanent
        redir @ersa /xplane/charts/ERSA-{vars.cycle2}/{re.1} permanent

        file_server {
                browse
        }
}

@mholt mholt self-assigned this Jun 10, 2024
@francislavoie
Copy link
Member

francislavoie commented Jun 10, 2024

Yeah you'd probably have to use 2 different variables in that case I guess. Or you could change to using an expression matcher and do the string comparison yourself, something like this:

	vars cycle "2024-MAR-21"
	@daps `!file() && {path} != ('/xplane/charts/DAPS-' + {vars.cycle} + '/') && path_regexp('^/xplane/charts/DAPS.+/(.*)$')`
	redir @daps ...

Placeholders inside a CEL expression act like a function call so they can't be inlined inside of a string, so concat needs to be used to form the path there. Simple enough though.

@CRCinAU
Copy link
Author

CRCinAU commented Jun 10, 2024

Or you could change to using an expression matcher and do the string comparison yourself, something like this:

This actually works perfectly. Thanks for this workaround! Gets me one step closer to moving my static site to Caddy :)

Will leave this issue open though, as I believe the original config should actually work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants