Closed Bug 1142083 Opened 10 years ago Closed 9 years ago

IDN Unicode domain redirect is broken in Firefox 36/37/38

Categories

(Core :: Networking: DNS, defect, P1)

36 Branch
x86_64
Windows 8.1
defect
Points:
13

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed
firefox42 + wontfix
firefox43 + wontfix
firefox44 + wontfix
firefox45 --- fixed
relnote-firefox --- 42+

People

(Reporter: kestutis.itsolutions, Assigned: valentin)

References

()

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [good first bug])

Attachments

(2 files, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150311004633

Steps to reproduce:

1. Enter http://zymiausifotografai.lt
2. Believe that should be redirected to http://www.žymiausifotografai.lt/


Actual results:

You get 404 (problem loading page) 'Server not found' error:
Firefox can't find the server at www.%c5%beymiausifotografai.lt.



Expected results:

You should be redirected to http://www.žymiausifotografai.lt/

Appears url do not converts to IDN name:
http://www.xn--ymiausifotografai-wzd.lt 
https://iwantmyname.com/domain-tools/idns/idn-punycode-converter?utf8=http%3A%2F%2Fwww.%C5%BEymiausifotografai.lt+&search=convert

It works well in Internet Explorer 11 and Google Chrome newest.
Severity: normal → blocker
Points: --- → 13
Priority: -- → P1
Whiteboard: [good first bug]
Attached image bug-idn.jpg
Bug report.
Version: 38 Branch → 36 Branch
I reproduced this with a local build and was able to fix it by commenting out a NS_EscapeURL call from nsHttpChannel.cpp:

nsresult                                                                                  
nsHttpChannel::AsyncProcessRedirection(uint32_t redirectType)
{   
    ...

    // make sure non-ASCII characters in the location header are escaped.                 
    nsAutoCString locationBuf;
    if (NS_EscapeURL(location, -1, esc_OnlyNonASCII, locationBuf))
        location = locationBuf.get();

Obviously the issue is that the host part of a URI should not be escaped using URL-encoding. That is causing the hostname to be incorrect (having %c5%be, etc.) in the subsequent request.

Does anyone know if this escaping is actually needed? In NewURI(), a nsStandardURL is constructed from the location; does that class handle Unicode URIs already?
Good catch, both of you. Let me needinfo jduell, and see if he can sort this out.
Flags: needinfo?(jduell.mcbugs)
This smells like something Valentin ought to know how to fix.
Assignee: nobody → valentin.gosu
Valentin, any thoughts here?  If you don't know we can find someone else.
Flags: needinfo?(jduell.mcbugs) → needinfo?(valentin.gosu)
(In reply to Jason Duell [:jduell] (needinfo? me) from comment #5)
> Valentin, any thoughts here?  If you don't know we can find someone else.

Yes, I've been looking into this.
Just commenting NS_EscapeURL triggers an assertion in NSS, so that's probably not good.
This will be fixed by bug 1023468 in which we start doing percent decoding for hostnames (a very requested feature).
Depends on: 1023468
Flags: needinfo?(valentin.gosu)
This is a patch to make sure we don't regress this feature. Once bug 1023468 lands, IDN domain redirect should work.
It might still be a while until we manage to land hostname percent decoding, so this a simple fix just for this one bug
Attachment #8623617 - Flags: review?(mcmanus)
This test still has a slight problem - my guess is that there is a problem in how httpd.js handles non-ascii headers.
Attachment #8623617 - Flags: review?(mcmanus) → review+
https://hg.mozilla.org/mozilla-central/rev/6c782e3a7078
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Attachment #8624913 - Flags: review?(mcmanus) → review+
Awaiting a green inbound in order to land the test.
Keywords: checkin-needed
Depends on: CVE-2015-7195
This is now unfixed on beta and aurora as it was backed out elsewhere.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
Flags: needinfo?(valentin.gosu)
Posted the site compatibility doc for reference: https://www.fxsitecompat.com/en-US/docs/2015/idn-urls-are-not-redirected-properly/

[Tracking Requested - why for this release]: See Comment 17.
Valentin, tomorrow, we are going to build beta 9. I guess this is going to be hard to get a fix, correct?

Release Note Request (optional, but appreciated)
[Why is this notable]: regression
[Suggested wording]: URLs containing a Unicode-format Internationalized Domain Name (IDN) are not redirected properly, leading to a Server Not Found error.
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
(In reply to Sylvestre Ledru [:sylvestre] from comment #19)
> Valentin, tomorrow, we are going to build beta 9. I guess this is going to
> be hard to get a fix, correct?

The fix will likely be a bit complex, and definitely not suitable for beta.
This was also talked about in bug 439616, but it hadn't reached a conclusion.
I'll try to find a smarter way to fix this.
Flags: needinfo?(valentin.gosu)
See Also: → 439616
OK, then, firefox 42 is ship with this bug. Please try to provide a fix for fx 43.
Added to the list of know issues in 42.
https://hg.mozilla.org/mozilla-central/rev/18b0d63d9f06
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Valentin, does this need to be uplifted to Beta43 and Aurora44?
Flags: needinfo?(valentin.gosu)
The patch contains only the test cases. The actual fix is in bug 412457.
Since this is a fairly large change, and the IDN redirect has been an issue for multiple years, I would rather let it ride the trains.
Flags: needinfo?(valentin.gosu)
(In reply to Valentin Gosu [:valentin] from comment #26)
> The patch contains only the test cases. The actual fix is in bug 412457.
> Since this is a fairly large change, and the IDN redirect has been an issue
> for multiple years, I would rather let it ride the trains.

Thanks Valentin. In that case, we will wontfix this for FF44.
Wontfix for 43. Sounds like we should also consider adding a release note for the fix for 45.
Flags: needinfo?(sledru)
Yes, added to the release notes with "URLs containing a Unicode-format Internationalized Domain Name (IDN) are now properly redirected" as wording for 45 (marked as fixed)
Flags: needinfo?(sledru)
Depends on: 412457
No longer depends on: 1023468
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: