Add a missing pipe between .orig and .save extentions

CommentFileSizeAuthor
#11 2779833-11.patch8.91 KBpoker10
#11 2779833-11_test-only.patch8.13 KBpoker10
#7 2779833-7.patch9.13 KBpoker10
#7 2779833-7_test-only.patch8.35 KBpoker10
#4 htaccess_missing_pipe-2779833-4.patch811 bytesamar.deokar
#2 htaccess_missing_pipe-2779833-2.patch715 bytesiryston

Comments

iryston created an issue. See original summary.

iryston’s picture

iryston commented
StatusFileSize
new htaccess_missing_pipe-2779833-2.patch715 bytes
iryston’s picture

iryston commented
Issue summary: View changes
amar.deokar’s picture

amar.deokar commented
StatusFileSize
new htaccess_missing_pipe-2779833-4.patch811 bytes

Updated patch for drupal-7.59 .

cilefen’s picture

cilefen commented
Status: Patch (to be ported) » Needs review

The last submitted patch, 2: htaccess_missing_pipe-2779833-2.patch, failed testing. View results

poker10’s picture

poker10 commented
Parent issue: » #2508666: Drupal 8 .htaccess rule to prevent php file access can be easily bypassed
Related issues: +#1587270: Forbid execution of PHP files in subfolders by default (except those needed by core)
StatusFileSize
new 2779833-7_test-only.patch8.35 KB
new 2779833-7.patch9.13 KB

As mentioned in the parent D8 issue, there were new tests introduced which needed to be added also to D7.

The patch #4 looks good and does exactly the same fix to FilesMatch as #2508666: Drupal 8 .htaccess rule to prevent php file access can be easily bypassed. I have added the tests from the mentioned issue, so we should have this coverage in D7. The patch itself is unchanged (other changes in D8 issue are not currently in D7, see: #1587270: Forbid execution of PHP files in subfolders by default (except those needed by core)).

Let's check the testbot.

The last submitted patch, 7: 2779833-7_test-only.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 7: 2779833-7.patch, failed testing. View results

poker10’s picture

poker10 commented

Hm, it seems like there is a problem with the patch or with the testbot with the new created files (probably returning 404 instead of 403). It applies locally and test passes locally. I have to look into it more closely.

poker10’s picture

poker10 commented
Status: Needs work » Needs review
Related issues: +#2332029: Add test coverage for .htaccess rules
StatusFileSize
new 2779833-11_test-only.patch8.13 KB
new 2779833-11.patch8.91 KB

Let's try this with 'external' => TRUE (still passes locally).

The last submitted patch, 11: 2779833-11_test-only.patch, failed testing. View results

mcdruid’s picture

mcdruid commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +RTBM

If we were starting from scratch I might ask whether we need to actually add all of the test files. Could we either generate them on-the-fly or just verify that we get a 403 when apache blocks the requests rather than a 404 when the file is not there?

However, this is how it was done in D8 ... and it's not wrong.

The backport looks good, the additional tests are great - without them it's nasty editing the long condition in the FilesMatch.

Thanks!

  • poker10 committed e887237 on 7.x 
    Issue #2779833 by iryston, amar.deokar, poker10: Fix Drupal 7 .htaccess...
poker10’s picture

poker10 commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -RTBM

Thanks all!

solideogloria’s picture

solideogloria commented
Status: Fixed » Needs work

The tests added for this issue include "system/test/fixtures/HtaccessTest/access_test.module" and similarly-named files, such as an info file.

After updating to Drupal 7.92, upon navigating to /admin/modules, I see an errored (unavailable) "access_test" module in the list that says "This version is not compatible with Drupal 7.x and should be replaced."

poker10’s picture

poker10 commented
Status: Needs work » Fixed

Thanks for reporting this @solideogloria! It seems like that the packaging script added some metadata to the originally empty .info file, which caused that the module is now displayed. This problem is not present in the 7.x-dev, as there are no metadata added.

I have created a follow-up issue here: #3308466: [D7] HtaccessTest - access_test module displayed in the modules list, where we can fix that (changing the status of this issue back to Fixed).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.