Fix symbolic mode usage in unarchive with zip files

Fixes #21090

lib/ansible/modules/files/unarchive.py

@@ -130,6 +130,7 @@ import pwd
 import re
 import stat
 import time
+import traceback
 from zipfile import ZipFile, BadZipfile
 
 from ansible.module_utils.basic import AnsibleModule
@@ -494,7 +495,10 @@ class ZipArchive(object):
                         try:
                             mode = int(self.file_args['mode'], 8)
                         except Exception as e:
-                            self.module.fail_json(path=path, msg="mode %(mode)s must be in octal form" % self.file_args, details=to_native(e))
+                            try:
+                                mode = AnsibleModule._symbolic_mode_to_octal(st, self.file_args['mode'])
+                            except ValueError as e:
+                                self.module.fail_json(path=path, msg="%s" % to_native(e), exception=traceback.format_exc())
                 # Only special files require no umask-handling
                 elif ztype == '?':
                     mode = self._permstr_to_octal(permstr, 0)

test/integration/targets/unarchive/tasks/main.yml

@@ -50,11 +50,10 @@
   copy: src=foo.txt dest={{output_dir}}/FOO-UNAR.TXT
 
 # This gets around an unzip timestamp bug in some distributions
-# Recent unzip on Fedora, Ubuntu, and BSD will randomly round some timestamps up.
+# Recent unzip on Ubuntu and BSD will randomly round some timestamps up.
 # But that doesn't seem to happen when the timestamp has an even second. 
 - name: Bug work around
   command: touch -t "201705111530.00" {{output_dir}}/foo-unarchive.txt {{output_dir}}/foo-unarchive-777.txt {{output_dir}}/FOO-UNAR.TXT
-# See Fedora bug 1451953: https://bugzilla.redhat.com/show_bug.cgi?id=1451953
 # See Ubuntu bug 1691636: https://bugs.launchpad.net/ubuntu/+source/unzip/+bug/1691636
 # When these are fixed, this code should be removed.  
 
@@ -168,14 +167,14 @@
       - "'foo-unarchive.txt' in unarchive03['files']"
       - "'foo-unarchive-777.txt' in unarchive03['files']"
       - "'FOO-UNAR.TXT' in unarchive03['files']"
-      
+
 - name: verify that the file was unarchived
   file: path={{output_dir}}/test-unarchive-zip/{{item}} state=file
   with_items:
     - foo-unarchive.txt
     - foo-unarchive-777.txt
     - FOO-UNAR.TXT
-    
+
 - name: repeat the last request to verify no changes
   unarchive: src={{output_dir}}/test-unarchive.zip dest={{output_dir | expanduser}}/test-unarchive-zip remote_src=yes list_files=True
   register: unarchive03b
@@ -184,7 +183,7 @@
   assert:
     that:
       - "unarchive03b.changed == false"
-      
+
 - name: remove our zip unarchive destination
   file: path={{output_dir}}/test-unarchive-zip state=absent
 
@@ -222,7 +221,7 @@
 - name: create our unarchive destination
   file: path={{output_dir}}/test-unarchive-tar-gz state=directory
 
-- name: unarchive and set mode to 0600
+- name: unarchive and set mode to 0600, directories 0700
   unarchive:
     src: "{{ output_dir }}/test-unarchive.tar.gz"
     dest: "{{ output_dir | expanduser }}/test-unarchive-tar-gz"
@@ -278,7 +277,7 @@
     src: "{{ output_dir }}/test-unarchive.tar.gz"
     dest: "{{ output_dir | expanduser }}/test-unarchive-tar-gz"
     remote_src: yes
-    mode: "u+rwX,g-wx,o-wx,g+r,o+r"
+    mode: "u+rwX-x,g-wx,o-wx,g+r,o+r"
     list_files: True
   register: unarchive07
 
@@ -295,6 +294,71 @@
   file: path={{ output_dir }}/test-unarchive-tar-gz state=absent
 
 
+- name: create our unarchive destination
+  file: path={{output_dir}}/test-unarchive-zip state=directory
+
+- name: unarchive and set mode to 0601, directories 0700
+  unarchive:
+    src: "{{ output_dir }}/test-unarchive.zip"
+    dest: "{{ output_dir | expanduser }}/test-unarchive-zip"
+    remote_src: yes
+    mode: "u+rwX-x,g-rwx,o=x"
+    list_files: True
+  register: unarchive08
+
+- name: Test that the file modes were changed
+  stat:
+    path: "{{ output_dir | expanduser }}/test-unarchive-zip/foo-unarchive.txt"
+  register: unarchive08_stat
+
+- name: Test that the file modes were changed
+  assert:
+    that:
+      - "unarchive08.changed == true"
+      - "unarchive08_stat.stat.mode == '0601'"
+      # Verify that file list is generated
+      - "'files' in unarchive08"
+      - "{{unarchive08['files']| length}} == 3"
+      - "'foo-unarchive.txt' in unarchive08['files']"
+      - "'foo-unarchive-777.txt' in unarchive08['files']"
+      - "'FOO-UNAR.TXT' in unarchive08['files']"
+
+- name: unarchive zipfile a second time and set mode to 0601, directories 0700
+  unarchive:
+    src: "{{ output_dir }}/test-unarchive.zip"
+    dest: "{{ output_dir | expanduser }}/test-unarchive-zip"
+    remote_src: yes
+    mode: "u+rwX-x,g-rwx,o=x"
+    list_files: True
+  register: unarchive08
+
+- name: Test that the file modes were not changed
+  stat:
+    path: "{{ output_dir | expanduser }}/test-unarchive-zip/foo-unarchive.txt"
+  register: unarchive08_stat
+
+- debug:
+    var: unarchive08
+
+- debug:
+    var: unarchive08_stat
+
+- name: Test that the files did not change
+  assert:
+    that:
+      - "unarchive08.changed == false"
+      - "unarchive08_stat.stat.mode == '0601'"
+      # Verify that file list is generated
+      - "'files' in unarchive08"
+      - "{{unarchive08['files']| length}} == 3"
+      - "'foo-unarchive.txt' in unarchive08['files']"
+      - "'foo-unarchive-777.txt' in unarchive08['files']"
+      - "'FOO-UNAR.TXT' in unarchive08['files']"
+
+- name: remove our zip unarchive destination
+  file: path={{ output_dir }}/test-unarchive-zip state=absent
+
+
 - name: create our unarchive destination
   file: path={{output_dir}}/test-unarchive-tar-gz state=directory