Closed Bug 1338758 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: jorgk-bmo, Assigned: emk)

Details

Attachments

(1 file)

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.
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?
Flags: needinfo?(VYV03354)
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
Yeah, this is a bug if UnEscapeNonAsciiURI(). Thanks for catching this.
Flags: needinfo?(VYV03354)
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
(In reply to Masatoshi Kimura [:emk] from comment #4)
> Thanks for catching this.
Thanks for fixing this ;-)
Comment on attachment 8836325 [details]
Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI.

https://reviewboard.mozilla.org/r/111790/#review113178
Attachment #8836325 - Flags: review?(m_kato) → review+
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/9aeb203f8d6f
Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI. r=m_kato
https://hg.mozilla.org/mozilla-central/rev/9aeb203f8d6f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
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 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.
Attachment #8836325 - Flags: approval-mozilla-beta?
Attachment #8836325 - Flags: approval-mozilla-aurora?
Comment on attachment 8836325 [details]
Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI.

UnEscapeNonAsciiURI fix, aurora53+, beta52+
Attachment #8836325 - Flags: approval-mozilla-beta?
Attachment #8836325 - Flags: approval-mozilla-beta+
Attachment #8836325 - Flags: approval-mozilla-aurora?
Attachment #8836325 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.