fix FPs

[Xhoenix]

FPs at PL 1.

[azurit]

@GenialHacker Hi and thanks! Can you look at my comment in the code?

[Xhoenix]

Hi @azurit, you're right! I don't remember why I did that, but I changed it as you said.

[azurit]

@GenialHacker Cool! I added one last comment.

[azurit]

@GenialHacker Sorry i noticed one more thing: We need to move all rules into phase 2 because of the TX variables (for plugins, they are not initialized in phase 1).

[Xhoenix]

Yeah, I also thought about that when writing those rules. But, if you look at my last PR #11, then you'll notice that rule causing this FP, i.e rule 911100 was moved to phase 1 three years ago. So, the FPs won't stop if you move the rules into phase 2.

[Xhoenix]

If you want me create a PR moving the rule 911100 to phase 2, then I'll be really glad to do that. :)

[azurit]

But all of them should be back to phase 2, see this: https://github.com/coreruleset/nextcloud-rule-exclusions-plugin/pull/3/files

@theseion Any suggestions how to resolve this?

[Xhoenix]

Looks like my last PR only fixed two of those and the rest also needs to be put back into phase 1 as those were move into phase 2 in PR #3.

[azurit]

It won't help as TX variables are initialized in phase 1 AFTER the plugins are executed, so changes done by plugins gets overwritten.

[azurit]

One possible solution for this problem would be to not modify TX variables but exclude the whole rule using ctl:ruleRemoveById.

[Xhoenix]

I can't see how that's going to help. I always thought setvar:'tx.allowed_methods is the right and elegant solution. Anyways, it's @theseion's call.

[theseion]

Oh boy. As I discovered in your other PR @azurit, at least some of the rules are actually required to run in phase 1.

I think we need to fix this properly, once and for all. The way to do that, I think, is to move most rules into a new *-after.conf file. That will ensure that variables have been initialised. Does that sound reasonable @azurit?

[azurit]

@theseion Will it really help for example for rule 911100 which runs in phase 1?

[theseion]

No, I just realised that too. If we want to be able to do this like we want to, then we will need a mechanism to run the rule after initialisation but before method enforcement rules.

Let me open up a separate issue to discuss this, as it affects the entire plugin system.

[theseion]

As the current rules are running in phase 1 at the moment and we don't have an immediate fix for the issue, I suggest that you still merge this PR, targeting phase 1. If we then start moving things around those rules will be included in the move.

[Xhoenix]

I think we should first change other rules that were moved to phase 2 in PR #3. Or those are okay with you guys?

[azurit]

If we want to be able to do this like we want to, then we will need a mechanism to run the rule after initialisation but before method enforcement rules.

We were already discussing this but @dune73 was not happy with this solution because it will complicate the CRS includes/installation. But i agree that this is the best option.

[azurit]

I think we should first change other rules that were moved to phase 2 in PR #3. Or those are okay with you guys?

Yes, for me.

[Xhoenix]

@azurit Can you look into this PR?

[azurit]

Looks good, merging!