Problem/Motivation

A new test (HtaccessTest::testFileAccess()) was introduced in 7.92 release. It includes a set of testing files including a access_test.info file. All these files are empty, but the packaging script automatically added information to the access_test.info, which caused this module to display in the list of modules.

------------------------

Original report from @solideogloria:

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."

------------------------

Steps to reproduce

Install Drupal 7.92 and go to "admin/modules".

Proposed resolution

We have three options:

1. Remove the check for the .info extension / access_test.info file (this will cause that the new test will be incomplete, e.g. not checking all blocked extensions by .htaccess).

2. Add at least hidden = TRUE to the empty access_test.info file to hide it from the list of modules.

3. Remove the access_test.info file from the modules/system/tests/fixtures/HtaccessTest directory entirely. Requesting the non-existing file will still return 403 (see .htaccess). This approach was also proposed here: #2779833-13: Fix Drupal 7 .htaccess to protect .orig and .save files from view . All other files could be removed as well, because they are practically not needed (except for the access_test.php-info.txt file, which should be kept as it should return HTTP status 200). This is the preferred option.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#14 drupal-generate-files-3308466-14.patch6.83 KBsolideogloria
#3 3308466-3_without_files.patch5.87 KBpoker10
#3 3308466-3_generate_files.patch1.54 KBpoker10
access_test.jpg32.31 KBpoker10

Comments

poker10 created an issue. See original summary.

solideogloria’s picture

solideogloria commented

I don't know how the packaging script works. But it might also be an option to add an exception specifically for these files?

poker10’s picture

poker10 commented
Status: Active » Needs review
StatusFileSize
new 3308466-3_generate_files.patch1.54 KB
new 3308466-3_without_files.patch5.87 KB

@solideogloria possibly yes, but I do not think it is worth the effort. Any such exemption can cause problems in the future.

Adding a patch for the option no. 3 (3308466-3_without_files.patch). But this has a disadvantage, that we cannot remove the whole directory, as the .txt file should be present to return HTTP status 200.

So uploading also a second path with another approach - it generates files directly in the test (3308466-3_generate_files.patch). This was also suggested by mcdruid in the original issue. Maybe this could fit the best, but will see.

Just a note to the second patch - # test content is intentional, so it works also with the .htaccess being created. And I have not put a drupal_unlink() there, because the whole directory is removed when the test finishes.

poker10’s picture

poker10 commented

Oh, it seems like the 3308466-3_generate_files.patch is missing the deletion of the whole modules/system/tests/fixtures/HtaccessTest directory. If we decide that this is a way to go, I will update the patch. But it will not affect the test results, as the generated files are currently created in the public://.

solideogloria’s picture

solideogloria commented

I like the idea of generating the files. The code change is straightforward and the test coverage remains the same.

long.skinny.boy’s picture

long.skinny.boy commented

Good afternoon, is the presence of this module in administration not critical for the operation of the system? That is, can I safely upgrade to 7.92 from 7.91?

poker10’s picture

poker10 commented

@long.skinny.boy this is only a cosmetic issue, it should not block the update for you.

However I recommend to check this issue: #3310081: javascript links in contrib broken since 7.92 and the corresponding change notice, to check if you are not affected (especially if you are using a Views Slideshow Simple Pager submodule from the Views Slideshow module).

long.skinny.boy’s picture

long.skinny.boy commented

@poker10 Thank you for the quick response, I will pay attention to your warning

jmizarela’s picture

jmizarela commented

Hey @poker10, your patch 3308466-3_without_files.patch worked for me smoothly, but 3308466-3_generate_files.patch didn't (ever after a cache clear) what is, well, expected as you said in #4... As "without files" covers the preferred option (3), this is (a little) RTBC, but I wanted to understand better (if you know about it) why access_test.info was included on 7.92 and how I could help test this more thoroughly.

poker10’s picture

poker10 commented

@jmizarela can you please describe in more detail what doesn't work in 3308466-3_generate_files.patch? The patch should work (it passes the tests) and my comment in #4 only mentioned that the old directory with files was not deleted. But the patch is using another directory (public://) for the dynamic generation of the files, so it should work.

For the reasons why access_test.info was included in 7.92 see the parent issue: #2779833: Fix Drupal 7 .htaccess to protect .orig and .save files from view. There was a typo in htaccess causing that the FilesMatch was not working correctly for all intented extensions. A new test was introduced together with the fix - and the test is checking access for all extensions blocked by htaccess.

jmizarela’s picture

jmizarela commented

Hey there @poker, thanks for the answer on access_test.info, it was helpful.
About 3308466-3_generate_files.patch, it doesn't seem to break tests or anything, but applying it didn't fix (at least for me) this issue, as access_test is still present on the Modules list, and still with the accompanying description of "This version is not compatible with Drupal 7.x and should be replaced.". That is the reason that led me to believe it is not suitable, as of now, as a solution for the reported problem, what was something 3308466-3_without_files.patch did right away. Please inform me if I missed any additional steps.
Cheers.

poker10’s picture

poker10 commented

@jmizarela Yes, you are right, sorry for the misunderstanding. The module is still listed on the Modules list page, because the directory is not deleted by the 3308466-3_generate_files.patch patch. I will discuss these two approaches with @mcdruid and once we choose the best one, we will either update the 3308466-3_generate_files.patch patch or continue with the second one to fix this. That means we cannot switch this to RTBC now, as we need to select one approach. Thanks!

mcdruid’s picture

mcdruid commented

I like 3308466-3_generate_files.patch so would be good to see that combined with removing the files / directory for the complete fix.

solideogloria’s picture

solideogloria commented
StatusFileSize
new drupal-generate-files-3308466-14.patch6.83 KB

This patch combines the two previously existing patches (generating files, removing existing files/dir)

poker10’s picture

poker10 commented

I confirm that the patch #14 removes the fixtures/HtaccessTest directories with all files as well.

mcdruid’s picture

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

Thanks, this looks good to me!

  • poker10 committed dbf6303 on 7.x 
    Issue #3308466 by poker10, solideogloria: [D7] HtaccessTest -...
poker10’s picture

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

Thanks everyone!

Status: Fixed » Closed (fixed)

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