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

RESOLVED FIXED in mozilla6

Status

()

Toolkit
View Source
P2
normal
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: YUKI "Piro" Hiroshi, Assigned: Niklas Bölter)

Tracking

Trunk
mozilla6
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
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".
(Reporter)

Comment 1

9 years ago
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

Updated

9 years ago
Component: General → View Source
Product: Firefox → Toolkit
QA Contact: general → view.source
Version: unspecified → 1.9.1 Branch

Comment 2

9 years ago
Confirmed on Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

8 years ago
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.

Updated

8 years ago
Duplicate of this bug: 556815

Updated

7 years ago
Version: 1.9.1 Branch → Trunk

Updated

7 years ago
OS: Windows Vista → All
Hardware: x86 → All

Comment 5

7 years ago
I just noticed this bug on http://företag.stockholm.se/
(Assignee)

Comment 6

7 years ago
Created attachment 504150 [details] [diff] [review]
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
(Assignee)

Updated

7 years ago
Attachment #504150 - Flags: review?(jst)

Updated

7 years ago
Assignee: nobody → niklas
Duplicate of this bug: 645129
(Assignee)

Updated

7 years ago
Attachment #504150 - Flags: review?(jst) → review?(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?
Niklas, ping?
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.
Attachment #504150 - Flags: review?(bzbarsky) → review-
Blocks: 17612
Created attachment 527722 [details] [diff] [review]
Do the append right

And indeed, fixing that fixes the behavior of the steps from comment 0.
Attachment #527722 - Flags: review?(mrbkap)
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....
Assignee: niklas → bzbarsky
Priority: -- → P2
Whiteboard: [need review]
(Assignee)

Comment 13

7 years ago
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. :)
No problem.  Now all I need is to get Blake to review... ;)

Updated

7 years ago
Attachment #527722 - Flags: review?(mrbkap) → review+
Whiteboard: [need review] → [need landing]
Pushed http://hg.mozilla.org/mozilla-central/rev/d8c12f825895
Assignee: bzbarsky → niklas
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [need landing]
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.