Skip Menu |

This queue is for tickets about the MailTools CPAN distribution.

Report information
The Basics
Id: 91309
Status: resolved
Priority: 0/
Queue: MailTools

People
Owner: Nobody in particular
Requestors: dolmen [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 2.12
Fixed in: (no value)



Subject: Mail::Header->unfold and ->fold don't work
Two issues: - Mail::Header->unfold keeps the spaces instead of removing them. - Mail::Header->fold(72) seems to do nothing at all, at least in my test case where the header value contains no spaces. See the testcase (using Test::More) attached to fully reproduce the issue. -- Olivier Mengué - http://perlresume.org/DOLMEN
Subject: unfold-fold-2.12-failure.pl
#!perl use Test::More 0.98; use Mail::Header; my $input = <<'EOF'; Import-Package: fr.bytel.mf.device;version="3.2",fr.bytel.mf.smartobject ;version="3.2",fr.bytel.mf.util;version="3.2",javax.net.ssl;resolution: =optional,org.osgi.framework;version="1.3.0",org.osgi.service.event;ver sion="1.1.0",org.osgi.util.tracker;version="[1.3,2)" EOF open my $in, '<', \$input or die; my $head = Mail::Header->new($in, FoldLength => 72); close $in; $head->unfold; my $imppkg = $head->get('Import-Package'); #note $imppkg; unlike($imppkg, qr/ /, "no spaces!"); # Fix it $imppkg =~ s/ //g; $head->replace('Import-Package', $imppkg); # Fold back $head->fold(72); my $output = $head->as_string; like($input, qr/\n /s, 'input is folded!'); # Just to check our regexp works like($output, qr/\n /s, 'output is folded!'); done_testing;
Subject: Re: [rt.cpan.org #91309] Mail::Header->unfold and ->fold don't work
Date: Tue, 10 Dec 2013 22:58:41 +0100
To: Olivier Mengué via RT <bug-MailTools [...] rt.cpan.org>
From: Mark Overmeer <secretaris [...] nluug.nl>
* Olivier Mengué via RT (bug-MailTools@rt.cpan.org) [131210 21:00]: Show quoted text
> Tue Dec 10 16:00:48 2013: Request 91309 was acted upon. > Transaction: Ticket created by DOLMEN > > Two issues: > - Mail::Header->unfold keeps the spaces instead of removing them.
It keeps 1 space. That's because the implementation is wrong (but kept that way for backwards compatibility. The replacement rule is { $$ln =~ s/\r?\n\s+/ /sog if defined $ln && defined $$ln; } The correct implementation would be { $$ln =~ s/\r?\n\s//sog if defined $ln && defined $$ln; } It think it will work for you with { $$ln =~ s/\r?\n\s(\s*)/length $1 ? ' ' : ''/soge if defined $ln && defined $$ln; } What do you think? Show quoted text
> - Mail::Header->fold(72) seems to do nothing at all, at least in > my test case where the header value contains no spaces.
Folding is a complex algorithm... it prefers to break lines on blanks. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions drs Mark A.C.J. Overmeer MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
Le 2013-12-10 22:58:58, secretaris@nluug.nl a écrit : Show quoted text
> * Olivier Mengué via RT (bug-MailTools@rt.cpan.org) [131210 21:00]:
> > Tue Dec 10 16:00:48 2013: Request 91309 was acted upon. > > Transaction: Ticket created by DOLMEN > > > > Two issues: > > - Mail::Header->unfold keeps the spaces instead of removing them.
> > It keeps 1 space. That's because the implementation is wrong (but > kept that way for backwards compatibility. The replacement rule is > > { $$ln =~ s/\r?\n\s+/ /sog > if defined $ln && defined $$ln; > } > > The correct implementation would be > > { $$ln =~ s/\r?\n\s//sog > if defined $ln && defined $$ln; > } > > It think it will work for you with > > { $$ln =~ s/\r?\n\s(\s*)/length $1 ? ' ' : ''/soge > if defined $ln && defined $$ln; > } > > What do you think? >
> > - Mail::Header->fold(72) seems to do nothing at all, at least in > > my test case where the header value contains no spaces.
> > Folding is a complex algorithm... it prefers to break lines on blanks.
That would be wrong. What if the first character of the content on the line is effectively a space? "\s*" would delete it instead of keeping it. Besides that, using \s is wrong because \s is more than just space and tab (and is tab allowed?). If only spaces is allowed [ ] must be used. If tab too, [ \t]. -- Olivier Mengué - http://perlresume.org/DOLMEN
Le 2013-12-10 22:58:58, secretaris@nluug.nl a écrit : Show quoted text
> Folding is a complex algorithm... it prefers to break lines on blanks.
Folding in mail headers is an algorithm for computers, not for humans. For old computers with old implementations that had short line buffers of less than 80 characters. If the folding doesn't work for that case when the header value has no space, what is it useful for? A folding implementation that does not fold is even worst that if the API user is not calling it. -- Olivier Mengué - http://perlresume.org/DOLMEN
Subject: Re: [rt.cpan.org #91309] Mail::Header->unfold and ->fold don't work
Date: Fri, 13 Dec 2013 15:42:06 +0100
To: Olivier Mengué via RT <bug-MailTools [...] rt.cpan.org>
From: Mark Overmeer <solutions [...] overmeer.net>
* Olivier Mengué via RT (bug-MailTools@rt.cpan.org) [131213 14:34]: Show quoted text
> Queue: MailTools > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=91309 >
> > The correct implementation would be > > > > { $$ln =~ s/\r?\n\s//sog > > if defined $ln && defined $$ln; > > } > > > > It think it will work for you with > > > > { $$ln =~ s/\r?\n\s(\s*)/length $1 ? ' ' : ''/soge > > if defined $ln && defined $$ln; > > } > > > > What do you think? > >
> > > - Mail::Header->fold(72) seems to do nothing at all, at least in > > > my test case where the header value contains no spaces.
> > > > Folding is a complex algorithm... it prefers to break lines on blanks.
> > That would be wrong. What if the first character of the content on the > line is effectively a space? "\s*" would delete it instead of keeping it.
The folding pattern is "\r\n<space>", but I am sure that many applications use <tab> instead of <space>. That first blank of the line needs to be removed. But to stay backwards compatible, I have to replace all leading blanks. I think the intermediate solution, which fixes the unfold where the header is correct, but does not break it when it is incorrect, is best. Show quoted text
> Besides that, using \s is wrong because \s is more than just space > and tab (and is tab allowed?). If only spaces is allowed [ ] must be > used. If tab too, [ \t].
Maybe better, yes. ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
Subject: Re: [rt.cpan.org #91309] Mail::Header->unfold and ->fold don't work
Date: Fri, 13 Dec 2013 15:49:32 +0100
To: Olivier Mengué via RT <bug-MailTools [...] rt.cpan.org>
From: Mark Overmeer <solutions [...] overmeer.net>
* Olivier Mengué via RT (bug-MailTools@rt.cpan.org) [131213 14:41]: Show quoted text
> Queue: MailTools > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=91309 > > > Le 2013-12-10 22:58:58, secretaris@nluug.nl a écrit : >
> > Folding is a complex algorithm... it prefers to break lines on blanks.
> > A folding implementation that does not fold is even worst that if the > API user is not calling it.
It does fold. Read the code. Just not as you expect. The parameter is a guideline, not fixed maximum length. Besides, folding is always optional, until your lines get longer than 992 characters. MailTools first got released as one of the first modules to CPAN in 1995. The code is in many books, and that is the only reason that I maintain it: it is not good code, but has a huge number of users who can live with its current condition and mistakes. I will keep it working, but not break backwards compatibility. There are many better email libraries on CPAN. For instance, my own MailBox. Much newer: started in 2000 ;-) Strictly follows all RFCs. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
not taken