Support from Acquia helps fund testing for Drupal Acquia logo

Comments

iryston created an issue. See original summary.

iryston’s picture

iryston’s picture

Issue summary: View changes
amar.deokar’s picture

Updated patch for drupal-7.59 .

cilefen’s picture

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

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

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.

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

mcdruid’s picture

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

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

Thanks all!

solideogloria’s picture

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

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.