ISO8859-1 encoding is used to upload iCal data instead of UTF-8

RESOLVED FIXED

Status

Calendar
Internal Components
--
critical
RESOLVED FIXED
12 years ago
4 years ago

People

(Reporter: 石庭豐 (Seak, Teng-Fong), Assigned: Michiel van Leeuwen (email: mvl+moz@))

Tracking

Trunk
x86
Windows XP

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

12 years ago
I'm using icalx.com to store my iCal.  I've just found that 20050831 build (and
probably quite a lot of builds before that) upload data in Latin-1 instead of
UTF-8 encoding.  For example, when I use the 'é' (e-acute) character like in
Chloé, it's displayed wrongly in icalx.com.  Downloading the raw ics file and
reading the content with a binary editor confirms this.

This bug is marked as critical because the next time Sunbird is run again, it
can no more reload the remote calendar.  Actually, that bug is more responsible
than this one, but since this is the main cause, I set this as critical.

PS: I think this bug belongs to libical. If not, please change the component.
(Assignee)

Comment 1

12 years ago
Created attachment 194704 [details] [diff] [review]
convert to/from utf8

I just totally forgot to convert to utf8 before uploading, exporting or
importing. The patch should fix that. (One day, when i understand streams, i
should clean all this messy stream usage)
Assignee: gray → mvl
Status: NEW → ASSIGNED
Attachment #194704 - Flags: first-review?(dmose)
(Assignee)

Updated

12 years ago
Blocks: 298936

Comment 2

12 years ago
Comment on attachment 194704 [details] [diff] [review]
convert to/from utf8

As per discussion on IRC, this patch seems to be fixing the symptom and not the
cause.	We need to prevent the data from ever ending up in Latin-1 in the first
place.
Attachment #194704 - Flags: first-review?(dmose) → first-review-
(Assignee)

Comment 3

12 years ago
Created attachment 195416 [details] [diff] [review]
updated patch

Updated the patch, with more comments and some symbolic constants.
The conversion isn't to convert to an utf8 string, but to a byte array. And
when downloading, to interpret the array of bytes as utf8, and convert into a
string. So this fix really is needed.
Attachment #194704 - Attachment is obsolete: true
Attachment #195416 - Flags: first-review?(dmose)
(Assignee)

Comment 4

12 years ago
Comment on attachment 195416 [details] [diff] [review]
updated patch

>+                const segmentSize = 32768 // Must be a power of 2
For review purposes, pretend i added a ; there.

Comment 5

12 years ago
Comment on attachment 195416 [details] [diff] [review]
updated patch

After irc discussion, the plan is to replace the nsIStorageStream with an
nsIPipe.
Attachment #195416 - Flags: first-review?(dmose)
(Assignee)

Comment 6

12 years ago
Created attachment 195518 [details] [diff] [review]
use nsIPipe

Patch uses nsIPipe now.
Attachment #195416 - Attachment is obsolete: true
Attachment #195518 - Flags: first-review?(dmose)

Comment 7

12 years ago
Comment on attachment 195518 [details] [diff] [review]
use nsIPipe

>+                convStream.init(pipe.outputStream, 'UTF-8', 0, '?');

Passing in '?' for the default char seems like it's asking for silent data
corruption.  How about passing in 0x0000 in order to force the convStream to
throw an exception if it sees something it doesn't understand.	And do whatever
we need to propagate any such exception out to the UI.	I'd suggest the same
change for the importer.

Otherwise, looks good.
Attachment #195518 - Flags: first-review?(dmose) → first-review-
(Assignee)

Comment 8

12 years ago
Created attachment 195781 [details] [diff] [review]
call onError

Patch calls onError when the en/decoding fails, and made the im/exporter throw.

For the onError, I added an observer that simply prompts. It needs a lot of
l10n love, but that can be done in a different bug.
(The throw from the importer is already caught, and prompts about it)
Attachment #195518 - Attachment is obsolete: true
Attachment #195781 - Flags: first-review?(dmose)

Comment 9

12 years ago
Comment on attachment 195781 [details] [diff] [review]
call onError

I'd slightly vote for using 0x0000 as documented in the interface file rather
null for the converter stream initializations.	But whatever you think is best.
 r=dmose with or without that change.
Attachment #195781 - Flags: first-review?(dmose) → first-review+
(Assignee)

Comment 10

12 years ago
Patch checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
+                // This converter is needed to convert the javascript unicode 
+                // string to an array of bytes.

nsIScriptableUnicodeConverter.convertToInputStream?
http://lxr.mozilla.org/seamonkey/source/intl/uconv/idl/nsIScriptableUConv.idl#90

+    // Convert the javascript string to an araay of bytes, using the
+    // utf8 encoder

typo "araay"

Also note... the converter in/output streams are not really services... if they
were, you couldn't have two of them at the same time!
Component: libical → Internal Components
The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!
QA Contact: libical → base
(Reporter)

Comment 13

11 years ago
What is this???  The same comment is posted on a lot of bugs, if not all. Bugzilla being spammed by robots or what?  
(In reply to comment #13)
> What is this???  The same comment is posted on a lot of bugs, if not all.
> Bugzilla being spammed by robots or what?  

Many calendar bugs in bugzilla were being moved to new components.
Putting that goofy text in the comment when moving allows you to find all messages with the unique text and delete them easily.
You need to log in before you can comment on or make changes to this bug.