Fixes #928 Prevent corruption of htaccess file on update

[remyperona]

Description

Update the flush_rocket_htaccess() function to use insert_with_markers() function instead of custom code to update the htaccess file content with WP Rocket rules.

Fixes #928

Type of change

• Bug fix (non-breaking change which fixes an issue)

Is the solution different from the one proposed during the grooming?

No

How Has This Been Tested?

• Manual tests

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• My changes generate no new warnings
• Existing unit tests pass locally with my changes

[CrochetFeve0251]

@Tabrisrp tests are failing

[Mai-Saad]

@Tabrisrp Thanks for the PR.
As per 1st round of fresh install smoke there, @piotrbak here are my notes, please let me know which point(s) needs to be changed if any
1- we write to htaccess just after activation without visiting our settings (on the trunk, we write after visiting settings) => I think we are good here
2- we add our WPR part after what is existing in the file (on the trunk, we add our code at the beginning of the file)=> are we ok here
3- Our rewrites, 1st line not containing the version but exists in another line (on the trunk it exists on both lines) => is this ok?
4- After deactivating WPR, we leave part (comment) from our rewrite (I think we shall remove that)

[piotrbak]

The 2nd and 4th points look like something we should take a look at

[engahmeds3ed]

@Mai-Saad @piotrbak
When using insert_with_markers, and the begin/end comments are not there, it adds the needed rules at the end of the htaccess after everything there.

Also when we deactivate WPR, we are calling insert_with_markers with empty content so it adds the begin/end comments with the note.

So, this is expected from our side and to fix the points u mentioned I need to use the old rocket_put_content to remove the left comments with deactivation and add them again before WP markers when activating.

[remyperona]

Modifying the file ourselves to remove the leftover comments would be a risk to get back to the issue we're trying to fix. Those 2 points are a change compared to before, but should have no impact on the functionality itself.

[engahmeds3ed]

Modifying the file ourselves to remove the leftover comments would be a risk to get back to the issue we're trying to fix. Those 2 points are a change compared to before, but should have no impact on the functionality itself.

Exactly, that's why I mentioned here first without implementing it.
Do u think having WPR rules below WP core rules will be a problem like not serving the html cache file as the rule of redirecting everything to index.php?

[MathieuLamiot]

I think rewrite rules are order-sensitive in htaccess ; aren't we using some?

The idea of our fix was to leverage WP Core implementation as we think it is safer than what we have currently in the plugin. But if that implementation does not fit exactly our need, would you consider duplicating insert_with_markers and creating a customized version? Looking at the implementation, we could easily change the ordering to meet our needs.
Duplicating code is rarely a good idea, but since htaccess is a sensitive topic, it might actually be better that we have full control of what we do, no? This way we could benefit from the robustness of this implementation, manage ordering (and the comments leftovers) and be independent from whatever WP Core could do with this function someday?

[Mai-Saad]

@Tabrisrp Thanks for the updates, Currently WPR rules are added at the beginning of the file same as on trunk. However,
1- When we deactivate WPR we leave the begin/end comment of WPR in the file (on trunk, we remove that) => The impact here, is that if we activated WPR on the site with those comments while not having permissions on htaccess, the warning about missing permissions won't be displayed
2- Note: If we enabled our helper to remove our rules, only the comment will be there without our version in the comment (same as point 3 here)
@piotrbak do we need further changes here?

[piotrbak]

@Tabrisrp Do you see why the comment is preserved? I'm thinking about any other possible conflicts there, it would be good to fully clean the files after deactivation

[remyperona]

Updated the code to completely remove the markers when deactivating

[Mai-Saad]

Working as expected now and markers have been removed