Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
We've introduced .htaccess files to prevent access to images and content. We only need to introduce one file and we could test using the testing profile for speedier tests.
Proposed resolution
Do it
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#27 | 2942769-followup.patch | 1008 bytes | neclimdul |
#21 | interdiff-19-21.txt | 2.36 KB | Anonymous (not verified) |
#21 | 2942769-21.patch | 5.24 KB | Anonymous (not verified) |
#19 | 2942769-19.patch | 5.25 KB | alexpott |
#19 | 12-19-interdiff.txt | 3.01 KB | alexpott |
Comments
Comment #2
alexpotthere's a patch. We also get additional protection since we won't serve core/profiles/demo_umami/modules/demo_umami_content/default_content/articles.csv either. Plus the test will be way way quicker. A single test and using the testing profile since install Umami is not required for testing this.
Comment #3
Eli-THave applied patch in #2
Patch reviewed - looks good.
Tested umami still installs and the recipes and articles are still created.
Have tested the following paths successfully return a 503 response:
/core/profiles/demo_umami/modules/demo_umami_content/default_content/images/chocolate-brownie-umami.jpg
/core/profiles/demo_umami/modules/demo_umami_content/default_content/articles.csv
/core/profiles/demo_umami/modules/demo_umami_content/default_content/articlesNONSESNE.csv
/core/profiles/demo_umami/modules/demo_umami_content/default_content/recipes.csv
/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/lets-hear-it-for-carrots.html
Therefore putting this to RTBC.
Comment #4
Gábor HojtsyComment #7
Gábor HojtsyMakes total sense, thanks! Committed to 8.6 and backported to 8.5.
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedRandom fails:
Comment #11
alexpott@vaplas thanks for reporting - Ho hum that's weird how we got a pass. But this is a module discovery issue.
Comment #12
alexpottHere's the fix. Also going to create an issue about the 8.6.x module listing behaviour. Being able to discover modules from not installed profiles is a serious bug.
Comment #13
Gábor HojtsyDuh this is the nth time the new module discovery is biting us. (At least we are working out the bugs now I guess).
Comment #14
alexpottYep totally!
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous commentedThe problem with fail on 8.5.x is resolved. @alexpott opened #2945306: The new ModuleList in 8.6.x can discover modules in tests the previous code could not. to address module discovery problem. So, back to RTBC.
Comment #16
Gábor HojtsyApparently some stuff I committed in the meantime, sorry :/
Comment #17
smazInterdiff was empty, patch is the same, just fixes:
Hunk #1 succeeded at 163 with fuzz 1.
Comment #18
smazHmm, my patch seems to miss moving /core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/.htaccess.
Comment #19
alexpottHere's another approach. What we really want to ensure is that these files are not accessible regardless of the whether umami is installed on not. So let's just hardcode the path.
Interdiff back to 12.
Comment #21
Anonymous (not verified) CreditAttribution: Anonymous commentedAnother approach looks nice, but there are a couple of proposals:
Move to assert
sonse/sinse
merge in one variable
Do not convert to OS-independent path. In Windows this assert will work too. A problem only arises when a straight string comparison of two paths as is. Example.
Done.
Comment #22
alexpottThanks @vaplas that looks really nice! +1
Comment #23
borisson_Looks super solid, great work @vaplas!
Comment #26
Gábor Hojtsy"should not *be* used" (be missing AFAIS). Fixed that on commit.
Thanks all!
Comment #27
neclimdulcommit on fix added whitespace to the end of the line and broke phpcs testing ;)
I hate the one character followup issue but I'll create it if you want. Here's the patch if you just want to fix it.
Comment #28
alexpottCommitted and pushed 097d71c2fd to 8.6.x and 9a4e8a9a98 to 8.5.x. Thanks!
Fixed the CS issue.
Comment #31
Gábor HojtsyDuh I have no idea how did that happen. The committer script did not let me commit due to the CS fail so I fixed the CS fail locally and then the committer scripts let me commit (without a CS fail). Oh well.
Comment #32
alexpott@Gábor Hojtsy that happens because you didn't
git add
your updates. I need to work on https://github.com/alexpott/d8githooks/pull/1