354996 - Bad encoding of IDN (accented) domains in keychain for web form passwords

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Francois Bonzon]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7
Build Identifier: Camino 1.0.3

The "Name" property of keychain items for web form passwords on IDN domains uses a bad encoding. IDN domains are ones with accented letters. Passwords saved in such items are correctly stored and retrieved though. Only the display in Keychain Access is bad, probably because Camino and Keychain Access use a different character encoding.

Reproducible: Always

Steps to Reproduce:
1. Submit test form, see URL above

Actual Results:  
Item stored in keychain has name "www.fran√ßois.ch". Bad encoding for "www.françois.ch".

Expected Results:  
Item should be stored in keychain with name "xn--franois-xxa.ch". Safari does the same. For HTTP auth, Camino (and Safari) use the ASCII string as well.

Comment 1

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

Sounds like something for Stuart ;) at some point.

Comment 2

[Stuart Morgan]

Confirming.  Presumably there's a Gecko function to get the host in Punycode format, which would make this pretty easy to fix.

Comment 3

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

(In reply to comment #2)
> Presumably there's a Gecko function to get the host in Punycode format

Wevah seems to think there is (bug 309226 comment 7).

Comment 4

[Wevah (Nate Weaver)]

I just assumed. Maybe it's in Firefox-only code and I don't know that it is.

Comment 5

[Håkan Waara]

There's no gecko API to get the host in punycode (I just confirmed with an lxr search), so if Wevah had code to do it, let's use that?

Comment 6

[Stuart Morgan]

For my own reference, there's GetAsciiSpec for nsIURI. I may have to round-trip through nsIURI for HTTP Auth if there's not convenient API given just a host (alternately, Cocoa may have an API).

Comment 7

[Stuart Morgan]

Nevermind the part about HTTP Auth; I missed the last sentence of the initial report.

I have a fix for this, so pulling to 1.6. I'll post it after I've landed and resolved with the keychain cache patch, since it will conflict.

Comment 8

[Stuart Morgan]

Attachment: fix

This makes us compatible with Safari for IDN domains. More legacy code that will take several releases to get rid of, unfortunately :(

Comment 9

[Josh Aas]

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

+      nsCAutoString asciiHostCAString;
+      docURL->GetAsciiHost(asciiHostCAString);

You might want to check the return value here to make sure the call worked. This happens twice in the patch.

Comment 10

[Stuart Morgan]

Attachment: v2

Checks for failure, and falls back to the actual host if necessary.

Comment 11

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 268445 [details] [diff] [review]
v2

sr=pink.

should we file a bug for the cleanup TODO?

Comment 12

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH. Bug 388091 filed for the post-1.6 follow-up.