523055 - "Show Cookies" sheet doesn't appear in 2.0b4. NSCFDictionary setObject:forKey in Console on 10.6.1

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, defect)

Description

[Allan Nyholm Nielsen]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; da; rv:1.9.0.15pre) Gecko/2009091516 Camino/2.0b4 (like Firefox/3.0.15pre)
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; da; rv:1.9.0.15pre) Gecko/2009091516 Camino/2.0b4 (like Firefox/3.0.15pre)


First the Console entry:

"19/10/09 06.48.34   Camino[989]   *** -[NSCFDictionary setObject:forKey:]: attempt to insert nil value (key: Value)"

And now the details. The Show Cookies dialog sheet fails to appear after I set a cookie to log in to my online banking site with my private key from my Mac. I can easily just go to the main page of the bank and have that set a cookie but as soon as I enter the "private area" when the key from my harddrive is put and I log in and then back out - the problem arises.

I removed the cookies.sqlite file from my otherwise fresh profile and tried selecting the Show Cookies button to see if the sheet appears, and I can say that it does appear, but blank.

I can choose to have Camino remember all cookies or just from the pages I visit but it doesn't help either way. The Exception list works okay.

I was in the midst of translating Camino 2.0beta4 to Danish when I wanted to test the translation to make sure everything was working when I noticed the problem. I never thought of clicking this Show Cookies button before.


Reproducible: Always

Steps to Reproduce:
1. Go to www.sparnord.dk
2. accept to set a cookie.
3. Go to "Netbanken" in the upper right corner just below the gray bar - the button itself is blue.
3. this step is tricky because it requires a login and a private key.
4. upon entering the private site I can no longer view my cookies inside Camino's Privacy preferencepane.
Actual Results:  
No longer able to select "Show Cookies" in the Privacy section of Camino 2.0b4 - Either English or Danish on fresh profiles. Eg. no bookmarks, or other settings.

Expected Results:  
Expected results is that I am able to view my cookies by selecting "Show Cookies"

Comment 1

[Chris Lawson (gone)]

Allan, if you can reliably reproduce this with the steps you provided in comment 0, please do the following:

1) Quit Camino.
2) Move aside ~/Library/Application Support/Camino/
3) Launch Camino.
4) Run through steps 1 through 3 above, including the login.
5) Ensure that you have, indeed, caused the cookies to be un-viewable.

Once you've done that, copy the cookies.sqlite file from

~/Library/Application Support/Camino/

and e-mail it to me, and I'll investigate further.

Comment 2

[Chris Lawson (gone)]

This sounds a lot like a new version of bug 458539, but I can't figure out how else it could be happening, because I thought I fixed every possibility there.

Comment 3

[Stuart Morgan]

Domain, path, value, and date setting are all still unchecked. Maybe there's something that's not valid UTF-8 in one of those.

We should wrap the guts of the conversion method in a try block that logs and returns nil on failure.

Comment 4

[Allan Nyholm Nielsen]

Additionally.. I used a 10.4.11 Intel Mac Mini test machine using Camino2.0b4 English and after doing the steps I usually do I came to the same result.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I think we should consider blocking on this.

Comment 6

[Stuart Morgan]

We should, since it's easy to at least control the symptoms so that only the problematic cookie won't be displayed.

Comment 7

[Chris Lawson (gone)]

If SQLite Database Browser is to be believed, there is one cookie in that file that had a likely non-UTF-8 character in its "value" field (the classic sci-fi "alien head" enclosed in a box). I deleted that cookie from the file and sent the modified file to Allan. He can no longer trigger the bug with the modified file.

I probably won't be able to get to this until at least Wednesday evening, so if someone has time before then, feel free.

Comment 8

[Allan Nyholm Nielsen]

Attachment: character that appears after deleting an alien character

I think I know where the whole deboggle is - the screenshot attached to this email shows a character that appears after I delete the alien head. I'm thinking that these two characters makes out the letter "ø" (oe) because "Nørresundby" is the town in which I live. The "akr" you see is my bank advisor's initials - I'm thinking this all comes up because my bank has a feature where it shows who I'm connected to at the bank.

The Show Cookies button and sheet fails to work just by deleting the alien head, but after deleting both characters and starting Camino up again the cookies sheet appears as if nothing happened. But after logging in again on my banking site the whole thing starts over.

This is still on Camino 2.0beta4

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Chris asked me to create some testcases to try and reproduce this.  I was unable to trigger this with ø, but I could come up with a testcase that breaks us, with this exception, using some Arabic.

http://www.ardisson.org/smokey/moz/cookies/

cookie_value tries to set a cookie with a value containing ø; it's an UTF-8 page with an ambiguous charset.  This works.

cookie_value1 tries to set a cookie with the value قالA; again, UTf-8 page with ambiguous charset, and this creates the exception.

cookie_value1-utf8 is the same as above, but it declares the charset with a meta http-equiv tag.  This works (but note the cookie is displayed as B'DA).

I'm pretty sure there is an encoding problem somewhere, but I don't know where it is.  My guess is also that the site is setting a cookie in one encoding and it's being written into cookies in another encoding.

Aha!

cookie_value-win1252 uses ø and causes the bug; it's a Windows-1252 (Windows Latin 1)-encoded document, declaring Window-1252 as the charset. ø (Unicode U+00F8) gets written as U+10FEF8, if SQLite Database Browser is to be believed.

Comment 10

[Stuart Morgan]

Turns out the underlying bug is my fault. From the RFC:
      The VALUE is opaque to the user agent and may be anything the
      origin server chooses to send, possibly in a server-selected
      printable ASCII encoding.

So value is not necessarily UTF-8. It's not clear to me how we should display it given that, but we could always just pick something if UTF-8 doesn't work out I guess.

It's not clear to me what the name's encoding is, but the nsICookie header suggests it too could be non-UTF-8.

So we should be more lenient in those fields, but still go ahead and add the safety net just in case since this is the second problem we've had in this code.

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

We probably want this for 1.6.11, too, once we have a patch.

Comment 12

[Stuart Morgan]

Attachment: fix

This:
- Gives us a more reliable conversion from bytes to an NSString by doing an ISO-Latin-1 fallback
- Adds a placeholder fallback if even that fails
- Adds a try block as a safety incase there are any more similar bugs in this method
- Reorders the creation of the dictionary so that if there is an exception, the server and path will be logged to make tracking it down easier

Comment 13

[Stuart Morgan]

Attachment: 1.8 branch fix

1.8 branch version; stringWithCString:encoding: is 10.4+, but since I noticed that after I made the first patch and the init version is uglier, I figured we may as well use the new form for 2.0. The difference is trivial, so no need to get a separate sr, but it would be great if you could build this on branch to make sure I didn't miss anything else.

Comment 14

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 407220 [details] [diff] [review]
1.8 branch fix

This works with all the existing cookie testcases in this bug and bug 458539 on my PBG4 running 10.3.9.  If you have ideas of additional things to test, let me know; otherwise, r=ardissone.

Comment 15

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 407217 [details] [diff] [review]
fix

sr=pink

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Landed various patches on cvs trunk, CAMINO_2_0_BRANCH, and MOZILLA_1_8_BRANCH for Stuart (fumbled who did approval for 1.6.11, though :P ).