search

guzba / zippy

Pure Nim implementation of deflate, zlib, gzip and zip.
MIT License
246 stars 29 forks source link

Fix extractAll() not setting owner permissions for some files #37

Closed SnwMds closed 2 years ago

SnwMds commented 2 years ago

The size of the header containing information about the permissions of each file can sometimes exceed 6 bytes (e.g. it can be 0000755 instead of 000755), so I increased the bytes range check from 100 ..< 106 to 100 ..< 107 to match them.

I'm not sure what criteria is used for this variation of files, however.

guzba commented 2 years ago

Thanks for finding the issue here! I've been working on Zippy again lately so I'll be looking at this very soon. I will improve the tests and get this fix merged in.

guzba commented 2 years ago

Hello and thanks again for the PR here. In the end, I did spend a bunch of time looking at this and discovered this is not quite a complete fix so I'm going to close this in favor of what I've got in the works.

I am completely redoing how extractAll works. It's much better now and also fixes this issue among others along the way.

You are correct that we need to read further, however we also need to treat these as "zero terminated oct numbers":

proc parseTarOctInt(s: string): int =
  try:
    if s[0] == '\0':
      0
    else:
      parseOctInt(s)
  except ValueError:
    raise currentExceptionAsZippyError()

I'm still working on this new extractAll but I'll post in the issue https://github.com/guzba/zippy/issues/36 with updates.