Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The current implementation of SkipDotsRecursiveDirectoryIterator doesn't work on systems where dotfiles are included in the list, and the first entry is a dotfile. Next is only called when iterating to the next element, rewind is triggered to reset the iterator to the first element.
I triggered this bug on a php 5.4.11 system.
The fix is simple, skip the dot files either on next or on rewind calls.
Comment | File | Size | Author |
---|---|---|---|
#10 | SkipDotsRecursiveDirectoryIterator-skipping-all-dot-files-1915088-2.patch | 662 bytes | bfroehle |
#2 | SkipDotsRecursiveDirectoryIterator-skipping-all-dot-files-1915088.patch | 597 bytes | openminds |
Comments
Comment #1
openminds CreditAttribution: openminds commentedComment #2
openminds CreditAttribution: openminds commentedattached patch
Comment #3
PatchRanger CreditAttribution: PatchRanger commentedI assume you meant that it needs review.
Comment #4
openminds CreditAttribution: openminds commentedI don't know - it works for me, so it should somehow work its way through all the drupal-how-to-get-a-patch-in-core...
Comment #5
bfroehle CreditAttribution: bfroehle commentedOkay yes. This looks good. Do we need to advance in __construct() too? Or does that call rewind?
Comment #6
Krizalys CreditAttribution: Krizalys commentedAccording to the PHP documentation,
rewind()
is automatically called when anIterator
instance is being iterated, as demonstrated in this example. Combined to the patch proposed, this behavior would guarantee that dots are always skipped while iterating.Comment #7
bfroehle CreditAttribution: bfroehle commented@Krizalys: Yes, but I just read through the PHP source and their implementation (at the C level) would end up calling the equivalent of our
skipdots
in__construct
,next
, andrewind
.It's a little pedantic, but I think we should also call
skipdots
in__construct
. This would fix the result of creating an instance and immediately callingcurrent
.Comment #8
compa CreditAttribution: compa commentedJust applied this to a 7.20 as well (un PHP 5.4.11) - confirmed it fixed my modules-upgrade issue
Comment #9
Krizalys CreditAttribution: Krizalys commentedI'm raising the priority of this issue since Drupal 7 will face the same bug (not being able to upgrade some modules, among other things, will be one consequence) in PHP 5.3.23, the next version to be released. Ideally, this patch should be applied to Drupal 7.21.
Comment #10
bfroehle CreditAttribution: bfroehle commentedComment #11
Christopher Riley CreditAttribution: Christopher Riley commentedThank you for the patch it worked perfectly and kept me from going bald by pulling my hair out.
Comment #12
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedAwesome, thanks!
Since this is not needed for D8 the patch is probably good to go. Is there a good way to catch the problem in an automated test?
Comment #13
hansfn CreditAttribution: hansfn commentedThe patch in comment #10 is good. RTBC-ing.
Comment #14
mjross CreditAttribution: mjross commentedI applied the patch in comment #10, and it didn't resolve the module update problem in a clean install (Drupal 7.21 and PHP 5.4.11). I'd be happy try any other proposed solutions or alternate patches, and report the results.
Comment #15
hansfn CreditAttribution: hansfn commentedAre you 110% sure that you did the test correctly - cleared cache and all that before testing? What exactly was the error message you got?
Comment #16
mjross CreditAttribution: mjross commentedYes, I verified that the patch changes were applied to /includes/filetransfer/filetransfer.inc, and cleared the cache. The error message is the type listed at that issue I linked to. For instance, for the Token module:
Comment #17
hansfn CreditAttribution: hansfn commentedOK, I don't have any other suggestions.
PS! Why is the slash backward in stead of forward. On what OS are you testing.
Comment #18
mjross CreditAttribution: mjross commentedWindows 7. I haven't examined the Drupal code in detail, but presumably Drupal uses the standard "/" in the directory path, and when it seeks the parent directory, Windows returns "\..".
Downgrading the version of PHP might be a final option, but I would much rather avoid that.
Comment #19
David_Rothstein CreditAttribution: David_Rothstein commentedIt sounds like this needs more manual testing, especially if we can't write automated tests for it (and if there are no existing ones)? Especially to make sure it doesn't cause any regressions in the module install/update functionality.
The code looks pretty reasonable though.
Comment #20
David_Rothstein CreditAttribution: David_Rothstein commentedSo basically, if people can report that they've tested actual Drupal functionality that uses this code, and make sure it continues to work correctly, we should be able to get it committed.
Comment #21
Yazzbe CreditAttribution: Yazzbe commented#10 is working for me too. D 7.21 installed.
Comment #22
David_Rothstein CreditAttribution: David_Rothstein commented@yazzy008 (and others who commented earlier): It would be useful if you could summarize in more detail what steps you actually took to test the patch :) Thanks!
Comment #23
millionleaves CreditAttribution: millionleaves commentedI encountered an error running module updates from within Drupal.
This comment summarises the iterations I went through to fix the issue: http://drupal.org/node/1900580#comment-7209694
Essentially, I'm running PHP 5.4 and Drupal 7.21; the patch referred to in #10 resolved the issue I was experiencing with module updates. I have not tested it any more than that.
Comment #24
mjross CreditAttribution: mjross commentedI repeated the module update process, and it worked this time. It is baffling that different results occurred even though no changes were made to the files. The only apparent change from yesterday vs. today is the system reboot (this morning), which of course means Apache was restarted; but I can't imagine how that would affect the results. Anyway, the patch seems fine. Thank you to everyone who worked on it.
Comment #25
Anonymous (not verified) CreditAttribution: Anonymous commentedI ran into this bug today and applying the patch at #10 fixed that module update error on 7.21. I did not try downgrading PHP (currently 5.4). Thanks a lot!
Comment #26
anomalophobe CreditAttribution: anomalophobe commentedRan into the module update problem on a fresh install of Drupal 7.21 (PHP 5.4), and #10 fixed my issue - thanks much! Will report back if I have any further issues once I configure the site.
Comment #27
arne@dbyte.nl CreditAttribution: arne@dbyte.nl commentedPatch at #10 also fixed the problem for me (drupal 7.19 + PHP 5.3.23)
Comment #28
greggmarshallPatch at #10 worked for me Drupal 7.21 + PHP 5.3.23 on a Linux server
Comment #29
hansfn CreditAttribution: hansfn commentedI think we have gotten enough manually tests now. The only negative report was positive on the second try.
Comment #30
David_Rothstein CreditAttribution: David_Rothstein commentedThanks for the extra testing, and the patch!
Committed to 7.x. http://drupalcode.org/project/drupal.git/commit/d9cf661
This issue still has the "needs tests" tag... if anyone can figure out how to write automated tests for it, it would be great to reopen to do so.
Comment #31
David_Rothstein CreditAttribution: David_Rothstein commentedOops, wrong issue tag.
Comment #32
briflo CreditAttribution: briflo commentedThis patch fixed the update problem for me. Xammp on Windows. Drupal 7.21 PHP 5.4.7
Comment #34
toni.mueller CreditAttribution: toni.mueller commentedHello,
how can I apply this patch? I have the issue "File Transfer failed, reason: Cannot remove directory".
Best regards,
Toni