Repository: jaraco/zipp

Issue:
glob() recursively matches on *
This a bug made it into the Python 3.12 stdlib.

## Issue

When you have a `.zip` containing `a/b/c.txt`, and on the top-level do `.glob('*.txt')`, it produces a match when it shouldn't.

`.glob()` is accidentally implemented to always behave like the recursive `rglob()` if a `*` is used.

## Repro

Using the released Python 3.12 `zipfile` stdlib; the same happens with current `zipp`:

```sh
#! /usr/bin/env bash
set -e

mkdir -p zipp-testdir/a/b/
touch zipp-testdir/a/b/c.txt

rm -f zipp-testdir.zip
zip -r zipp.zip zipp-testdir

python3 --version
python3 -c 'import zipfile; print(list( zipfile.Path(zipfile.ZipFile("zipp.zip")).glob("*.txt") ))'
```

Prints

```python
[Path('zipp.zip', 'zipp-testdir/a/b/c.txt')]
```

when it shouldn't.


## Reason

This code

https://github.com/jaraco/zipp/blob/ee6d7117023cd90be7647f894f43aa4fef16c13d/zipp/__init__.py#L370-L375

uses `fnmatch.translate()` when the [`fnmatch` docs](https://docs.python.org/3/library/fnmatch.html) say

> Note that the filename separator (`'/'` on Unix) is not special to this module. See module [glob](https://docs.python.org/3/library/glob.html#module-glob) for pathname expansion

Discussion:
jaraco: Although this latest commit does address the failure. I think I want to go farther and test more edge cases, especially if zipp is going to be owning a regex.

Patch:
diff --git a/docs/index.rst b/docs/index.rst
index 743a493..d76eb3c 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -18,6 +18,11 @@ Welcome to |project| documentation!
     :undoc-members:
     :show-inheritance:
 
+.. automodule:: zipp.glob
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
 
 Indices and tables
 ==================
diff --git a/newsfragments/98.bugfix.rst b/newsfragments/98.bugfix.rst
new file mode 100644
index 0000000..552444d
--- /dev/null
+++ b/newsfragments/98.bugfix.rst
@@ -0,0 +1,1 @@
+Replaced the ``fnmatch.translate`` with a fresh glob-to-regex translator for more correct matching behavior.
diff --git a/zipp/__init__.py b/zipp/__init__.py
index ddfa0a6..55a2deb 100644
--- a/zipp/__init__.py
+++ b/zipp/__init__.py
@@ -5,9 +5,9 @@ import itertools
 import contextlib
 import pathlib
 import re
-import fnmatch
 
 from .py310compat import text_encoding
+from .glob import translate
 
 
 __all__ = ['Path']
@@ -367,7 +367,7 @@ class Path:
         if not pattern:
             raise ValueError(f"Unacceptable pattern: {pattern!r}")
 
-        matches = re.compile(fnmatch.translate(pattern)).fullmatch
+        matches = re.compile(translate(pattern)).fullmatch
         return (
             child
             for child in self._descendants()
diff --git a/zipp/glob.py b/zipp/glob.py
new file mode 100644
index 0000000..4a2e665
--- /dev/null
+++ b/zipp/glob.py
@@ -0,0 +1,40 @@
+import re
+
+
+def translate(pattern):
+    r"""
+    Given a glob pattern, produce a regex that matches it.
+
+    >>> translate('*.txt')
+    '[^/]*\\.txt'
+    >>> translate('a?txt')
+    'a.txt'
+    >>> translate('**/*')
+    '.*/[^/]*'
+    """
+    return ''.join(map(replace, separate(pattern)))
+
+
+def separate(pattern):
+    """
+    Separate out character sets to avoid translating their contents.
+
+    >>> [m.group(0) for m in separate('*.txt')]
+    ['*.txt']
+    >>> [m.group(0) for m in separate('a[?]txt')]
+    ['a', '[?]', 'txt']
+    """
+    return re.finditer(r'([^\[]+)|(?P<set>[\[].*?[\]])|([\[][^\]]*$)', pattern)
+
+
+def replace(match):
+    """
+    Perform the replacements for a match from :func:`separate`.
+    """
+
+    return match.group('set') or (
+        re.escape(match.group(0))
+        .replace('\\*\\*', r'.*')
+        .replace('\\*', r'[^/]*')
+        .replace('\\?', r'.')
+    )


Test patch:
diff --git a/tests/test_zipp.py b/tests/test_zipp.py
index 9e06fbe..eb4b29e 100644
--- a/tests/test_zipp.py
+++ b/tests/test_zipp.py
@@ -456,12 +456,49 @@ class TestPath(unittest.TestCase):
         assert not root.match("*.txt")
 
         assert list(root.glob("b/c.*")) == [zipp.Path(alpharep, "b/c.txt")]
+        assert list(root.glob("b/*.txt")) == [
+            zipp.Path(alpharep, "b/c.txt"),
+            zipp.Path(alpharep, "b/f.txt"),
+        ]
 
+    @pass_alpharep
+    def test_glob_recursive(self, alpharep):
+        root = zipp.Path(alpharep)
         files = root.glob("**/*.txt")
         assert all(each.match("*.txt") for each in files)
 
         assert list(root.glob("**/*.txt")) == list(root.rglob("*.txt"))
 
+    @pass_alpharep
+    def test_glob_subdirs(self, alpharep):
+        root = zipp.Path(alpharep)
+
+        assert list(root.glob("*/i.txt")) == []
+        assert list(root.rglob("*/i.txt")) == [zipp.Path(alpharep, "g/h/i.txt")]
+
+    @pass_alpharep
+    def test_glob_does_not_overmatch_dot(self, alpharep):
+        root = zipp.Path(alpharep)
+
+        assert list(root.glob("*.xt")) == []
+
+    @pass_alpharep
+    def test_glob_single_char(self, alpharep):
+        root = zipp.Path(alpharep)
+
+        assert list(root.glob("a?txt")) == [zipp.Path(alpharep, "a.txt")]
+        assert list(root.glob("a[.]txt")) == [zipp.Path(alpharep, "a.txt")]
+        assert list(root.glob("a[?]txt")) == []
+
+    @pass_alpharep
+    def test_glob_chars(self, alpharep):
+        root = zipp.Path(alpharep)
+
+        assert list(root.glob("j/?.b[ai][nz]")) == [
+            zipp.Path(alpharep, "j/k.bin"),
+            zipp.Path(alpharep, "j/l.baz"),
+        ]
+
     def test_glob_empty(self):
         root = zipp.Path(zipfile.ZipFile(io.BytesIO(), 'w'))
         with self.assertRaises(ValueError):