Repository: fsspec/gcsfs (python)

Issue:
Doesn't work for a file path containing a hashtag (`#`)
The lib cannot read a file with the file path containing `#`. Weird thing is that it has no error too, just nothing is read. A silent bug.

```python
fs = gcsfs.GCSFileSystem()

# Nothing printed out
data = fs.open("gs://sample_project/my_folder/#this_is_file.json")
print(data.read())
data.close()

# Then I change the file name to remove `#`
# This works
data = fs.open("gs://sample_project/my_folder/this_is_file.json")
print(data.read())
data.close()
```

Not sure if there is an easy way to fix?

Pull request:
Escape all special characters in URL paths
Filenames including `#` and possibly other special characters are currently not supported.
I would like to change the escaping which is done to use object names in URL paths to be closer to the implementation in `google-cloud-storage` [here](https://github.com/googleapis/python-storage/blob/928ebbccbe183666f3b35adb7226bd259d4e71c0/google/cloud/storage/blob.py#L4403).

Closes #447, #270

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 65f7e5c..57d4db2 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -1,6 +1,6 @@
 name: CI
 
-on: [push, pull_request]
+on: [push, pull_request, workflow_dispatch]
 
 jobs:
   test:
@@ -26,7 +26,7 @@ jobs:
       - name: Install dependencies
         shell: bash -l {0}
         run: |
-          conda install -c conda-forge pytest ujson requests decorator google-auth aiohttp google-auth-oauthlib flake8 black google-cloud-core google-api-core google-api-python-client -y
+          conda install -c conda-forge pytest ujson requests decorator google-auth aiohttp google-auth-oauthlib google-cloud-core google-api-core google-api-python-client -y
           pip install git+https://github.com/fsspec/filesystem_spec --no-deps
           conda list
           conda --version
@@ -41,8 +41,10 @@ jobs:
             export GOOGLE_APPLICATION_CREDENTIALS=$(pwd)/gcsfs/tests/fake-secret.json
             py.test -vv gcsfs
 
-      - name: Run pre-commit hooks
-        shell: bash -l {0}
-        run: |
-          pip install pre-commit
-          pre-commit run --all-file
+  lint:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-python@v2
+    - uses: pre-commit/action@v2.0.0
diff --git a/gcsfs/core.py b/gcsfs/core.py
index d383972..f1defb7 100644
--- a/gcsfs/core.py
+++ b/gcsfs/core.py
@@ -22,6 +22,7 @@ from .retry import retry_request, validate_response
 from .checkers import get_consistency_checker
 from .credentials import GoogleCredentials
 from . import __version__ as version
+from urllib.parse import quote as quote_urllib
 
 logger = logging.getLogger("gcsfs")
 
@@ -52,15 +53,6 @@ GCS_MIN_BLOCK_SIZE = 2**18
 GCS_MAX_BLOCK_SIZE = 2**28
 DEFAULT_BLOCK_SIZE = 5 * 2**20
 
-
-QUOTE_TABLE = str.maketrans(
-    {
-        "%": "%25",
-        "/": "%2F",
-        " ": "%20",
-    }
-)
-
 SUPPORTED_FIXED_KEY_METADATA = {
     "content_encoding": "contentEncoding",
     "cache_control": "cacheControl",
@@ -70,12 +62,10 @@ SUPPORTED_FIXED_KEY_METADATA = {
 }
 
 
-def quote_plus(s):
+def quote(s):
     """
-    Convert some URL elements to be HTTP-safe.
-
-    Not the same as in urllib, because, for instance, parentheses and commas
-    are passed through.
+    Quote characters to be safe for URL paths.
+    Also quotes '/'.
 
     Parameters
     ----------
@@ -85,7 +75,8 @@ def quote_plus(s):
     -------
     corrected URL
     """
-    return s.translate(QUOTE_TABLE)
+    # Encode everything, including slashes
+    return quote_urllib(s, safe="")
 
 
 def norm_path(path):
@@ -368,7 +359,7 @@ class GCSFileSystem(AsyncFileSystem):
             path = self.base + path
 
         if args:
-            path = path.format(*[quote_plus(p) for p in args])
+            path = path.format(*[quote(p) for p in args])
         return path
 
     @retry_request(retries=retries)
@@ -779,7 +770,7 @@ class GCSFileSystem(AsyncFileSystem):
         """Get HTTP URL of the given path"""
         u = "{}/download/storage/v1/b/{}/o/{}?alt=media"
         bucket, object = self.split_path(path)
-        object = quote_plus(object)
+        object = quote(object)
         return u.format(self._location, bucket, object)
 
     async def _cat_file(self, path, start=None, end=None, **kwargs):
@@ -937,7 +928,7 @@ class GCSFileSystem(AsyncFileSystem):
                     template.format(
                         i=i + 1,
                         bucket=p.split("/", 1)[0],
-                        key=quote_plus(p.split("/", 1)[1]),
+                        key=quote(p.split("/", 1)[1]),
                     )
                     for i, p in enumerate(chunk)
                 ]
@@ -1474,7 +1465,7 @@ class GCSFile(fsspec.spec.AbstractBufferedFile):
         uid = re.findall("upload_id=([^&=?]+)", self.location)
         self.gcsfs.call(
             "DELETE",
-            f"{self.fs._location}/upload/storage/v1/b/{quote_plus(self.bucket)}/o",
+            f"{self.fs._location}/upload/storage/v1/b/{quote(self.bucket)}/o",
             params={"uploadType": "resumable", "upload_id": uid},
             json_out=True,
         )
@@ -1570,7 +1561,7 @@ async def initiate_upload(
     j.update(_convert_fixed_key_metadata(fixed_key_metadata))
     headers, _ = await fs._call(
         method="POST",
-        path=f"{fs._location}/upload/storage/v1/b/{quote_plus(bucket)}/o",
+        path=f"{fs._location}/upload/storage/v1/b/{quote(bucket)}/o",
         uploadType="resumable",
         json=j,
         headers={"X-Upload-Content-Type": content_type},
@@ -1593,7 +1584,7 @@ async def simple_upload(
     fixed_key_metadata=None,
 ):
     checker = get_consistency_checker(consistency)
-    path = f"{fs._location}/upload/storage/v1/b/{quote_plus(bucket)}/o"
+    path = f"{fs._location}/upload/storage/v1/b/{quote(bucket)}/o"
     metadata = {"name": key}
     if metadatain is not None:
         metadata["metadata"] = metadatain
diff --git a/gcsfs/retry.py b/gcsfs/retry.py
index 31d9fe4..b8b8b89 100644
--- a/gcsfs/retry.py
+++ b/gcsfs/retry.py
@@ -77,9 +77,9 @@ def validate_response(status, content, path, args=None):
     """
     if status >= 400:
         if args:
-            from .core import quote_plus
+            from .core import quote
 
-            path = path.format(*[quote_plus(p) for p in args])
+            path = path.format(*[quote(p) for p in args])
         if status == 404:
             raise FileNotFoundError(path)
 


Test patch:
diff --git a/gcsfs/tests/test_core.py b/gcsfs/tests/test_core.py
index 1a18481..86b0a52 100644
--- a/gcsfs/tests/test_core.py
+++ b/gcsfs/tests/test_core.py
@@ -23,7 +23,7 @@ from gcsfs.tests.conftest import (
     text_files,
 )
 from gcsfs.tests.utils import tempdir, tmpfile
-from gcsfs.core import GCSFileSystem, quote_plus
+from gcsfs.core import GCSFileSystem, quote
 from gcsfs.credentials import GoogleCredentials
 import gcsfs.checkers
 from gcsfs import __version__ as version
@@ -306,7 +306,7 @@ def test_url(gcs):
     fn = TEST_BUCKET + "/nested/file1"
     url = gcs.url(fn)
     assert "http" in url
-    assert quote_plus("nested/file1") in url
+    assert quote("nested/file1") in url
     with gcs.open(fn) as f:
         assert "http" in f.url()
 
@@ -476,6 +476,36 @@ def test_get_put_file_in_dir(protocol, gcs):
         assert gcs.cat(protocol + TEST_BUCKET + "/temp_dir/accounts.1.json") == data1
 
 
+def test_special_characters_filename(gcs: GCSFileSystem):
+    special_filename = """'!"`#$%&'()+,-.<=>?@[]^_{}~/'"""
+    full_path = TEST_BUCKET + "/" + special_filename
+    gcs.touch(full_path)
+    info = gcs.info(full_path)
+    assert info["name"] == full_path
+    # Normal cat currently doesn't work with special characters,
+    # because it invokes expand_path (and in turn glob) without escaping the characters.
+    # This would need to be fixed in fsspec.
+    assert gcs.cat_file(full_path) == b""
+
+
+def test_slash_filename(gcs: GCSFileSystem):
+    slash_filename = """abc/def"""
+    full_path = TEST_BUCKET + "/" + slash_filename
+    gcs.touch(full_path)
+    info = gcs.info(full_path)
+    assert info["name"] == full_path
+    assert gcs.cat_file(full_path) == b""
+
+
+def test_hash_filename(gcs: GCSFileSystem):
+    slash_filename = """a#b#c"""
+    full_path = TEST_BUCKET + "/" + slash_filename
+    gcs.touch(full_path)
+    info = gcs.info(full_path)
+    assert info["name"] == full_path
+    assert gcs.cat_file(full_path) == b""
+
+
 def test_errors(gcs):
     with pytest.raises((IOError, OSError)):
         gcs.open(TEST_BUCKET + "/tmp/test/shfoshf", "rb")