Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add update hook to update .htaccess in files directory on existing sites #19

Open
wants to merge 1 commit into
base: 6.x
from

Conversation

Projects
None yet
2 participants
@dsnopek
Copy link
Contributor

commented Oct 4, 2018

This is to fix #16

I'm a little unsure about this. I've never customized the .htaccess file in the files directory, but I wonder if others might have? I don't want to wipe out their changes, but I don't want to leave sites on PHP vulnerable to attack.

@DrupaListo-com

This comment has been minimized.

Copy link

commented Oct 26, 2018

99% sure nobody customizes this .htaccess file BUT if we want to be 100% sure we could do the following safer procedure:

  • make a list of known D6 sites/default/files/ .htaccess files md5 hashes -> we can take all D6 git revisions of file.inc where file_htaccess_lines() is defined, run a loop where we include all file.inc's revisions one by one, check if file_htaccess_lines() exists in it and call it and calc the string output's md5

  • then in the update hook we can load those md5-s and read the current .htaccess file and check if current .htaccess file's md5 sum matches any from the list of know ones...
    If YES - then it's safe to overwrite.
    if NO - then it's not safe and we could do nothing OR for bonus points - we can set a drupal variable show_htaccess_warning and if that's set - put code in some module that'll show system message on all admin pages or any site page or just show a msg in the drupal status report.

Sounds like a plan?

PS. If file.inc does not define file_htaccess_lines() right from the start and that function - to put the htaccess file in place - was done in some other way - we need to further analyze. If it was done in the same way all the time - we're fine. Finally if file_htaccess_lines() func did not exist and then appeared - we're still fine. Don't know if I'm making sense - but the overall idea is to take al possible .htaccess contents that drupal ever defined and consider those known good .htaccesses that are safe to overwrite if we got them still present on our site...

by the way if we're lucky the md5sum might be just ONE.

@DrupaListo-com

This comment has been minimized.

Copy link

commented Oct 26, 2018

okay using git log + git show and a bit of manual search and replace I managed to isolate ALL possible htaccess file contents provided by D6 at any momment in time.

What I did:

  1. write all revisions of includes/file.inc glued into a file.

// c - commit hash
// f - file contents

git log --follow --oneline -- "includes/file.inc" | awk '{print $1}' | while read c; do f=git --no-pager show $c:includes/file.inc; if echo "$f" | grep htaccess >/dev/null; then echo "$f" ; fi; done > tmpfile.php

it's slow inefficient maybe but runs in a second and gets the job done.

  1. Then one has to run through that 2.8 MB file with search for "/.htaccess" as just htaccess gets too many results, then I found out recently in the last 4 revisions a function named file_htaccess_lines() was added - so I found all 4 definitions of it and copied the htaccess text, then deleted the function definitions to not stand in the way.
    Then I compared the 4 variants and found out via md5sum-ing that it's 2 uniq variants.

Then I found that before that function htaccess contents were written from within/by a function named file_check_directory() towards the end - where the htaccess string was hardcoded.
I found all unique instances of these writes / strings , one instance was used in 61 revisions - exactly the same - so I selected it once and added it to my extractions file. Then selected the PHP line with 1-2 lines of context and ran a replace-all search and replace to get rid of those strings containing .htacess in them..

rinse and repeat pumping F3 for .htaccess, until there's nothing left.

@DrupaListo-com

This comment has been minimized.

Copy link

commented Oct 26, 2018

finally here's the end result with all variants of htaccess file's contents we can safely overwrite:
uniq-htaccess-files.txt

@DrupaListo-com

This comment has been minimized.

Copy link

commented Oct 26, 2018

next step is to either put those as values in a PHP array or generate the hashes of the variants and use those in the update hook...

There are 6 variants.

When diffing we must take into account possible empty lines in the start/end of the diff src and dest, so we must either diff as text after trimming both src and dest... or generate like 2N md5sums for each variant where N = number of empty lines added to start/end...

I hope the idea is clear and the heavy lifting was to extract the htaccess variants as they were in different flavours and functions.

@dsnopek

This comment has been minimized.

Copy link
Contributor Author

commented Oct 26, 2018

Thanks for all your work on this!

then it's not safe and we could do nothing OR for bonus points - we can set a drupal variable show_htaccess_warning and if that's set - put code in some module that'll show system message on all admin pages or any site page or just show a msg in the drupal status report.

I wonder if the hook_requirements() that you mentioned could just look for the specific lines about mod_php7 in the .htaccess and give a warning in case they are missing (as opposed to setting some variable)? That would also help in the case that someone updated Drupal, but forgot to run the drush updb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.