zipfile.Path does not work properly with zip archives where paths start with /

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

BPO	41035
Nosy	@Yhg1s, @jaraco, @serhiy-storchaka

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = created_at = labels = ['3.8', 'type-bug', 'library', 'invalid'] title = 'zipfile.Path does not work properly with zip archives where paths start with /' updated_at = user = 'https://bugs.python.org/sorrow' ``` bugs.python.org fields: ```python activity = actor = 'sorrow' assignee = 'none' closed = True closed_date = closer = 'jaraco' components = ['Library (Lib)'] creation = creator = 'sorrow' dependencies = [] files = [] hgrepos = [] issue_num = 41035 keywords = [] message_count = 12.0 messages = ['371880', '371881', '371961', '371982', '371998', '372000', '372003', '372004', '372005', '372013', '372107', '372108'] nosy_count = 5.0 nosy_names = ['twouters', 'jaraco', 'alanmcintyre', 'serhiy.storchaka', 'sorrow'] pr_nums = [] priority = 'normal' resolution = 'not a bug' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue41035' versions = ['Python 3.8'] ```

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

I encountered errors when I had to work with ZPI file where path start with "/"

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

>>> import zipfile
>>> import io
>>> data = io.BytesIO()
>>> zf = zipfile.ZipFile(data, 'w')
>>> zf.writestr('/a.txt', 'content of a')
>>> zf.filename = 'abcde.zip'
>>> root = zipfile.Path(zf)
>>> list(root.iterdir())
[]
>>> root.exists()
False

[jaraco]

Thanks sorrow for filing a report.

I primarily developed this functionality. As I did, I found the 'zip' format to be under-specified, so I used real-world examples as models to infer a spec.

It seems you may have discovered a use-case that violates that expectation, a case where /a.txt is identical to a.txt.

My instinct is that zipfile.Path should support 99% of real-world use-cases and that other use-cases may not be supported or may require additional consideration (wrappers, subclasses) to support.

Can you tell me more about your use-case and why zipp.Path/zipfile.Path should support it? Is this behavior a result of a real-world example (please share details about the origin) or contrived?

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

It seems you may have discovered a use-case that violates that expectation, a case where /a.txt is identical to a.txt.

The thing is: it's not.

Can you tell me more about your use-case and why zipp.Path/zipfile.Path should support it?

I received a .zip file and zipfile.Path in my code didn't work with it. I did some testing and discovered that id does not work properly with these archives. It cannot list the contents of such archive.

Is this behavior a result of a real-world example

Yes, it is.

(please share details about the origin)

I can't. First, the origin of this archive is not totally clear to me (and I do not want to investigate). And second, I'm under NDA.

I provided minimal example where archive created with zipfile.ZipFile itself reproduces this behaviour. Just prerpend all paths with / an it does not work.

[jaraco]

I created jaraco/zipp#56 to track the issue in the backport.

[jaraco]

It seems you may have discovered a use-case that violates that expectation, a case where /a.txt is identical to a.txt.

The thing is: it's not.

I think maybe you misunderstood. I mean that the zipfile you have seems to be treating /a.txt as a file a.txt at the root of the zipfile, identical to another zipfile that has an item named a.txt.

I'm not saying that zipfile.Path handles that situation; your example clearly contradicts that notion.

I provided minimal example where archive created with zipfile.ZipFile itself reproduces this behaviour. Just prerpend all paths with / an it does not work.

Thank you. I'm grateful for the minimal example. What I'm trying to assess here is the impact - how common is this use-case and should it be supported. One option here might be to document the library as not supporting files whose names begin with a leading slash.

Digging into the spec, Section 4.4.17.1 explicitly states:

The path stored MUST NOT contain a drive or device letter, or a leading slash.

It appears the file your client has sent and the minimal example you've generated represents an invalid zip file.

In this branch, I started exploring what it would take to support this format. Unfortunately, just patching the namelist was not enough. Supporting this change interacts with behaviors across a number of methods, so would add substantial complexity to the implementation. It becomes inelegant to manage the position in the file (.at property) when there's ambiguity about the underlying format. It opens up lots of questions, like:

• should at include the leading slash?
• should the class support zip files with mixed leading and non-leading slashes?
• at what point does Path become aware of the format used?
• are there emergent performance concerns?

In other words, the design relies heavily on the assumption that there's one way to store a file and two ways to store a directory (explicitly and implicitly).

Based on these findings, I'm disinclined to support the format in the canonical Path implementation.

What I recommend is that you develop a subclass of zipfile.Path that supports the abnormal format, use that for your work, and publish it (perhaps here, perhaps as a library) for others with the same problem to use. If enough people report it having usefulness, then I'd definitely consider incorporating it into the library, either as a separate implementation or perhaps integrating it (especially if that can be done without substantially complicating the canonical implementation).

Alternately, ask if the client can generate valid zip files. I'm eager to hear your thoughts in light of my work. Can we close this as invalid?

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

how common is this use-case

I don't know about that. I just know that I have to make my program work with these files. Asking the clients to stop using this (presumably incorrect) format (or the program that makes it) is not an option.

It appears the file your client has sent and the minimal example you've generated represents an invalid zip file.

Well, I didn't know that. But it doesn't change the matter for me.

What I recommend is that you develop a subclass of zipfile.Path that supports the abnormal format, use that for your work, and publish it (perhaps here, perhaps as a library) for others with the same problem to use.

I guess I'll have to go this way.

It opens up lots of questions, like:

I'll share my considerations about how I see it should be implemented.

• should at include the leading slash?

I think it doesn't matter, because at property is not part of Path public AP. Ideally, the files with leading slashes and without them should be treated exactly the same (as if they didn't have the leading slash). So, the only thing I'm concerned with is at argument of Path.__init__. It shouldn't require leading slash in any case.

• should the class support zip files with mixed leading and non-leading slashes?

No, this is definitely an error and shouldn't work.

• at what point does Path become aware of the format used?

When the class is created?

• are there emergent performance concerns?

For me - no, there aren't. I don't know how this kind of questions is resolved in python community though.

Can we close this as invalid?

I guess you can.

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

Path public AP.

API of course

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

When the class is created?

I mean the class instance

[jaraco]

Yes, I generally agree with your assessment. Let me know if you have any questions about the implementation as you're exploring a solution.

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

Here's what I came up with:

class ZipPath(zipfile.Path):
def __init__(self, root, at=""):
    super().__init__(root, at)
    if not at.startswith("/") and self.root.namelist()[0].startswith("/"):
        self.at = f"/{at}"

def __repr__(self):
    return (
        f"{self.__class__.__name__}({self.root.filename!r}, "
        f"{self.at.lstrip('/')!r})"
    )

def __str__(self):
    return posixpath.join(self.root.filename, self.at.lstrip("/"))

def _is_child(self, path):
    return posixpath.dirname(path.at.strip("/")) == self.at.strip("/")

def _next(self, at):
    return self.__class__(self.root, at)

Pretty simple. The main things are going on in __init__ and _is_child methods. These changes are enough for iteritems() to work. I decided to include the leading slash in the at, but strip it for the outside world (str and repr). Also, I had to override the _next method because it makes Path objects no matter what class it's called from.

[f0ca0edf-001e-4684-b5dd-a82edc455b70]

iteritems()

I meant iterdir() of course.