[issue16611] multiple problems with Cookie.py
John Dennis added the comment: I think your basic approach is fine and it's O.K. to break backwards compatibility. I'm not sure anyone was using the httponly and secure flags in the past because it was so broken, the logic was completely contradictory, so backwards compatibility should not trump fixing obviously broken logic. I didn't review your patch carefully, just a really quick glance. The one thing that did pop out was compiling the static regexp every time the function is called. Why not make it a module or class object and compile once at load time? -- ___ Python tracker <http://bugs.python.org/issue16611> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16611] multiple problems with Cookie.py
John Dennis added the comment: That's because #3073 never addressed the core problems, so yes I would expect you would see failures. The point of the attached test is to illustrate the deficiencies in Cookie.py, so apparently it's doing it's job :-) FWIW, we wrote a new cookie library to get around the inherent problems in Cookie.py and cookielib. We could contribute it if there is interest. -- ___ Python tracker <http://bugs.python.org/issue16611> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16611] multiple problems with Cookie.py
New submission from John Dennis: There are multiple problems with Cookie.py. Some of the issues are covered in http://bugs.python.org/issue3073 which is still open (after 4.5 years). In all honesty the API and the implementation are not great perhaps the best thing would be to remove it from the core libraries, however you can't remove a core library. There is cookielib.py is which is pretty good however cookielib.py is tightly coupled to urllib2 and if you're not using urllib2 you can't use cookielib.py so you're stuck using Cookie.py which means the best thing is to get the bugs in Cookie.py fixed. Of the problems illustrated in the attached unittest (test_cookie.py) the absolute must fix issues are the inability to parse an Expires attribute and the impossibility of testing the HttpOnly & Secure flags for a truth value after parsing. Those are critical because it makes using Cookie.py impossible. The other errors would be nice to get fixed, but not as critical. Next in importance would be respecting the truth value when setting the HttpOnly & Secure flags. Failing to detect an improperly formatted cookie when parsing is the least important because hopefully you won't have improperly formatted cookies (unfortunately a weak assumption) Note: the HttpOnly and Secure issues are symmetrical, they both suffer the same problems because they're both boolean flags whose True value is asserted by the flag's presence and it's False value by it's absence. Cookie parsing problems: * Cannot read a properly formatted Expires attribute (see also issue 3073) * Impossible to determine state of HttpOnly boolean flag after parsing * Impossible to determine state of Secure boolean flag after parsing * Fails to raise any errors when parsing invalid cookie strings Cookie creation/initialization problems: * Setting HttpOnly flag to a value which evaluates to False results in the flag being set to True (there is no check whatsoever on the value). * Setting Secure flag to a value which evaluates to False results in the flag being set to True (there is no check whatsoever on the value). Attached is a unittest illustrating the problems (more details are in the unittest). python test_cookie.py FF.FF...F -- Ran 13 tests in 0.003s FAILED (failures=9) -- components: Library (Lib) files: test_cookie.py messages: 176957 nosy: jdennis priority: normal severity: normal status: open title: multiple problems with Cookie.py versions: Python 2.7 Added file: http://bugs.python.org/file28208/test_cookie.py ___ Python tracker <http://bugs.python.org/issue16611> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
John Dennis added the comment: The changesets are not in the release27-maint branch. sdist still does not generate a correct archive for release, this is a very serious regression. -- resolution: fixed -> status: closed -> open ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
Changes by John Dennis : -- nosy: +dmalcolm ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
John Dennis added the comment: No, I don't think I'm going to turn the tarball into a unit test, etc. I didn't break the code but I did report the problem, researched the history, provided a clear explanation, a reproducer and a patch to fix the problem. I think I've done my part as a community member. -- ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
John Dennis added the comment: $ tar xzf distutils_bug.tar.gz $ cd distutils_bug $ ./setup.py sdist $ ./setup.py sdist running sdist running check warning: sdist: manifest template 'MANIFEST.in' does not exist (using default file list) not writing to manually maintained manifest file 'MANIFEST' creating foobar-1.0 making hard links in foobar-1.0... hard linking README -> foobar-1.0 hard linking setup.py -> foobar-1.0 creating dist Creating tar archive removing 'foobar-1.0' (and everything under it) $ cat MANIFEST README setup.py my_special_file Note, the MANIFEST should have been read and the resulting tar file should have had my_special_file in it. sdist complains about a missing MANIFEST.in template which it shouldn't, it should use the existing MANIFEST, it also emits a warning about not overwriting a manually maintained MANIFEST which it shouldn't. It should just use the existing MANIFEST. -- Added file: http://bugs.python.org/file20672/distutils_bug.tar.gz ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
Changes by John Dennis : Added file: http://bugs.python.org/file20659/sdist_new ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
Changes by John Dennis : Added file: http://bugs.python.org/file20658/sdist.patch ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
Changes by John Dennis : Removed file: http://bugs.python.org/file20656/sdist_new ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
Changes by John Dennis : Removed file: http://bugs.python.org/file20655/sdist.patch ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
Changes by John Dennis : Added file: http://bugs.python.org/file20656/sdist_new ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
Changes by John Dennis : -- keywords: +patch Added file: http://bugs.python.org/file20655/sdist.patch ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11104] distutils sdist ignores MANIFEST
New submission from John Dennis : The behaviour of sdist has changed dramatically in Python 2.7. Some projects prefer to maintain their own manifest file instead of utilizing automatic manifest generation from a template. The defined behaviour of sdist is to check for the presence of both a template and a manifest, if the template is absent but a manifest exists sdist is supposed to read the existing manifest, it no longer does this. Instead it creates a default file list with the catastrophic result of omitting the bulk of a projects files. It appears this bug was introduced in r81255 and is discussed in #8688. Unfortunately #8688 contained a number of different issues and was closed addressing only a subset of the problems. Changeset r83996 was introduced to prevent sdist from overwriting a project maintained manifest by testing for a comment at the head of the manifest file. It's not clear to me this was necessary because the write_manifest() should never have been called if the template was absent but a manifest existed. Even after the application of changeset r83996 one of the fundamental problems in #8688 remained, the manifest is not read. The solution is to check for both the manifest and template (as was formerly done) and if the template is absent but the manifest exists then the manifest should be read. I have made modifications to get_file_list() to reintroduce the defined behaviour. With the introduction of r83996 it is now legal syntax to have comments in the manifest however read_manifest() was not enhanced to account for the possible presence of comments. I also modified read_manifest() to handle comments. These suggested fixes are attached as a patch against the current 2.7 maintenance branch. I've also attached a file with the two modified methods because sometimes it's difficult to comprehend a patch. -- assignee: tarek components: Distutils messages: 12 nosy: eric.araujo, jdennis, tarek priority: normal severity: normal status: open title: distutils sdist ignores MANIFEST type: behavior versions: Python 2.7 ___ Python tracker <http://bugs.python.org/issue11104> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com