rryqszq4 / ngx-php

ngx-php - Embedded php7 or php8 scripting language for nginx module. Mainline development version of the ngx-php.
BSD 2-Clause "Simplified" License
582 stars 56 forks source link

Does not work as expected with "try_files" directive #116

Open Rifle-Rex opened 3 years ago

Rifle-Rex commented 3 years ago

Hello. I had found some issues when I trying to use "try_files" directive. Below are my nginx.conf

init_worker_by_php_block {
    class a {
        static $a = 1;
        public static function add_one(){
            a::$a += 1;
        }
    }
}
server {
    server_name localhost;
    listen 808;

    location / {
        index index.html index.htm;
        try_files $uri $uri/ /index.php;
    }

    location /index.php{
        content_by_php_block {
            ngx_header_set("Content-Type", "text/html; charset=UTF-8");
            echo "12312\n";
            a::add_one();
            var_dump(a::$a);
        }
    }
    location /favicon.ico{
        return 403;
    }
}

when I open "http://localhost:808/p", A blank page with status code 200 was returned. But when I open "http://localhost:808/index.php", Everything was just fine. And the code in "content_by_php_block" is seen not be run in "http://localhost:808/p" because the a::$a is not add after the request. Is there anything I misunderstanding?

rryqszq4 commented 3 years ago

@Rifle-Rex Thanks for your report, and I will track this problem.

joanhey commented 3 years ago

I can confirm that try_files don't redirect if file not found. Only return a blank page with status 200.

And in my case, that I don't use a uri location redirection (best practice) only send a blank 200.

location / {
        index index.html index.htm;
        try_files $uri $uri/ @php-ngx;
}

location @php-ngx {
        content_by_php_block {
            ngx_header_set("Content-Type", "text/html; charset=UTF-8");
            echo "12312\n";
        }
}

This is necessary to use the Front Controller pattern, that use almost all frameworks.

An app can use a static dir for the static files, but some need to be in the root dir, like: favicon.ico, robots.txt, ...
And we need to also add all that uri locations. Normally is better add that uris, to configure cache, headers, log, ... but the try_files need to work too.

Qwoker commented 10 months ago

@Rifle-Rex Thanks for your report, and I will track this problem.

Havent update this issue?

joanhey commented 10 months ago

I don't know, but I'll try to add tests for that first, and later try to fix it.

~PD: I need to read better this issue, and try it because it don't seems normal for me.~