227873 - semicolons in vcard fields

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Andree Jacobson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6b) Gecko/20031110
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6b) Gecko/20031110

Adding a semicolon to fields in the vCard have the following consequences
("it" is referring to the semicolon):

Title field: 
  Replaces anything before semicolon with whatever is after it

Department field: 
  Erases whatever was written after it, and including itself

Organization field:
  Replaces the Department field with whatever was written after it

Address field 1:
  Places whatever was written after it in the City field, and pushes all other
fields (up to the Web Page field) in front of it.

Address field 2:
  See Address field 1

City field:
  Similar behavior as Address field, but moves whatever is after it to the next
field and keeps pushing.

State/Province field:
  Same behavior as City field

Zip/Postal field:
  Same behavior as above

Country:
  Removes whatever is written after it

Did not check any other fields, but I guess semicolons are used as field
separators or something. 


Reproducible: Always

Steps to Reproduce:
1. Open the vcard dialogue
2. Enter something in fields as described above
3. Save vcard, and open the dialogue again...

Actual Results:  
See details above, i.e., fields get reorganized and semicolons disappear.

Expected Results:  
Should have saved semicolons as part of the text field and kept fields intact as
entered.

Since semicolons are field delimiters for vcards, perhaps they should be escaped
when stored...

Comment 1

[Jo Hermans]

According to RFC2426 §2.4.2, semicolons must be escaped.

Comment 2

[Hugh Kennedy]

to quote from the RFC:

   In addition, any COMMA character (ASCII decimal 44),
   SEMI-COLON character (ASCII decimal 59) and COLON character (ASCII
   decimal 58) MUST be escaped with the BACKSLASH character (ASCII
   decimal 92).

so this will go beyond just the semi's.

Comment 3

[Andrew Taylor]

Attachment: Escape semi-colons, colons, and commas preceded with a backslash

Here is a simple patch to the vcard parser that allows semi-colons, colons, and
commas to be escaped with a backslash.

Comment 4

[Jo Hermans]

example of an incorrect VCard (from Netscape 4.72 !!!) :

begin:vcard 
n:De Wit;Luc
tel;work:[phone]
x-mozilla-html:FALSE
adr:;;;;;;
version:2.1
email;internet:[e-mail]
x-mozilla-cpt:;-10720
fn:Someone
end:vcard

the 'version:2.1' was displayed inside the second address-line.

Comment 5

[David :Bienvenu]

Comment on attachment 145638 [details] [diff] [review]
Escape semi-colons, colons, and commas preceded with a backslash

so we're supposed to treat all other \x as \x and not x? And \\ is supposed to
be \\? If so, then this looks OK.

Comment 6

[Andrew Taylor]

Attachment: Escape backslashes, semi-colons, colons, and commas preceded with a backslash and expand \n newlines

Oops, the RFC does specify \\ represents \, plus it also says \n represents a
newline.  Here is an updated patch that also handles \\ and \n escapes.

Comment 7

[David :Bienvenu]

Comment on attachment 159000 [details] [diff] [review]
Escape backslashes, semi-colons, colons, and commas preceded with a backslash and expand \n newlines

need to handle \N as well, according to the RFC, so it can just be else if (a
== 'n' || (a == 'N') - r=bienvenu with that change.

Comment 8

[Andrew Taylor]

Attachment: Backslash escape patch v3 - handle \N as well as \n

Updated patch to handle \N.

Comment 9

[[e-mail]]

Do we actually write vcards with embedded semicolons correctly?

Comment 10

[Andrew Taylor]

I just tested vCard output, and semicolons are not escaped there.

Comment 11

[Frank Wein [:mcsmurf]]

What's the state of this reviewed patch here? Does it need re-working or does it
just need to be checked in?

Comment 12

[Andrew Taylor]

It has a review/superreview, so it's ready to go.  Someone with CVS access needs
to check it in.

Comment 13

[Jean-Marc Desperrier]

Andrew, when a bug ony needs a check-in, you should push, and bug the people
with the rights to do it so that it happens. 
Now, the tree is frozen so it won't happen, and I don't know if it will be
considered important enough to get a chance before 1.9, you missed a large
window of opportunities since the SR time.

Comment 14

[Scott MacGregor]

Comment on attachment 159001 [details] [diff] [review]
Backslash escape patch v3 - handle \N as well as \n

I just noticed this ancient patch that has r/sr.

Comment 15

[Scott MacGregor]

thanks for the patch Andrew. 

Comment 16

[franCk]

Shouldn't a follow-up bug be filled on comment #10? 
If I understand the comments well, now we read the vcard correctly, but we don't
write them correctly (; not escaped) - shouldn't this be fixed?
Andrew did I get your comment right?