Repository: zarr-developers/zarr-python

Issue:
permission problem in zarr.ZipStore
#### Minimal, reproducible code sample, a copy-pastable example if possible

```python
import numpy as np
import pandas as pd
import xarray as xr
import zarr
import os

# create dataset from xarray documentation
temp = 15 + 8 * np.random.randn(2, 2, 3)
precip = 10 * np.random.rand(2, 2, 3)
lon = [[-99.83, -99.32], [-99.79, -99.23]]
lat = [[42.25, 42.21], [42.63, 42.59]]
ds = xr.Dataset({'temperature': (['x', 'y', 'time'],  temp),
                 'precipitation': (['x', 'y', 'time'], precip)},
                coords={'lon': (['x', 'y'], lon),
                        'lat': (['x', 'y'], lat),
                        'time': pd.date_range('2014-09-06', periods=3),
                        'reference_time': pd.Timestamp('2014-09-05')})


if os.path.isfile('example.zip'):
    print('removing example file')
    os.system('rm example.zip')

zipstore = zarr.ZipStore('example.zip', mode='w')
ds.to_zarr(zipstore, mode='w')
zipstore.close()


os.system('unzip -o example.zip')
os.system('ls -laR')

```

#### Problem description

The permission problem fixed by #493 for DirectoryStore remains in ZipStore. I looked at the zarr code and fix but couldn't figure out how to make a similar fix. Ideas welcome! 


#### Version and installation information

* zarr = 2.3.3.dev30
* numcodecs = 0.6.3
* python = 3.7.3
* Linux Red Hat
* zarr installed from source (hash e6667aa3)


Discussion:

Patch:
diff --git a/docs/release.rst b/docs/release.rst
index a10dd934..00a80242 100644
--- a/docs/release.rst
+++ b/docs/release.rst
@@ -4,6 +4,10 @@ Release notes
 Upcoming Release
 ----------------
 
+* Add intermediate step (using ``zipfile.ZipInfo`` object) to write
+  inside ``ZipStore`` to solve too restrictive permission issue.
+  By :user:`Raphael Dussin <raphaeldussin>`; :issue:`505`.
+
 * Add key normalization option for ``DirectoryStore``, ``NestedDirectoryStore``,
   ``TempStore``, and ``N5Store``.
   By :user:`James Bourbeau <jrbourbeau>`; :issue:`459`.
@@ -21,7 +25,7 @@ Upcoming Release
 * Upgrade dependencies in the test matrices and resolve a
   compatibility issue with testing against the Azure Storage
   Emulator. By :user:`alimanfoo`; :issue:`468`, :issue:`467`.
-  
+
 * Do not rename Blosc parameters in n5 backend and add `blocksize` parameter,
   compatible with n5-blosc. By :user:`axtimwalde`, :issue:`485`.
 
@@ -94,7 +98,7 @@ Enhancements
 ~~~~~~~~~~~~
 
 * New storage backend, backed by Azure Blob Storage, class :class:`zarr.storage.ABSStore`.
-  All data is stored as block blobs. By :user:`Shikhar Goenka <shikarsg>`, 
+  All data is stored as block blobs. By :user:`Shikhar Goenka <shikarsg>`,
   :user:`Tim Crone <tjcrone>` and :user:`Zain Patel <mzjp2>`; :issue:`345`.
 
 * Add "consolidated" metadata as an experimental feature: use
diff --git a/zarr/creation.py b/zarr/creation.py
index 632dd249..3958e2eb 100644
--- a/zarr/creation.py
+++ b/zarr/creation.py
@@ -132,7 +132,7 @@ def normalize_store_arg(store, clobber=False, default=dict):
         return default()
     elif isinstance(store, str):
         if store.endswith('.zip'):
-            mode = 'w' if clobber else 'a'
+            mode = 'w' if clobber else 'r'
             return ZipStore(store, mode=mode)
         elif store.endswith('.n5'):
             return N5Store(store)
diff --git a/zarr/storage.py b/zarr/storage.py
index 5a33940a..12154c29 100644
--- a/zarr/storage.py
+++ b/zarr/storage.py
@@ -33,6 +33,7 @@ from os import scandir
 from pickle import PicklingError
 from threading import Lock, RLock
 import uuid
+import time
 
 from numcodecs.compat import ensure_bytes, ensure_contiguous_ndarray
 from numcodecs.registry import codec_registry
@@ -1254,7 +1255,19 @@ class ZipStore(MutableMapping):
             err_read_only()
         value = ensure_contiguous_ndarray(value)
         with self.mutex:
-            self.zf.writestr(key, value)
+            # writestr(key, value) writes with default permissions from
+            # zipfile (600) that are too restrictive, build ZipInfo for
+            # the key to work around limitation
+            keyinfo = zipfile.ZipInfo(filename=key,
+                                      date_time=time.localtime(time.time())[:6])
+            keyinfo.compress_type = self.compression
+            if keyinfo.filename[-1] == os.sep:
+                keyinfo.external_attr = 0o40775 << 16   # drwxrwxr-x
+                keyinfo.external_attr |= 0x10           # MS-DOS directory flag
+            else:
+                keyinfo.external_attr = 0o644 << 16     # ?rw-r--r--
+
+            self.zf.writestr(keyinfo, value)
 
     def __delitem__(self, key):
         raise NotImplementedError


Test patch:
diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py
index 5488eeab..cd6a43b0 100644
--- a/zarr/tests/test_storage.py
+++ b/zarr/tests/test_storage.py
@@ -9,6 +9,7 @@ import tempfile
 import unittest
 from contextlib import contextmanager
 from pickle import PicklingError
+from zipfile import ZipFile
 
 import numpy as np
 import pytest
@@ -947,6 +948,23 @@ class TestZipStore(StoreTests, unittest.TestCase):
         with pytest.raises(NotImplementedError):
             store.popitem()
 
+    def test_permissions(self):
+        store = ZipStore('data/store.zip', mode='w')
+        store['foo'] = b'bar'
+        store['baz/'] = b''
+        store.flush()
+        store.close()
+        z = ZipFile('data/store.zip', 'r')
+        info = z.getinfo('foo')
+        perm = oct(info.external_attr >> 16)
+        assert perm == '0o644'
+        info = z.getinfo('baz/')
+        perm = oct(info.external_attr >> 16)
+        # only for posix platforms
+        if os.name == 'posix':
+            assert perm == '0o40775'
+        z.close()
+
 
 class TestDBMStore(StoreTests, unittest.TestCase):