[issue28494] is_zipfile false positives

[miss-islington]

miss-islington  added the comment:


New changeset 7acb22e6e9061f85988c0c6c5ee25ebdf2950841 by Miss Islington (bot) 
in branch '3.8':
bpo-28494: install ziptestdata to fix install bot (GH-15902)
https://github.com/python/cpython/commit/7acb22e6e9061f85988c0c6c5ee25ebdf2950841


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[miss-islington]

Change by miss-islington :


--
pull_requests: +15553
pull_request: https://github.com/python/cpython/pull/15912

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset c37447481ec8f6d0e49d0587ec0de3f9e7d56b28 by Gregory P. Smith in 
branch 'master':
bpo-28494: install ziptestdata to fix install bot (GH-15902)
https://github.com/python/cpython/commit/c37447481ec8f6d0e49d0587ec0de3f9e7d56b28


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Gregory P. Smith]

Change by Gregory P. Smith :


--
pull_requests: +15543
pull_request: https://github.com/python/cpython/pull/15902

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[miss-islington]

miss-islington  added the comment:


New changeset 74b0291b03db60dd244d31e9c97407cccb8d30dd by Miss Islington (bot) 
in branch '3.8':
bpo-28494: Test existing zipfile working behavior. (GH-15853)
https://github.com/python/cpython/commit/74b0291b03db60dd244d31e9c97407cccb8d30dd


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[STINNER Victor]

STINNER Victor  added the comment:

> The new ziptestdata/ subdir appears to not be part of the install that make 
> install does. :/

It seems like Lib/test/eintrdata/ (for example) is installed using LIBSUBDIRS 
variable in Makefile.pre.in.

Note: The Windows installer copies recursively Lib/test/ and subdirectories: 
see  in 
Tools/msi/test/test.wixproj.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Gregory P. Smith]

Gregory P. Smith  added the comment:

The new ziptestdata/ subdir appears to not be part of the install that make 
install does. :/

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[STINNER Victor]

STINNER Victor  added the comment:

x86 Gentoo Installed with X 3.x buildbot is unhappy:

https://buildbot.python.org/all/#/builders/103/builds/3051

==
ERROR: test_execute_zip2 (test.test_zipfile.TestExecutablePrependedZip)
--
Traceback (most recent call last):
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/test/test_zipfile.py",
 line 2502, in test_execute_zip2
output = subprocess.check_output([self.exe_zip, sys.executable])
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 411, in check_output
return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 489, in run
with Popen(*popenargs, **kwargs) as process:
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 845, in __init__
self._execute_child(args, executable, preexec_fn, close_fds,
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 1689, in _execute_child
raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 
'ziptestdata/exe_with_zip'

==
ERROR: test_execute_zip64 (test.test_zipfile.TestExecutablePrependedZip)
--
Traceback (most recent call last):
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/test/test_zipfile.py",
 line 2509, in test_execute_zip64
output = subprocess.check_output([self.exe_zip64, sys.executable])
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 411, in check_output
return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 489, in run
with Popen(*popenargs, **kwargs) as process:
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 845, in __init__
self._execute_child(args, executable, preexec_fn, close_fds,
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/subprocess.py",
 line 1689, in _execute_child
raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 
'ziptestdata/exe_with_z64'

==
FAIL: test_read_zip64_with_exe_prepended 
(test.test_zipfile.TestExecutablePrependedZip)
--
Traceback (most recent call last):
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/test/test_zipfile.py",
 line 2496, in test_read_zip64_with_exe_prepended
self._test_zip_works(self.exe_zip64)
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/test/test_zipfile.py",
 line 2484, in _test_zip_works
self.assertTrue(zipfile.is_zipfile(name),
AssertionError: False is not true : is_zipfile failed on 
ziptestdata/exe_with_z64

==
FAIL: test_read_zip_with_exe_prepended 
(test.test_zipfile.TestExecutablePrependedZip)
--
Traceback (most recent call last):
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/test/test_zipfile.py",
 line 2493, in test_read_zip_with_exe_prepended
self._test_zip_works(self.exe_zip)
  File 
"/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.9/test/test_zipfile.py",
 line 2484, in _test_zip_works
self.assertTrue(zipfile.is_zipfile(name),
AssertionError: False is not true : is_zipfile failed on 
ziptestdata/exe_with_zip

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[miss-islington]

Change by miss-islington :


--
pull_requests: +15532
pull_request: https://github.com/python/cpython/pull/15891

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Thomas Wouters]

Thomas Wouters  added the comment:


New changeset 3f4db4a0bab073b768fae958e93288bd5d24eadd by T. Wouters (Gregory 
P. Smith) in branch 'master':
bpo-28494: Test existing zipfile working behavior. (GH-15853)
https://github.com/python/cpython/commit/3f4db4a0bab073b768fae958e93288bd5d24eadd


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Gregory P. Smith]

Change by Gregory P. Smith :


--
pull_requests: +15502
pull_request: https://github.com/python/cpython/pull/15853

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Gregory P. Smith]

Gregory P. Smith  added the comment:

it's a bugfix, it seems reasonable for 3.7 to me.  I agree that the previous 
is_zipfile check is too lenient.  I'll follow up on jjolly's PR for any 
specific concerns I have with the implementation.

--
assignee: serhiy.storchaka -> gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Gregory P. Smith]

Change by Gregory P. Smith :


--
versions: +Python 3.8 -Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Matthew Ryan]

Change by Matthew Ryan :


--
nosy: +mryan1539

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[John Jolly]

Change by John Jolly :


--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[John Jolly]

John Jolly  added the comment:

Is there any chance that this will make it into 3.7 or is my reminder too late?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[John Jolly]

John Jolly  added the comment:

Fix submitted that evaluates the ECD structure and validates the first CD 
entry. The fix also handles empty zipfiles.

IMO the purpose of this API is to *quickly* verify that the file is a valid 
zipfile. With this fix, the API only reads another 46 bytes of data (after a 
seek, of course). This should still qualify as "quick", especially after having 
potentially read 64k of data.

Perhaps a full zip validator would be appropriate in addition to is_zipfile. 
That would be more appropriate as a full feature rather than in this bugfix.

--
nosy: +jjolly

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[John Jolly]

Change by John Jolly :


--
pull_requests: +4934
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

No, checking the first bytes of the file is not appropriate option. zipfile 
should support the Python zip application format [1].

I see two options:

1. Make is_zipfile() more strict that the ZipFile constructor. The later 
supports ZIP files with a data past the comment or with truncated comments, but 
the former should reject them.

2. Make both is_zipfile() and the ZipFile constructor more robust. They should 
check not just the EOCD signature, but check the Zip64 end of central directory 
record (if exists) and the first central file header signature (if the ZIP file 
is not empty).

It may be that PDF files contain PK\005\006 not accidentally, but because they 
contain embedded ZIP files (I don't know if this is a case). In that 
circumstances is_zipfile() returning True is correct.

[1] https://docs.python.org/3/library/zipapp.html

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Thomas Waldmann]

Thomas Waldmann added the comment:

Well, if you have a better idea how to fix is_zipfile, go on.

I even suggested an alternative, how about that?

It is a miserable state when the is_zipfile function in the stdlib detects 
random crap as a zip file.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The problem is that the zipfile module supports even not well-formed archives, 
with a data appended past a comment, and with truncated comment. There are 
special tests for this, and the proposed patch breaks these tests: 
test_comments, test_ignores_newline_at_end, 
test_ignores_stuff_appended_past_comments. See issue10694 and issue1622.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Thomas Waldmann]

Thomas Waldmann added the comment:

Note: checking the first bytes of the file (PK..) might be another option.

But this has the "problem" that a self-extracting zip starts with an executable 
that has different first bytes.

So whether this is an option or not depends on whether is_zipfile() should 
return truish for self-extracting ZIP files.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Thomas Waldmann]

Thomas Waldmann added the comment:

patch for py2.7

The EOCD structure is at EOF.

It either does not contain a comment (this is what the existing code checks 
first) or it contains a comment of the length that is specified in the 
structure.

The patch checks consistency specified length vs. real length (end of fixed 
part of structure up to EOF). If this does not match, it is likely not a zip 
file, but just a file that happens to have the magic 4 bytes somewhere in its 
last 64kB.

--
keywords: +patch
Added file: http://bugs.python.org/file45164/isz_fail_fix.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +alanmcintyre, serhiy.storchaka, twouters
versions: +Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28494] is_zipfile false positives

[Thomas Waldmann]

New submission from Thomas Waldmann:

zipfile.is_zipfile has false positives way too easily.

I just have seen it in practive when a MoinMoin wiki site with a lot of pdf 
attachments crashed with 500. This was caused by a valid PDF that just happened 
to contain PK\005\006 somewhere in the middle - this was enough to satisfy 
is_zipfile() and triggered further processing as a zipfile, which then crashed 
with IOError (which was not catched in our code, yet).

I have looked into zipfile code: if the usual EOCD structure (with empty 
comment) is not at EOF, it is suspected that there might be a non-empty comment 
and ~64K before EOF are searched for the PK\005\006 magic. If it is somewhere 
there, it is assumed that the file is a zip, without any further validity check.

Attached is a failure demo that works with at least 2.7 and 3.5.

https://en.wikipedia.org/wiki/Zip_(file_format)

--
components: Library (Lib)
files: isz_fail.py
messages: 279084
nosy: Thomas.Waldmann
priority: normal
severity: normal
status: open
title: is_zipfile false positives
type: behavior
versions: Python 2.7, Python 3.5
Added file: http://bugs.python.org/file45162/isz_fail.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com