1338758 - Passing http://example.com/?p=%E9 to nsIExternalProtocolService.loadURI() opens http://example.com/?p= (truncated)

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

Passing http://example.com/?p=%E9 to nsIExternalProtocolService.loadURI() opens http://example.com/?p= (truncated)
What else can I say ;-)

We've come here from bug 770022 comment #48. Note that nsIExternalProtocolService.loadURI() is used in Thunderbird's mail/base/content/contentAreaClick.js.

Comment 1

[Jorg K (CEST = GMT+2)]

This debug code
    printf("=== %s %s\n", urlCharset.get(), urlSpec.get());
    if (NS_FAILED(textToSubURI->UnEscapeNonAsciiURI(urlCharset, urlSpec, utf16Spec))) {
      CopyASCIItoUTF16(urlSpec, utf16Spec);
      printf("=== failed!\n");
    }
    printf("=== %s\n", NS_ConvertUTF16toUTF8(utf16Spec).get());

gives:
=== UTF-8 http://example.com/?p=%E9
=== http://example.com/?p=

This seems to be an error in UnEscapeNonAsciiURI(). Masatoshi-san, care to take a look?

Comment 2

[Jorg K (CEST = GMT+2)]

I should have mentioned, I added this here:
https://dxr.mozilla.org/mozilla-central/rev/855e6b2f6199189f37cea093cbdd1735e297e8aa/uriloader/exthandler/win/nsMIMEInfoWin.cpp#238

Comment 3

[Jorg K (CEST = GMT+2)]

More examples where the click works since UnEscapeNonAsciiURI() does report the error:

=== UTF-8 http://example.com/?p=%E9e
=== failed!
=== http://example.com/?p=%E9e

=== UTF-8 http://example.com/?p=%E9%E9
=== failed!
=== http://example.com/?p=%E9%E9

Comment 4

[Masatoshi Kimura [:emk]]

Yeah, this is a bug if UnEscapeNonAsciiURI(). Thanks for catching this.

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI.

Review commit: https://reviewboard.mozilla.org/r/111790/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111790/

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to Masatoshi Kimura [:emk] from comment #4)
> Thanks for catching this.
Thanks for fixing this ;-)

Comment 7

[Makoto Kato [:m_kato] (fever, slow response)]

Comment on attachment 8836325 [details]
Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI.

https://reviewboard.mozilla.org/r/111790/#review113178

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9aeb203f8d6f
Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI. r=m_kato

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9aeb203f8d6f

Comment 10

[Jorg K (CEST = GMT+2)]

Masatoshi-san, thanks again for fixing this. Could you please request uplifts. This is essentially a one-line change which is well-covered by tests, so it shouldn't be risky to uplift.

Comment 11

[Masatoshi Kimura [:emk]]

Comment on attachment 8836325 [details]
Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: Sometimes ncorrect URLs will open when clicking some URLs on an e-mail view.
[Is this code covered by automated tests?]: Yes, unit tests added.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This is essentially a one-line change which is well-covered by tests.
[String changes made/needed]: None.

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8836325 [details]
Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI.

UnEscapeNonAsciiURI fix, aurora53+, beta52+

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/64915a68d190

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3d077ece238f

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/3d077ece238f