archive.extracted: 2016.11.2 returns state failure for some zip formats, if already extracted

[morganwillcock]

This is probably Windows specific and related to changes here: d6adfb6

Depending on the internal format of the zip file, the archive.extracted state may return a failure, implying that content which is already extracted is the wrong type.

# cat testing.sls
7zip_test:
  archive.extracted:
    - name: C:\made_in_7zip
    - source: salt://archive_7zip.zip
    - source_hash: md5=3a6b51aa96d73409bc919e343ba7e61b
    - archive_format: zip
    - enforce_toplevel: False

explorer_test:
  archive.extracted:
    - name: C:\made_in_explorer            
    - source: salt://archive_explorer.zip
    - source_hash: md5=c73fb61cf82eb726c96e54bf845bec90
    - archive_format: zip
    - enforce_toplevel: False

There is no problem applying when the archive hasn't been extracted yet:

# salt salt-test state.apply testing
salt-test:
----------
          ID: 7zip_test
    Function: archive.extracted
        Name: C:\made_in_7zip
      Result: True
     Comment: salt://archive_7zip.zip extracted to C:\made_in_7zip\
     Started: 16:03:43.771000
    Duration: 359.0 ms
     Changes:   
              ----------
              directories_created:
                  - C:\made_in_7zip\
              extracted_files:
                  - New folder/
                  - New folder/New Text Document.txt
                  - New Text Document.txt
----------
          ID: explorer_test
    Function: archive.extracted
        Name: C:\made_in_explorer
      Result: True
     Comment: salt://archive_explorer.zip extracted to C:\made_in_explorer\
     Started: 16:03:44.130000
    Duration: 281.0 ms
     Changes:   
              ----------
              directories_created:
                  - C:\made_in_explorer\
              extracted_files:
                  - New folder/New Text Document.txt
                  - New Text Document.txt

...but note the list of extracted files differs between the two. Applying it a second time returns a failure for the first zip file, but not the second:

# salt salt-test state.apply testing
salt-test:
----------
          ID: 7zip_test
    Function: archive.extracted
        Name: C:\made_in_7zip
      Result: False
     Comment: The below paths (relative to C:\made_in_7zip\) exist, but are the incorrect type (file instead of directory, symlink instead of file, etc.). To proceed with extraction, set 'force' to True. Note that this will remove these paths before extracting.
              
              - New folder/
     Started: 16:03:52.679000
    Duration: 359.0 ms
     Changes:   
----------
          ID: explorer_test
    Function: archive.extracted
        Name: C:\made_in_explorer
      Result: True
     Comment: All files in archive are already present
     Started: 16:03:53.038000
    Duration: 249.0 ms
     Changes:

You can see the difference with zipinfo. The extracted contents are identical, but the internal structure is different between the two.

# zipinfo archive_7zip.zip
Archive:  archive_7zip.zip
Zip file size: 486 bytes, number of entries: 3
drwx---     6.3 fat        0 bx stor 17-Feb-01 15:51 New folder/
-rw-a--     6.3 fat        0 bx stor 17-Feb-01 15:51 New folder/New Text Document.txt
-rw-a--     6.3 fat        0 bx stor 17-Feb-01 15:51 New Text Document.txt
3 files, 0 bytes uncompressed, 0 bytes compressed:  0.0%

# zipinfo archive_explorer.zip
Archive:  archive_explorer.zip
Zip file size: 280 bytes, number of entries: 2
-rw-a--     2.0 fat        0 b- stor 17-Feb-01 15:51 New folder/New Text Document.txt
-rw-a--     2.0 fat        0 b- stor 17-Feb-01 15:51 New Text Document.txt
2 files, 0 bytes uncompressed, 0 bytes compressed:  0.0%

Modifying to add the force option returns a different error, but I imagine the underlying issue is the same:

# cat testing.sls
7zip_test:
  archive.extracted:
    - name: C:\made_in_7zip
    - source: salt://archive_7zip.zip
    - source_hash: md5=3a6b51aa96d73409bc919e343ba7e61b
    - archive_format: zip
    - enforce_toplevel: False
    - force: True

# salt salt-test state.apply testing
salt-test:
----------
          ID: 7zip_test
    Function: archive.extracted
        Name: C:\made_in_7zip
      Result: False
     Comment: One or more paths existed by were the incorrect type (i.e. file instead of directory or vice-versa), but could not be removed. The following errors were observed:
              
              - [Error 5] Access is denied: 'C:\\made_in_7zip\\New folder/'
     Started: 16:19:13.018000
    Duration: 110.0 ms
     Changes:

This problem is new to 2016.11.2 (did not exist in 2016.11.1).

[terminalmage]

@morganwillcock Does New Folder/ show up when you run salt salt-test archive.list salt://archive_7zip.zip verbose=True? If so, in which keys from the return dict does it appear?

[morganwillcock]

@terminalmage Thanks for looking at this. It's listed in files.

salt salt-test archive.list salt://archive_7zip.zip verbose=True

salt-test:
    ----------
    dirs:
    files:
        - New folder/
        - New folder/New Text Document.txt
        - New Text Document.txt
    links:
    top_level_dirs:
    top_level_files:
        - New Text Document.txt
    top_level_links:

[terminalmage]

OK. This suggests either that the zip file was created incorrectly, or that there is a bug in the Python stdlib. I'm launching a Windows VM instance to troubleshoot. Did you create the ZIP file on a Windows box, or another platform? Was it created using the CLI or the GUI? What version of 7-zip?

[morganwillcock]

Created on Windows, using the shell extension.

• Make a directory
• Make a file in the directory
• Make a file alongside the directory
• Select the directory and file and right-click to get the context menu
• Choose to add to a new zip file from the 7zip options on the menu

I just double checked and it was 7zip version 15.14, I also upgraded to the newest version 16.04 with the same result in the dictionary output.

[morganwillcock]

It looks like it's common to find both types of layout, so I don't think it's a 7zip bug as such.
http://stackoverflow.com/questions/11617450/check-if-a-directory-exists-in-a-zip-file-with-python

[terminalmage]

Actually, the truth is that external_attr works differently on Windows. Working on a fix.

[terminalmage]

Fixed in #39128.

There are actually two issues here. One is the external_attr one I mentioned earlier, the other is that the ZIP file support built into Explorer does not make entries in the ZIP file for directories. That is the actual reason for it not showing up in the changes in the state return.

[morganwillcock]

Thank you for the quick fix.

# salt salt-test state.apply testing
salt-test:
----------
          ID: 7zip_test
    Function: archive.extracted
        Name: C:\made_in_7zip
      Result: True
     Comment: salt://archive_7zip.zip extracted to C:\made_in_7zip\
     Started: 11:34:34.801000
    Duration: 203.0 ms
     Changes:   
              ----------
              directories_created:
                  - C:\made_in_7zip\
              extracted_files:
                  - New folder/
                  - New folder/New Text Document.txt
                  - New Text Document.txt
----------
          ID: explorer_test
    Function: archive.extracted
        Name: C:\made_in_explorer
      Result: True
     Comment: salt://archive_explorer.zip extracted to C:\made_in_explorer\
     Started: 11:34:35.004000
    Duration: 0.0 ms
     Changes:   
              ----------
              directories_created:
                  - C:\made_in_explorer\
              extracted_files:
                  - New folder/New Text Document.txt
                  - New Text Document.txt

# salt salt-test state.apply testing
salt-test:
----------
          ID: 7zip_test
    Function: archive.extracted
        Name: C:\made_in_7zip
      Result: True
     Comment: All files in archive are already present
     Started: 11:34:57
    Duration: 468.0 ms
     Changes:   
----------
          ID: explorer_test
    Function: archive.extracted
        Name: C:\made_in_explorer
      Result: True
     Comment: All files in archive are already present
     Started: 11:34:57.468000
    Duration: 234.0 ms
     Changes: