Double Carriage return breaks header ..

[John Fawcett]

Patrik Henningsson wrote:
> John Fawcett wrote:
> 
>> I found out that as I'm using amavis, the message arrives ok.
>> Amavis "corrects" the bad header and adds an additional header:
>>
>> X-Amavis-Alert: BAD HEADER Improper use of control character (char 0D
>> hex) in message header 'Subject'
>>      Subject: TEST\r\n ^
>>
>> If I take amavis out of the equation I see the same behaviour as you.
> 
> 
> I'm also running amavis and i get the "BAD HEADER" but i can't see that
> amavis is "correcting" anything for me.

I'll run some more tests.

>> I can confirm this is nothing to do with spool_copy_msg().
>> When spool_copy_msg() receives the input stream, the headers have
>> all been previous read from the stream.
>>
>> The interpretation of the end of headers is in
>> spool_fill_hdrcache(), when a call to parseheader() returns without
>> a header name filled in.
>>
>> It seems as though parseheader() will accept (apart from the correct
>> \r\n) also bare \r or bare \n as the end of a line and outputs \r\n.
>>
>> Removing bare \r instead of interpreting as "end of line" can
>> probably be addressed in parseheader(), but why have
>> logic to correct bare \r and bare \n in two different places
>> (for headers in parseheader() and for bodies in spool_copy_msg()).
>> This is a problem common to  headers and bodies so really this needs
>> a more structural change. General character checking and correcting
>> should be done by passing the stream through a filter function, then
>> parseheader() and spool_copy_msg() would only have to worry about
>> dealing with well-formed messages or carrying out checks which are
>> specific to headers or bodies.
>>
>> An advantage of this would also be to introduce the nul char
>> check into headers which is only present in spool_copy_msg().
>>
>> I think before anyone does a patch some guidance is needed from
>> a project committer to avoid going the wrong way.
> 
> 
> Should we move this thread to cyrus-devel?
yes. But I'd better sign up for it: I'd forgotten there was
a separate developers list.

John

>> John
>> ---
>> Cyrus Home Page: http://asg.web.cmu.edu/cyrus
>> Cyrus Wiki/FAQ: http://cyruswiki.andrew.cmu.edu
>> List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
> 
> 
> 
> /PH
> ---
> Cyrus Home Page: http://asg.web.cmu.edu/cyrus
> Cyrus Wiki/FAQ: http://cyruswiki.andrew.cmu.edu
> List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html

---
Cyrus Home Page: http://asg.web.cmu.edu/cyrus
Cyrus Wiki/FAQ: http://cyruswiki.andrew.cmu.edu
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html