502644 - Links in "view source" for IDN pages refer wrong URIs

Status: RESOLVED FIXED

(Toolkit :: View Source, defect, P2)

Description

[YUKI "Piro" Hiroshi]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.1pre) Gecko/20090704 Shiretoko/3.5.1pre (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.1pre) Gecko/20090704 Shiretoko/3.5.1pre (.NET CLR 3.5.30729)

In "view source", "href" and other attributes are linkified automatically. However, if the URI of the original page has IDN, most links don't work correctly.

Reproducible: Always

Steps to Reproduce:
1. Go to "http://例え.テスト/" (this is an IDN, punycode version is "http://xn--r8jz45g.xn--zckzah/")
2. "View" => "Page Source"
3. Click a link in the tag:
   <link rel="shortcut icon" href="/images/favicon.ico" />

Actual Results:  
Firefox tries to load "view-source:http://%c3%a4%c2%be%c2%8b%c3%a3%c2%81%c2%88.%c3%a3%c2%83%c2%86%c3%a3%c2%82%c2%b9%c3%a3%c2%83%c2%88/images/favicon.ico" and shows "Server not found" error.

Expected Results:  
Firefox loads "view-source:http://xn--r8jz45g.xn--zckzah/images/favicon.ico".

Comment 1

[YUKI "Piro" Hiroshi]

FYI: the protocol handler for "view-source:" uses "asciiSpec" instead of "spec", to get punycode version of the URI.
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/viewsource/src/nsViewSourceHandler.cpp#78

Comment 2

[Kevin Brosnan]

Confirmed on Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

Comment 3

[petr.pisar]

I can confirm this bug on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a2pre) Gecko/20100216 Minefield/3.7a2pre.

This bug appears only if used top level domain is whitelisted to interprest punycode.

Comment 5

[Kai de Leeuw]

I just noticed this bug on http://företag.stockholm.se/

Comment 6

[Niklas Bölter]

Attachment: Fetch the absolute URL, convert it according to IDNA

I can confirm this with latest hg checkout, patch attached.

I use the same method as documented in /netwerk/protocol/viewsource/nsViewSourceHandler.cpp, Line 85

Comment 8

[Boris Zbarsky [:bzbarsky]]

Hmm.  But GetSpec should work as well, I'd think, as long as the encoding is not confused....  Is the problem in the output of GetSpec (in that it's %-escaping inside the hostname before returning the value) or in what code later on does with the string GetSpec produced?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Niklas, ping?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 504150 [details] [diff] [review]
Fetch the absolute URL, convert it according to IDNA

I believe the real problem is here, further down in the method in question:

  viewSourceUrl.AppendWithConversion(absoluteLinkUrl);

That converts absoluteLinkUrl to UTF-16 by byte-inflating instead of doing a proper UTF-8 to UTF-16 conversion.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Do the append right

And indeed, fixing that fixes the behavior of the steps from comment 0.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Stealing so I can track this.

Niklas, are you OK with me crediting you for the fix from comment 11?  It's basically your work, since you found where things are going wrong....

Comment 13

[Niklas Bölter]

Hey, sorry for not replying to your questions but i didn't have the sourcecode checked out anymore and i thought i would do it when i had time.

I didn't really understood what went wrong, i just applied the same method that was used in nsViewSourceHandler.cpp. As long as it is now fixed (correctly even), i'm happy. Credit is fine, no credit is also fine. Thanks for looking at it. :)

Comment 14

[Boris Zbarsky [:bzbarsky]]

No problem.  Now all I need is to get Blake to review... ;)

Comment 15

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/d8c12f825895