Automatic creation of .htaccess files can cause errors

So this one is coming as the "One" for bugsmash so thought I'd give it a quick look.

Took the patch in #82 and only slightly fixed up with today's checks: Constructor promotion, return types, deprecation version.

#87 I agree and changed that to an error()

Also tweaked the CR some but it was pretty straight forward.

Still NW for

The new setting needs to be added to settings.php with a comment.

Rebased on 11.x and added docs to default.settings.php

Settings read well to me.

16 years and this one may be good

Back to NR for the feedback from @acbramley

I reviewed the MR and added some comments.

I also think this still needs an issue summary update, because these parts from IS are not implemented:

• When Apache is reported as the web server, automatically create .htaccess files. Otherwise, do not create them and warn the user about the possible implications of this
• New warning message in File System section of Status Report.

When manually tested this, I was unable to find the warning entry in the log (Auto-creating htaccess disabled.), as the file creation is blocked in the parent function (HtaccessWriter::ensure()), so if the auto_create_htaccess config is set to FALSE, it will never reach the code in HtaccessWriter::write(). Not sure it was the intent. Personally I think that we do not need this logging at all and it will be sufficient to create a follow-up to sort-out the errors in the status report, which are displayed if .htaccess files are missing and the auto_create_htaccess config is set to FALSE:

Private files directory - Not fully protected
See https://www.drupal.org/SA-CORE-2013-003 → for information about the recommended .htaccess file which should be added to the private:// directory to help protect against arbitrary code execution.

I think it does not make sense to have it as errors anymore, when you use this new switch and disable it's creation on purpose.

Moving to NW for these.

Thanks!

Addressed feedback and updated the IS.

I think we can create a new requirements check for the config setting. However, there was some pushback (#44_) on showing warnings on a standard install, so we need to be sure this is considered.

However, there was some pushback (#44_) on showing warnings on a standard install, so we need to be sure this is considered.

Yes, I was thinking more about adjusting the existing error messages, which are displayed when .htaccess files are missing, so that these have a different wording when the auto_create_htaccess config is set to FALSE, or hide these messages entirely. I agree that adding a new requirements check with another warning or so is not ideal.

Believe feedback has been addressed here.

If we need a follow up ping me and I’ll happily do that