paketo-buildpacks / php-nginx

PHP Nginx Cloud Native Buildpack
Apache License 2.0
4 stars 4 forks source link

NGINX Root Location #19

Open fg-j opened 2 years ago

fg-j commented 2 years ago

Linking to an issue that was lingering on the archived php-web repo: https://github.com/paketo-buildpacks/php-web/issues/207

dvigueras commented 2 years ago

I was deploying a Laravel app with paketo-buildpacks/php and BP_PHP_SERVER = nginx and then I realized that URLs were not working (nginx returns 404).

I had to create a .nginx.conf.d/location-server.conf file with the following content:

location / {
    try_files $uri $uri/ /index.php?$args;
}

Redirecting every request to the index.php is a common practice in many PHP Frameworks/CMS like Laravel, Symfony, WordPress, etc.

Since this piece of config is very common maybe it would be a good idea to add a new environment variable, something like BP_PHP_ENABLE_ROOT_TRY_INDEX_PHP (example name, it could be other), which would add this location config block. The default value would be false, so it is backwards compatible and won't break any existing scenario.

I would like to know what you think about this change. If you are open to merge something like this I can work on this and I would send a PR with the changes needed.

sophiewigmore commented 1 year ago

@dvigueras this seems like a reasonable thing for the buildpacks to do. I wonder if there is a way to introduce this functionality without needing an environment variable.

Could you clarify what your app structure looks like that you needed to add that configuration?

dvigueras commented 1 year ago

Hi @sophiewigmore

I'm building a container with a Laravel application (a blog). I'm using the paketobuildpacks/builder:full builder and the paketo-buildpacks/php buildpack.

Laravel needs an nginx directive (https://laravel.com/docs/9.x/deployment#nginx) to direct the incoming requests where the requested file doesn't exist to the index.php.

location / {
        try_files $uri $uri/ /index.php?$query_string;
    }

This way the Framework can intercept the request and the look for the requested route in the list of routes defined in the Laravel application.

e.g.: nginx receives an incoming request for the /login path. a login file doesn't exist in the filesystem, so nginx passes the request to the index.php file. Then this request is forwared to php-fpm and therefore Laravel is able to process the request. Laravel matches /login with an entry in the application route list and finds which Framework Controller will process the request.

If Laravel is not able to match the requested path then it will return the 404 Framework page.

I've been able to achieve this behaviour with a custom .nginx.conf.d/location-server.conf file. So I thought that this seems to be a very common scenario that would be interesting to be "officially supported".

dvigueras commented 1 year ago

Also I've noticed of another issue with my Laravel project and the current implementation of the buildpack.

In my project I am using Livewire (https://laravel-livewire.com/). This library adds a route to /livewire/livewire.js which is processed by a Livewire PHP Controller.

What is happening right now in nginx is that the incoming request is matching this location block.

location ~* \.(?:ico|css|js|gif|jpeg|jpg|png)$ {
    expires         max;
    add_header      Pragma public;
    add_header      Cache-Control "public, must-revalidate, proxy-revalidate";
}

instead of my location / block. Then nginx tries to serve an existing livewire/livewire.js file which doesn't exist and then it returns 404. This happens because nginx location precedence matches the previous block first.

A possible idea would be to add a new environment variable to be able to disable this location block (it would be similar as the work done with the BP_PHP_ENABLE_HTTPS_REDIRECT env var / DisableHTTPSRedirect variable)