collectstatic copies wrong files to STATIC_ROOT

[Maciej Wiśniowski]

In specific situation, in Django 1.3.1, second run of collecstatic command might copy wrong files to STATIC_ROOT. This is dangerous situation in production environment.

Attached test and patch against tags/1.3.1

This issue doesn't exist in current trunk (Django 1.4) but seems to me that it might been fixed 'accidentally' so I also attach (passing) test for current trunk.

To reproduce the issue:

1. INSTALLED_APPS = ['appA', 'appB']
2. static files in apps:

   appA/
      static/
           file.js
   
   appB/
      static/
           file.js
   

3. Modification date of file.js in appB is newer than modification date of file.js in appA

4. run collectstatic twice

At first run everything is OK. File from appA is copied to STATIC_ROOT (as documentation says duplicate files are taken in order of apps in INSTALLED_APPS), but in second run file.js is replaced by wrong one - from appB.

This is caused by use of:

 shutil.copy2(source_path, full_path)

in copy_file method at collectstatic.py.

shutil.copy2 preservers original file modification date what causes that file.js from appB is considered newer than file in STATIC_ROOT.

[Jannis Leidel]

This is a release blocker for 1.3.X as it's a potential data loss bug.

In trunk it was indeed fixed by using the staticfiles storage's save method.

[Paul Winkler]

I think I just discovered a different way to trigger the same problem - different file being collected on second run.
What I had was duplicate paths found via different Finders.

To reproduce w/ Django 1.3.1:

• Use default value of settings.STATICFILES_FINDERS

• Put ['appA', 'appB'] in settings.INSTALLED_APPS. Order does not matter.

• Static files in apps:

   appA/
     static/
       file.js   # MORE RECENT
  
   appB/
     something_else/
       file.js   # OLDER
  

• settings.STATICFILES_DIRS = [ '/path/to/appB/something_else' ]

Now, the first time you run, the file.js in appB/something_else will be collected first because FileSystemFinder runs first. Then the AppDirectoriesFinder will find the one in appA/static, but it's skipped because "file.js" is already in self.copied_files.
So appB wins.

The second time you run, the file.js in appB/something_else is found first again, but skipped because it's not modified. Then the AppDirectoriesFinder runs, and the file.js in appA/static is found, but this time "file.js" is not in self.copied_files, and self.delete_file() finds that it's more recent than the one we collected on the previous run.
So appA wins.

If I understand the original bug report correctly, this variant should be fixed on trunk too.

[Maciej Wiśniowski]

Replying to slinkp:

The second time you run, the file.js in appB/something_else is found first again, but skipped because it's not modified. Then the AppDirectoriesFinder runs, and the file.js in appA/static is found, but this time "file.js" is not in self.copied_files, and self.delete_file() finds that it's more recent than the one we collected on the previous run.
So appA wins.

If I understand the original bug report correctly, this variant should be fixed on trunk too.

I think that you're right there but I just started to wonder about slightly different way to get wrong results with collectstatic, even with trunk version(!).

According to your example structure: if appA/static/file.js will be modified after collectstatic was run, then, even in trunk version, it will be copied to STATIC_ROOT, because it's modification date will be newer than modification date of STATIC_ROOT/file.js.

[Maciej Wiśniowski]

Tests and patch against current trunk

[Maciej Wiśniowski]

Tests and patch against 1.3.1 tag

[Maciej Wiśniowski]

Added tests and patch for both 1.3 and trunk. Interesting thing is that after the patch I had to slightly modify another test (at trunk) that used cached storage. This was caused by use of wrong file name in that test.

[Jannis Leidel]

In [17612]:

Fixed #17737 -- Stopped the collectstatic management command from copying the wrong file in repeated runs. Thanks, pigletto.

[Jannis Leidel]

In [17613]:

[1.3.X] Fixed #17737 -- Stopped the collectstatic management command from copying the wrong file in repeated runs. Thanks, pigletto.

Backport from trunk (r17612).

[Paul Winkler]

Works for my variant, thanks. Jezdez, will this get backported to the standalone django-staticfiles? Thanks!

[Jannis Leidel]

Yeah, I'll backport the change asap to the django-staticfiles apps.