handle deprecated http2_push_preload conf for nginx >= 1.25.1

[OpJePl44tsm4n]

Updated the config so it will work for fresh installs.

removed http2 and http2_push_preload because they are deprecated in latest Nginx versions.

[drbyte]

I agree that this is the correct fix.

Unfortunately it only works if the Nginx version is 1.25.1 or greater, which is quite new. The changed syntax is not backward-compatible. (Ref: nginx docs )

2023-08-15 | nginx-1.25.2 mainline version has been released.
2023-06-13 | nginx-1.25.1 mainline version has been released.

If someone is using (or is forced to use) an older Nginx version, they will get the following anytime they run valet secure or re-secure their sites with valet tld, and Valet will refuse to work until the old syntax is restored to all the "secured" Sites configs.

  Nginx cannot start; please check your nginx.conf [1: nginx: [warn] protocol options redefined for 127.0.0.1:443 in ~/.config/valet/Nginx/foo.test:9
  nginx: [emerg] unknown directive "http2" in ~/.config/valet/Nginx/foo.test:15
  nginx: configuration file /usr/local/etc/nginx/nginx.conf test failed
  ].

@mattstauffer how do you feel about incorporating nginx-version-detection so we can handle version-specific stub file handling?

[OpJePl44tsm4n]

@drbyte thank you for your response, I was thinking about that. Without a mayor version bump, probably a version check would be needed, since installing Valet requires the latest nginx version by default.

I'll take a look and will update the PR.

[drbyte]

At least the old syntax is not throwing Fatal errors and preventing Nginx from starting.
Unfortunately the new syntax does throw fatal errors on older Nginx versions. 😢

[OpJePl44tsm4n]

@drbyte true but fresh installs will require lates version of nginx so now it's broken as is.
option is to cap the nginx version in the install command and update this part.

$this->brew->installOrFail('nginx', []);

my suggestion is to add a check in Site.php to check the nginx version and require
secure.valet.outdated.config for the older versions. This would make it work for all versions.

I'll make a fix so you can check it.

[drbyte]

Since ltrim() technically doesn't support passing a "phrase" (it treats it as a bunch of characters, not a phrase), perhaps simply str_replace() might be better?

[drbyte]

Looks good. Thx. 👍

[drbyte]

@OpJePl44tsm4n perhaps also rework the Title of this PR slightly to incorporate "http2" into it?
Maybe something like "handle deprecated http2_push_preload conf for nginx >= 1.25.1" ?

[drbyte]

@mattstauffer @driesvints I think this is ready for merge.

Of course, feel free to test on your local machine before+after upgrading nginx via brew

[mattstauffer]

@drbyte thank you so much for reviews and back-and-forths! Again, apologies for my delay here. @OpJePl44tsm4n thank you for this PR!

Testing now, and will merge if it doesn't bonk things for me.