fpm: Implement access log filtering

[maaarghk]

Fixes GH-8174, #80428

Before merging I think it could be doing with more tests:

• check the config parser fails in sensible ways
• check the config dumper outputs what it is supposed to
• check the request method / body filtering works

[bukka]

From the very quick look, it's looking good. I was initially thinking something more in line with listen.allowed_clients but this is actually nicer IMO.

[maaarghk]

@bukka do you happen to have any POST-doing tester.inc versions lying around somewhere to save me a bit of time?

[bukka]

do you mean something like

$tester->request('', ['REQUEST_METHOD' => 'POST'])->expect...

[maaarghk]

nah I meant actually passing STDIN, wasn't too hard to work out anyway so just pushed it up and think I'm done with this

[maaarghk]

Just popped into my head here whilst thinking about something unrelated, but - since proc->request_uri actually gets set to script_name, because that is what's read into PHP_SELF sapi->register_variables - is it the right thing to check? I know at least on servers I admin I put %{REQUEST_URI}e in the access.format for this reason. From the perspective of fpm_log.c should %r / request uri be redefined from proc.request_uri to coalesce(fcgi_getenv(REQUEST_URI), proc.request_uri)? Could be overthinking it of course. Probably makes sense for health check scripts to be separate PHP files that don't go through routers and whatever anyway.

[bukka]

Looks mostly good.

[bukka]

Just popped into my head here whilst thinking about something unrelated, but - since proc->request_uri actually gets set to script_name, because that is what's read into PHP_SELF sapi->register_variables - is it the right thing to check? I know at least on servers I admin I put %{REQUEST_URI}e in the access.format for this reason. From the perspective of fpm_log.c should %r / request uri be redefined from proc.request_uri to coalesce(fcgi_getenv(REQUEST_URI), proc.request_uri)? Could be overthinking it of course. Probably makes sense for health check scripts to be separate PHP files that don't go through routers and whatever anyway.

I would probably default to request_uri as it might be better to later add another option to specify path pattern where it could be possible to set how to get the path in a similar way like it is for the actual log pattern.

[bukka]

@maaarghk Just a reminder that a feature freeze is on 5th July so it would be good to add this potentially. The comments are just NIT's so will try to give it a try over the weekend and see if we can merge it and I can additional fix them possibly. Although if you want to fix them before that, it would be appreciated!

[maaarghk]

@bukka I noticed the bot removed SAPI: fpm label from this, just flagging it incase that makes it drop off your radar

[bukka]

Thanks, bot should be now fixed. Will try to give it a try tomorrow.

[bukka]

If you could squash it to a single commit in the meantime, that would be great!

[bukka]

I tried the changes and all looks good to me. Just added few minor suggestions that you are free to ignore if you are too busy. I will merge it in the next few days.

[bukka]

One correction that the feature freeze is actually on 19th. I confused it with a deadline for RFCs so there is a bit more time.

[bukka]

Merged in 327bb21 . Thanks!