322226 - iCalendar exports don't use CRLF

Status: NEW

(Bugzilla :: Query/Bug List, defect, P1)

Description

[Cain Random]

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0b; Windows NT 5.1; Hotbar 4.1.8.0; .NET CLR 1.1.4322; .NET CLR 2.0.50215; InfoPath.1)
Build Identifier: 

I'm writing a calendaring agent, and I ran a Bugzilla iCal export through it just for hoots, and it didn't take.  When I looked more closely, that was because the lines in Bugzilla's exports are delimited with naked LF's.  I understand that this is probably a Windows/Unix difference, but the iCal standard specifically states that lines need to end in CRLF.

Reproducible: Always

Comment 1

[Max Kanat-Alexander]

That sounds like it could be true.

Anybody know if there's any way to make CGI output CRLF for the content instead of just LF?

Comment 2

[Cain Random]

If anybody cares, the relevant document is RFC 2445, 4.1 (page 13).

Comment 3

[Gervase Markham [:gerv]]

Max: I suspect it's less CGI and more Template Toolkit. And I also suspect it's merely a matter of downloading the template, changing the LFs to CRLFs, and checking it in.

This, however, is inherently fragile, because a lot of people have their editors set up to convert loaded files to LF. So, alternatives:

- a TT2 patch to implement a directive to set line ending style
- a TT2 filter/plugin we could invoke
- defining a CR variable and using [% CR %] at the end of each relevant line

This last is nasty, but it's the easiest one to implement and I seem to remember we've used things like it before to preserve whitespace at the end of lines.

Gerv

Comment 4

[Max Kanat-Alexander]

I think the second one is the way to go, one way or another.

Comment 5

[victory <[e-mail]>]

Attachment: patch for tip

Comment 6

[Max Kanat-Alexander]

Comment on attachment 235871 [details] [diff] [review]
patch for tip

Okay, when I open the downloaded output in vim, it says it's in DOS format, so that looks right to me.

Comment 7

[victory <[e-mail]>]

it seems the same patch can be used for 2.22 :-D

patching file Bugzilla/Template.pm
Hunk #1 succeeded at 398 (offset -228 lines).

patching file template/en/default/list/list.ics.tmpl
Hunk #2 succeeded at 54 with fuzz 1.

Comment 8

[Frédéric Buclin]

Comment on attachment 235871 [details] [diff] [review]
patch for tip

>Index: Bugzilla/Util.pm

>+sub crlf {
>+    my ($var) = (@_);
>+    $var =~ s/\r\n/\n/g;
>+    $var =~ s/\n\r/\n/g;
>+    $var =~ s/\r/\n/g;
>+    $var =~ s/\n/\r\n/g;
>+    return $var;
>+}

This routine has no POD. Also, wouldn't 'dos_format' be a bit more explicit than 'crlf'?

Comment 9

[Frédéric Buclin]

Comment on attachment 235871 [details] [diff] [review]
patch for tip

Per my previous comment.

Comment 10

[victory <[e-mail]>]

Attachment: patch v2 for 2.22

Comment 11

[Dave Miller [:justdave]]

er, get a review first!

Comment 12

[Max Kanat-Alexander]

Comment on attachment 235928 [details] [diff] [review]
patch v2 for 2.22

>+=item C<dos_line_endings>
>+
>+Force CRLF (aka DOS line endings) as line endings.

  To what?? Mention the input string.

  Looks fine otherwise.

Comment 13

[Frédéric Buclin]

The 2.22 branch is restricted to security bugs -> 3.0.

I don't get it. mkanat granted review but the patch is now marked as obsolete. Is the patch broken or is it another example of victory's crisis?

Comment 14

[Max Kanat-Alexander]

Comment on attachment 235928 [details] [diff] [review]
patch v2 for 2.22

Yeah, that was some weird mistake by maki.

Comment 15

[Frédéric Buclin]

The Bugzilla 3.0 branch is now locked to security bugs and dataloss fixes only. This bug doesn't fit into one of these two categories and is retargetted to 3.2 as part of a mass-change. To catch bugmails related to this mass-change, use lts081207 in your email client filter.

Comment 16

[Max Kanat-Alexander]

Comment on attachment 235928 [details] [diff] [review]
patch v2 for 2.22

Perhaps there could be some switch we could pass to template->process to ensure we get CRLF line endings, instead of using a filter?

Comment 17

[Bradley Baetz (:bbaetz)]

(In reply to comment #16)
> (From update of attachment 235928 [details] [diff] [review])
> Perhaps there could be some switch we could pass to template->process to ensure
> we get CRLF line endings, instead of using a filter?

Not really, plus its possible that you may want this for only part of a file.

something like |s/[\r\n]+$/\r\n/mg| may be a better regexp, though, since it'll work regardless of the native line ending, at least for what we care about (Untested)

Comment 18

[Max Kanat-Alexander]

(In reply to comment #17)
> Not really, plus its possible that you may want this for only part of a file.

  There are no files currently in Bugzilla that we would want it for only part of the file, so I'm not worried about that. I actually can't imagine a file where we'd want it for only part, actually.

Comment 19

[Bradley Baetz (:bbaetz)]

> I actually can't imagine a file where we'd want it for only part, actually.

Neither. (CDATA of some sort??)

Anyway, since TT uses whats in the template source (which is unix line endings for bugzilla) rather than generating its own, a FILTER is the way to go.

Comment 20

[Max Kanat-Alexander]

(In reply to comment #19)
> Anyway, since TT uses whats in the template source (which is unix line endings
> for bugzilla) rather than generating its own, a FILTER is the way to go.

  We could also just override process() or just filter the data when it comes back (like we do for emails now).

Comment 21

[Bradley Baetz (:bbaetz)]

Why not filter, though? Its what we're trying to do, and we're not trying to replace TT as much as we're trying to replace our templates. We could always save the template with dos line endings.....

Comment 22

[Frédéric Buclin]

Bugzilla 3.4 is now restricted to security bugs. We will retarget this bug (to 3.6 or later) when it's fixed.