[Patches] [ python-Patches-1117398 ] cookielib LWPCookieJar and MozillaCookieJar exceptions

SourceForge.net noreply at sourceforge.net
Sat Mar 5 16:28:23 CET 2005


Patches item #1117398, was opened at 2005-02-06 17:39
Message generated for change (Comment added) made by jjlee
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1117398&group_id=5470

Category: Library (Lib)
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: John J Lee (jjlee)
Assigned to: Nobody/Anonymous (nobody)
Summary: cookielib LWPCookieJar and MozillaCookieJar exceptions

Initial Comment:
cookielib.LWPCookieJar and .MozillaCookieJar are
documented to raise cookielib.LoadError on attempt to
load an invalid cookies file, but do not.

I think this should be backported to the 2.4
maintenance branch.
Reason: I suspect more people will be bitten by the bug
than will be bitten by the patch, since cookies files
will rarely be invalid, so people are likely to have
written except statements based on the docs in this
case, rather than based on the actual exception that
currently occurs.


----------------------------------------------------------------------

>Comment By: John J Lee (jjlee)
Date: 2005-03-05 15:28

Message:
Logged In: YES 
user_id=261020

This comment is not relevant to the patch, just a comment on
my own comment:

> Even then, I wonder if it would have received any
significant testing in practice from people other than me,
since people might not have bothered to switch from the
backwards-compatible (<=2.4) version.

Hmm, that's not true: four bugs might have been picked up if
I'd released a final version of the <=2.4
backwards-compatible ClientCookie 0.9 version (with the
interface changes from ClientCookie 0.4 to cookielib), then
released a ClientCookie 1.0 after Python 2.4 came out (there
was enough time without needing to wait for 2.5).  That's
what I should have done, instead of trying to protect
ClientCookie users from two rounds of interface changes. 
Live and learn, I suppose.

OTOH, no guilt on 1117339 or 1028908 (except regret that I
was suddenly unable to work on it for a period up to the 2.4
beta release, hence missing deadline to get the latter
applied before it was released).


----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-03-04 23:56

Message:
Logged In: YES 
user_id=261020

Revised patch attached.


----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-03-04 18:39

Message:
Logged In: YES 
user_id=261020

Re IOError: OK, I'll submit a revised patch.

Yes, I agree on your first para, with hindsight.  I
attempted to avoid making users change interfaces twice
(once for a release of a Python 2.4 candidate, once for 2.4
itself).  Stupid idea, especially since the changes (though
"minor") touched a lot of code.  Even then, I wonder if it
would have received any significant testing in practice from
people other than me, since people might not have bothered
to switch from the backwards-compatible (<=2.4) version.

Thanks for your attention to these bugs.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2005-03-04 14:52

Message:
Logged In: YES 
user_id=21627

I see. In retrospect, it might have been best to reject the
cookielib for Python 2.4, and wait for it to get a stable
interface and implementation. Without the time machine, we
have to accept the consequences, though, so we cannot break
existing code.

Therefore, I now propose that LoadError becomes a subclass a
*permanent* subclass of IOError, thus preserving the
historical interface.

----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-03-04 14:40

Message:
Logged In: YES 
user_id=261020

>>> open('bad_cookies.txt', 'w').write("blah")
>>> import cookielib
>>> mcj = cookielib.MozillaCookieJar(filename="bad_cookies.txt")
>>> mcj.load()
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "c:\Python24\lib\cookielib.py", line 1727, in load
    self._really_load(f, filename, ignore_discard,
ignore_expires)
  File "c:\Python24\lib\_MozillaCookieJar.py", line 53, in
_really_load
    raise IOError(
IOError: bad_cookies.txt does not look like a Netscape
format cookies file
>>>


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2005-03-04 14:27

Message:
Logged In: YES 
user_id=21627

Can you give an example of an invalid cookies file? How does
the library respond  to it right now?

----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-03-04 14:16

Message:
Logged In: YES 
user_id=261020

LoadError is not currently not used anywhere.

Without LoadError, how would one distinguish (for the
purpose of error handling) an error due to, on the one hand,
a missing file, insufficient permissions, etc. (IOError)
from an error due, on the other hand, to an invalid cookies
file (LoadError)?  This is already a problem with IOError,
of course, but I don't see why we should make it worse.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2005-03-03 11:03

Message:
Logged In: YES 
user_id=21627

Is it true that LoadError is currently not used anywhere? If
so, I think it would be best to eliminate this exception,
both from the code and the documentation. To support code
that might refer to LoadError already, you could make it a
synonym to IOError.

----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-02-11 18:57

Message:
Logged In: YES 
user_id=261020

I agree the backport (but not the trunk) should have
LoadError derive from IOError.

I'd be happy for comments to go in noting the change in all
places where LoadError is raised, if that's considered good
practice.

Any committers reading: should I submit an updated patch
with just those added comments?  Should I submit a patch for
the backport, with just that change plus the added (IOError)
for backwards-compat.?

(Jim: Besides the point for the matter at hand, but I should
point out that the "pretty-for-printing" cookielib docs and
the docstrings in cookielib.py form almost disjoint sets. 
Most of the documentation lives under Doc/lib, the stuff in
the module source is the more obscure detail.  And of
course, you have just demonstrated to yourself how not
reading the docs leaves you with an incomplete understanding
of the intent, which can be useful!  Not that I *ever* do
that, of course ;-)


----------------------------------------------------------------------

Comment By: Jim Jewett (jimjjewett)
Date: 2005-02-11 16:06

Message:
Logged In: YES 
user_id=764593

The first sentence was pointing out that many people 
(including me) never see the pretty-for-printing 
documentation, and even the html form isn't generally 
convenient when I'm actually programming.  So I rely on 
the introspection tools.  I see object names, signatures, 
and docstrings.  If I need more information, I look at the 
code, which raises IOError.

While I don't yet have code catching this particular error, I 
do write in a way that would break; it wouldn't have 
occurred to me to put both types of error in an except 
clause just in case it changed later.  (Well, unless the 
docstring or at least a comment in the code itself warned 
me to.)

So I would definately prefer that it remain (at least a 
subclass of) IOError for at least 2.4.x  I would also 
appreciate comments in the fixed 2.4 code if it is going to 
change for 2.5.


----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-02-10 22:25

Message:
Logged In: YES 
user_id=261020

Jim, I don't understand the first sentence in your comment.

Re a 2.4 backport that makes LoadError derive from IOError:
it makes me wince, but I can't think of an argument against it.

No, LoadError should not be a subclass of IOError in the
trunk, because the cases where LoadError is documented to be
raised do not involve failures of I/O, but rather invalid
data.  See the docs for IOError.  (FWIW, EnvironmentError
(IOError's base class) wouldn't be a suitable subclass
either: eg. what would we want with an .errno attribute?)


----------------------------------------------------------------------

Comment By: Jim Jewett (jimjjewett)
Date: 2005-02-08 16:58

Message:
Logged In: YES 
user_id=764593

I often look at the code in a second idle window rather 
than starting a web browser.  

Would it work to make LoadError a subclass of IOError, at 
least for the backport?  People who followed the docs will 
get a bugfix, but people who followed the code would get 
no breakage.

Should LoadError be a subclass of IOError even in the 
main trunk?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1117398&group_id=5470


More information about the Patches mailing list