@Findus23 opened this Pull Request on February 16th 2021 Member

It seems like the full createFilesForSecurity that also creates the files for IIS is only run on install and some updates and not on the normal update.

➜  ~/public_html/matomo git:(4.x-dev) ✗ rg "createFilesForSecurity"
core/Updates/4.0.0-b1.php
261:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/3.13.4-b1.php
21:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/3.0.0-b4.php
56:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/2.16.3-b1.php
20:        ServerFilesGenerator::createFilesForSecurity();

core/Updates/3.0.1-b1.php
22:        ServerFilesGenerator::createFilesForSecurity();

plugins/Installation/Controller.php
313:        ServerFilesGenerator::createFilesForSecurity();

plugins/Installation/ServerFilesGenerator.php
17:    public static function createFilesForSecurity()

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@sgiehl commented on February 17th 2021 Member

@Findus23 When doing that in updater, guess we could remove it from the single update scripts. Wouldn't make sense to do it multiple times when updating from an older version.

@Findus23 commented on February 17th 2021 Member

I'm not sure if modifying historic update scripts is worth it if the worst case is creating the same file twice after each other.

@sgiehl commented on February 17th 2021 Member

Don't have a strong opinion on that. That just came up to my mind when reviewing... It would only remove some unneeded operations. But should also be fine to keep them 🤷

@tsteur commented on February 17th 2021 Member

Be fine to keep them as the operation is quickly done and not happening too often

This Pull Request was closed on February 22nd 2021
Powered by GitHub Issue Mirror