Closed Bug 449578 Opened 16 years ago Closed 16 years ago

Wrong interpretation of escape sequence in charset iso-2022-kr

Categories

(Core :: Internationalization, defect)

x86
Linux
defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.9.1a2

People

(Reporter: cgarst, Assigned: smontagu)

References

Details

(Keywords: fixed1.8.1.17, fixed1.9.0.2, regression)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.14) Gecko/20061201 Firefox/2.0.0.14 (Ubuntu-feisty)
Build Identifier: Thunderbird 2.0.0.16 (20080724)

Thunderbird displays an unicode replacement character in front of each e-mail-header which has charset iso-2022-kr.

Reproducible: Always

Steps to Reproduce:
1. Compose an email with following To-Header: =?iso-2022-kr?q?=1B=24=29C=0E "some korean characters" =0F?= <foo@example.com>
2. View that mail in Thunderbird

Actual Results:  
There will be an additional unicode replacement character in front of the korean text

Expected Results:  
Don't show that character

In intl/uconv/ucvko/nsISO2022KRToUnicode.cpp at lines 107 and 132 there is a check if mRunLength == 0 (Length of non-ASCII run). The unicode replacement character is added if the decoder finds the escape sequence (ESC "$" ")" "C") in front of a line (where it should be, as rfc1557 states).
Attached patch Patch (obsolete) — Splinter Review
Attachment #333167 - Flags: review?(VYV03354)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Works, thanks.
Comment on attachment 333167 [details] [diff] [review]
Patch

How to prevent a XSS attack like bug 381412? Indeed, the sequence ESC "$" ")" "C" can occur any number of times per RFC 1557, I'm hesitating to allow appearing the designator twice (or more).
BTW I'm not a intl owner/peer. Is jshin inactive now?
Attached patch Improved patchSplinter Review
What do you think of this approach, only allowing ESC $)C at the beginning of a line? I am a little hesitant because I don't have any examples of actual ISO-2022-KR mails to test against, and I don't know how well existing clients that send ISO-2022-KR mails conform to RFC 1557. Christoph, maybe you can attach some samples or forward them to me?
Attachment #333167 - Attachment is obsolete: true
Attachment #333250 - Flags: review?(VYV03354)
Attachment #333167 - Flags: review?(VYV03354)
This would comply to RFC 1557 and works for me. I use gmime to create mails which uses iconv to convert from the internal utf-8 representation to a best fitting charset (my characters are japanese, but gmime encodes to iso-2022-kr). I don't have another sample and i don't know how wide spread this charset really is (this bug would have been found before).
Comment on attachment 333250 [details] [diff] [review]
Improved patch

Sorry for the late response.
I think it's a reasonable compromise. ISO-2022-KR is rarely used in the real world.
Attachment #333250 - Flags: review?(VYV03354) → review+
Keywords: checkin-needed
Pushed as 17110:b17a3d22447e.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2
Comment on attachment 333250 [details] [diff] [review]
Improved patch

This is a regression from bug 381412, so we need the fix on the branches where that went in.
The testcase includes a test for the regression and for issues similar to those in bug 381412.
Attachment #333250 - Flags: approval1.9.0.2?
Attachment #333250 - Flags: approval1.8.1.17?
Keywords: regression
Blocks: 381412
Comment on attachment 333250 [details] [diff] [review]
Improved patch

Approved for 1.8.1.17 and 1.9.0.2, a=dveditz for release-drivers.
Attachment #333250 - Flags: approval1.9.0.2?
Attachment #333250 - Flags: approval1.9.0.2+
Attachment #333250 - Flags: approval1.8.1.17?
Attachment #333250 - Flags: approval1.8.1.17+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: