Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In http.cookiejar.FileCookieJar() the .load() and .revert() methods don't work #61105

Closed
py-user mannequin opened this issue Jan 8, 2013 · 7 comments
Closed

In http.cookiejar.FileCookieJar() the .load() and .revert() methods don't work #61105

py-user mannequin opened this issue Jan 8, 2013 · 7 comments
Assignees
Labels
docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@py-user
Copy link
Mannequin

py-user mannequin commented Jan 8, 2013

BPO 16901
Nosy @terryjreedy, @asvetlov, @py-user, @vadmium, @demianbrecht
Files
  • cookiejar_16901.patch
  • 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:

    assignee = None
    closed_at = None
    created_at = <Date 2013-01-08.23:37:01.871>
    labels = ['type-bug', 'library', 'docs']
    title = "In http.cookiejar.FileCookieJar() the .load() and .revert() methods don't work"
    updated_at = <Date 2013-06-18.06:43:52.975>
    user = 'https://github.com/py-user'

    bugs.python.org fields:

    activity = <Date 2013-06-18.06:43:52.975>
    actor = 'martin.panter'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation', 'Library (Lib)']
    creation = <Date 2013-01-08.23:37:01.871>
    creator = 'py.user'
    dependencies = []
    files = ['29275']
    hgrepos = []
    issue_num = 16901
    keywords = ['patch']
    message_count = 7.0
    messages = ['179398', '179755', '183195', '183196', '183206', '183208', '183235']
    nosy_count = 6.0
    nosy_names = ['terry.reedy', 'asvetlov', 'docs@python', 'py.user', 'martin.panter', 'demian.brecht']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'test needed'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue16901'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4']

    @py-user
    Copy link
    Mannequin Author

    py-user mannequin commented Jan 8, 2013

    >>> import http.cookiejar
    >>> cjf = http.cookiejar.FileCookieJar()
    >>> cjf.load('file.txt')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/local/lib/python3.3/http/cookiejar.py", line 1767, in load
        self._really_load(f, filename, ignore_discard, ignore_expires)
    AttributeError: 'FileCookieJar' object has no attribute '_really_load'
    >>> 
    >>> 
    >>> import http.cookiejar
    >>> cjf = http.cookiejar.FileCookieJar('file.txt')
    >>> cjf.load()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/local/lib/python3.3/http/cookiejar.py", line 1767, in load
        self._really_load(f, filename, ignore_discard, ignore_expires)
    AttributeError: 'FileCookieJar' object has no attribute '_really_load'
    >>>
    >>> 
    >>> cjf.revert()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/local/lib/python3.3/http/cookiejar.py", line 1789, in revert
        self.load(filename, ignore_discard, ignore_expires)
      File "/usr/local/lib/python3.3/http/cookiejar.py", line 1767, in load
        self._really_load(f, filename, ignore_discard, ignore_expires)
    AttributeError: 'FileCookieJar' object has no attribute '_really_load'
    >>>

    @py-user py-user mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 8, 2013
    @terryjreedy
    Copy link
    Member

    First, a minor issue about class signatures:
    doc: FileCookieJar(filename, delayload=None, policy=None)
    code: def __init__(self, filename=None, delayload=False, policy=None)
    Pretty clearly, doc should be changed to match code, as later code allow for possibility of filename = None (meaning that that is intentional).

    Ditto for doc for FileCookieJar subclasses (which inherit __init__):
    MozillaCookieJar(filename, delayload=None, policy=None)
    LWPCookieJar(filename, delayload=None, policy=None)


    FileCookieJar has .load which in inherited by the subclasses. It checks for a filename and opens it and then calls ._really_load. The two subclasses have customized ._really_load methods that correspond to their customized .save methods.

    FileCookieJar itself does not. If it did, it would have to somehow be 'generic'. This suggests to me that FileCookieJar was not intended to be directly used. This impression is reinforced by the definition of .save().

        def save(self, filename=None, ignore_discard=False, ignore_expires=False):
            """Save cookies to a file."""
            raise NotImplementedError()

    In other words, there is no generic format to save to *or* load from.
    There should be a corresponding ._really_load to raise the same exception.

    Bottom line: as best I understand, your code is not intended to work, but both the doc and implementation are deficient in not saying so, and both should be improved.

    @terryjreedy terryjreedy added the docs Documentation in the Doc dir label Jan 12, 2013
    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 28, 2013

    (Note: Additional context can be found here: http://bugs.python.org/issue16942, which seems to be a dupe of this report)

    I haven't had any feedback to my proposal on python-ideas about the removal of LWPCookieJar and MozillaCookieJar and the introduction of LWPCookieProcessor and MozillaCookieProcessor yet (http://thread.gmane.org/gmane.comp.python.ideas/19673), which could be indicative of this change simply not being acceptable. However, on the outside chance that's not the case, I've submitted a patch covering the proposed implementations. All tests pass.

    The patch addresses some of the oddities around FileCookieJar looking, but not behaving as an abstract class as well as inheriting from a concrete class (CookieJar). The change aligns LWPCookies and MozillaCookies with the HTTPCookies. This should fix the questions around why FileCookieJar can't be used directly. If this change looks reasonable, corresponding documentation changes will be made as well.

    Note: This change /does/ break backwards compatibility. I'm not sure what the process is for that, so if this change is eventually applied, pointers as to what should be integrated where would be helpful.

    @terryjreedy
    Copy link
    Member

    I suspect most people don't use or care much about cookies and cookie jar and do not understand the issue of proposal enough to comment. My feeling is that the patch probably breaks more than is necessary to fix the immediate problem and too much for current releases. For one, just the name change seems unnecessary. I need to look at HTTPCookies before I can say more.

    @py-user
    Copy link
    Mannequin Author

    py-user mannequin commented Feb 28, 2013

    Demian Brecht wrote:

    I haven't had any feedback to my proposal on python-ideas about the >removal of LWPCookieJar and MozillaCookieJar and the introduction of >LWPCookieProcessor and MozillaCookieProcessor yet >(http://thread.gmane.org/gmane.comp.python.ideas/19673), which could be >indicative of this change simply not being acceptable.

    all python ideas are discussed in python lists (for users and for developers)

    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 28, 2013

    @terry: I don't think that the name change is unnecessary as the patch changes the semantics of the the LWP and Mozilla Cookie classes. In the patch, they no longer /are/ a subclass of a CookieJar, but they take a CookieJar object and implement the FileCookieProcessor interface in order to save/load/revert file-based cookies.

    This solves the problem of an abstract base class (or at least, what was /intended/ to be an abstract base class prior to abcs: FileCookieJar) extending a concrete class, which doesn't make sense from an architectural standpoint. It also fixes the problem that people are encountering when attempting to instantiate a FileCookieJar object, it doesn't just patch something that's fundamentally broken. It could be my lack of experience with large scale OSS, but I'd prefer a /correct/ fix (especially to something that few actually care about and is seemingly not in heavy use) over a duct taped "just make it work" solution.

    I absolutely agree, however, that this is a rather large change (as far as the cookiejar module goes anyway) and shouldn't be taken lightly. However, the cookiejar module /is/ in need of some love and I think that this takes a step to giving it that.

    @py.user: My proposal was made to python-ideas. I just prefer gmane to Google Group for links.

    @py-user
    Copy link
    Mannequin Author

    py-user mannequin commented Feb 28, 2013

    Demian Brecht:

    My proposal was made to python-ideas.

    try this
    http://mail.python.org/mailman/listinfo/python-dev

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @JelleZijlstra JelleZijlstra self-assigned this Oct 7, 2022
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 7, 2022
    …ythonGH-95427)
    
    (cherry picked from commit 5eaf4d6)
    
    Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 7, 2022
    …ythonGH-95427)
    
    (cherry picked from commit 5eaf4d6)
    
    Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
    miss-islington added a commit that referenced this issue Oct 7, 2022
    (cherry picked from commit 5eaf4d6)
    
    Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
    miss-islington added a commit that referenced this issue Oct 8, 2022
    (cherry picked from commit 5eaf4d6)
    
    Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
    @slateny slateny closed this as completed Oct 8, 2022
    carljm added a commit to carljm/cpython that referenced this issue Oct 8, 2022
    * main: (38 commits)
      pythongh-92886: make test_ast pass with -O (assertions off) (pythonGH-98058)
      pythongh-92886: make test_coroutines pass with -O (assertions off) (pythonGH-98060)
      pythongh-57179: Add note on symlinks for os.walk (python#94799)
      pythongh-94808: Fix regex on exotic platforms (python#98036)
      pythongh-90085: Remove vestigial -t and -c timeit options (python#94941)
      pythonGH-83901: Improve Signature.bind error message for missing keyword-only params (python#95347)
      pythongh-61105: Add default param, note on using cookiejar subclass (python#95427)
      pythongh-96288: Add a sentence to `os.mkdir`'s docstring. (python#96271)
      pythongh-96073: fix backticks in NEWS entry (pythonGH-98056)
      pythongh-92886: [clinic.py] raise exception on invalid input instead of assertion (pythonGH-98051)
      pythongh-97997: Add col_offset field to tokenizer and use that for AST nodes (python#98000)
      pythonGH-88968: Reject socket that is already used as a transport (python#98010)
      pythongh-96346: Use double caching for re._compile() (python#96347)
      pythongh-91708: Revert params note in urllib.parse.urlparse table (python#96699)
      pythongh-96265: Fix some formatting in faq/design.rst (python#96924)
      pythongh-73196: Add namespace/scope clarification for inheritance section (python#92840)
      pythongh-97646: Change `.js` and `.mjs` files mimetype to conform to RFC 9239 (python#97934)
      pythongh-97923: Always run Ubuntu SSL tests with others in CI (python#97940)
      pythongh-97956: Mention `generate_global_objects.py` in `AC How-To` (python#97957)
      pythongh-96959: Update HTTP links which are redirected to HTTPS (python#98039)
      ...
    pablogsal pushed a commit that referenced this issue Oct 22, 2022
    (cherry picked from commit 5eaf4d6)
    
    Co-authored-by: Stanley <46876382+slateny@users.noreply.github.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    Development

    No branches or pull requests

    3 participants