Improve `php_fastcgi` options for `try_files` overrides

[ttys3]

purpose:

I have a php-fpm site using nginx and now I replaced it with caddy2.

The site does not have a single entrypoint to handle all requests.

(I want to make it clear: for single entrypoint apps, current caddy implements works perfect-_-)

at first, I simply use php_fastcgi 127.0.0.1:9000, and it works. but the 404 status did not passed to the handle_errors handler, it was eaten all by php_fastcgi

---

the original try

my first config:

# listen http and https both for example.com
{$SITE_ADDRESS:http://example.com, https://example.com} {
    root * /app/public

    # https://caddyserver.com/docs/caddyfile/directives/php_fastcgi
    php_fastcgi 127.0.0.1:9000

    file_server

    handle_errors {
        rewrite * /error.html
        templates
        file_server
    }
}

for example, any 404 pages that should happened just passed all to index.php and in actual no 404 happend, urls like:

/this-is-not-found.html
/blah-this-is-another-notfound/xxxxxx
/xxxxxxx
/sub/xxx/sub/xxxxx

this is not my desired result.

---

first attempt try

So I checked the doc and found that is has an index option, so I changed php_fastcgi 127.0.0.1:9000 to

php_fastcgi 127.0.0.1:9000 {
    index off
}

but this caused another problem: when user access /, it can not serve /index.php. this is a problem.

---

second attempt try

so I copied the directive from the doc Expanded form, and changed it like this:

the change I made is changed try_files {path} {path}/index.php index.php

to

try_files {path} {path}/index.php

the full config:

# listen http and https both for example.com
{$SITE_ADDRESS:http://example.com, https://example.com} {
    root * /app/public

    # https://caddyserver.com/docs/caddyfile/directives/php_fastcgi
    # php_fastcgi 127.0.0.1:9000
    route {
        # Add trailing slash for directory requests
        @canonicalPath {
            file {path}/index.php
            not path */
        }
        redir @canonicalPath {path}/ 308

        # If the requested file does not exist, try index files
        @indexFiles file {
            # try_files {path} {path}/index.php index.php
            try_files {path} {path}/index.php
            split_path .php
        }
        rewrite @indexFiles {http.matchers.file.relative}

        # Proxy PHP files to the FastCGI responder
        @phpFiles path *.php
        reverse_proxy @phpFiles 127.0.0.1:9000 {
            transport fastcgi {
                split .php
            }
        }
    }

    file_server

    handle_errors {
        rewrite * /error.html
        templates
        file_server
    }
}

this will make all non *.php 404 works fine with handle_errors, but if user access urls like:

/this-is-no-fount.php
/sub1/sub2/sub3/xxxxx.php

since the request passed to php-fpm, so it respond a No input file specified. plain text error.

and the error did not passed to handle_errors handler.

but this, in nginx we can resolve this by add try_files $uri =404;

    # Pass the php scripts to fastcgi server specified in upstream declaration.
    location ~ \.php(/|$) {
        # avoid No input file specified
        try_files  $uri =404;

        include fastcgi_params;

        # the missing param in fastcgi_params copied from fastcgi.conf
        fastcgi_param  SCRIPT_FILENAME    $document_root$fastcgi_script_name;

        fastcgi_pass 127.0.0.1:9000;

        fastcgi_buffer_size   64k;
        fastcgi_buffers    8 256k;
        fastcgi_busy_buffers_size 256k;
    }

[francislavoie]

That's a good question. The simplest way to do this is to use the error directive: https://caddyserver.com/docs/caddyfile/directives/error

    route {
        # Add trailing slash for directory requests
        @canonicalPath {
            file {path}/index.php
            not path */
        }
        redir @canonicalPath {path}/ 308

        # If the requested file does not exist, try index files
        @indexFiles file {
            # try_files {path} {path}/index.php index.php
            try_files {path} {path}/index.php
            split_path .php
        }
        rewrite @indexFiles {http.matchers.file.relative}

        # Trigger error for non-existing PHP scripts
        @phpNotFound {
            path *.php
            not file
        }
        error @phpNotFound 404

        # Proxy PHP files to the FastCGI responder
        @phpFiles path *.php
        reverse_proxy @phpFiles 127.0.0.1:9000 {
            transport fastcgi {
                split .php
            }
        }
    }

You make good points about try_files missing =404 support, and index off not being quite sufficient to change that behaviour though.

I'll mark this as a feature request, I think it's worth investigating some better solutions to make this smoother.

[ttys3]

the error directive

Thanks, the error directive works like a charm -_- !

So the final workaround to solve this kind of works is:

1. Copy the Expanded form from the doc, and change

try_files {path} {path}/index.php index.php to try_files {path} {path}/index.php

this will avoid /non-exists-dir/non-exists/non-exists-file.php still served to index.php

2. Trigger error for non-existing PHP scripts

we need to add:

        # Trigger error for non-existing PHP scripts
        @phpNotFound {
            path *.php
            not file
        }
        error @phpNotFound 404

this will ensure that non-existing php files not proxied to php-fpm and thus, avoid the No input file specified. error and keep our handle_errors handler works again!

and, at last, waiting for php_fastcgi options been improved!

[ttys3]

For problem 1, I thought we can add an option and it is disabled by default, like this:

https://github.com/ttys3/caddy/commit/eef2a58e23ff212ccf499a2275bdb45f31919b57

for problem 2, because I'm not familiar with the caddy code, still got some time to figure out a way to add the "Trigger error for non-existing PHP scripts"

[francislavoie]

What I would like to see work is this:

php_fastcgi localhost:9000 {
    try_files {path} {path}/index.php =404
}

First thing is I'd like to make =404 work, but the problem is request matchers only return a boolean, and can't return an error, currently. It's probably possible to do it via pushing some data in the http.Request's context and have the handler chain check if the matcher resulted in an error, but I'm worried about harming performance by taking that approach.

And if that's implemented, then we can add try_files as an option for php_fastcgi which would just straight up override the try_files found in the expanded form. index off would still just turn off the whole rewrite behaviour, so they wouldn't really be useful together at the same time.

[ttys3]

Your implementation seems in a high level and more elegant, and what I think is can we keep the old implementation and simply add two options to php_fastcgi like:

php_fastcgi localhost:9000 {
    always_proxy_to_index off
    file_not_found_error on
}

always_proxy_to_index off is easy, and it will do try_files {path} {path}/index.php, we can change modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go (https://github.com/ttys3/caddy/commit/eef2a58e23ff212ccf499a2275bdb45f31919b57):

+               tryFiles := []string{"{http.request.uri.path}", "{http.request.uri.path}/" + indexFile}
+               if fcgiTransport.AlwaysProxyToIndex {
+                       tryFiles = append(tryFiles, indexFile)
+               }
                // route to rewrite to PHP index file
                rewriteMatcherSet := caddy.ModuleMap{
                        "file": h.JSON(fileserver.MatchFile{
-                               TryFiles:  []string{"{http.request.uri.path}", "{http.request.uri.path}/" + indexFile, indexFile},
+                               TryFiles:  tryFiles,
                                SplitPath: extensions,
                        }),
                }

and I don't know how to do file_not_found_error on

[francislavoie]

I'd rather not add such narrow-use options like that. I think it's confusing to users if there's too many options that do very specific things, rather than few options that can be used in a variety of ways.

I've written a PR https://github.com/caddyserver/caddy/pull/4346 for the first part, adding support for =404. I'm not in love with the implementation, but I don't see a much better way to do it.

[mholt]

I think this is resolved then? In #4346 and #4347 -- thanks Francis!