james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Burrell Donkin" <robertburrelldon...@gmail.com>
Subject Re: com.sun.mail.util.CRLFOutputStream alternatives
Date Sat, 21 Jun 2008 15:20:23 GMT
On Sat, Jun 21, 2008 at 3:39 PM, Stefano Bagnara <apache@bago.org> wrote:
> Robert Burrell Donkin ha scritto:
>>
>> On 6/20/08, Stefano Bagnara <apache@bago.org> wrote:
>>>
>>> Stefano Bagnara ha scritto:
>>>>
>>>> Robert Burrell Donkin ha scritto:
>>>>>
>>>>> On 6/20/08, Stefano Bagnara <apache@bago.org> wrote:
>>>>>>
>>>>>> Robert Burrell Donkin ha scritto:
>>>>>>>
>>>>>>> On Thu, Jun 19, 2008 at 7:20 AM, Bernd Fondermann
>>>>>>> <bernd.fondermann@googlemail.com> wrote:
>>>>>>>>
>>>>>>>> On Wed, Jun 18, 2008 at 1:34 PM, Stefano Bagnara <apache@bago.org>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> I found the org.apache.commons.net.io.ToNetASCIIOutputStream
class
>>>>>>>>> in
>>>>>>>>> commons-net, should we use it instead of
>>>>>>>>> com.sun.mail.util.CRLFOutputStream?
>>>>>>>>> We currently have almost 10 classes  in various modules
depending
>>>>>>>>> on
>>>>>>>>> CRLFOutputStream.
>>>>>>>>>
>>>>>>>>> Stefano
>>>>>>>>
>>>>>>>> Its worth giving a try.
>>>>>>>> +1
>>>>>>>
>>>>>>> hmmm...
>>>>>>>
>>>>>>> shouldn't creating this be very straightforward? what am i missing?
>>>>>>>
>>>>>>> - robert
>>>>>>
>>>>>> True. I'm not in a "coding" period and this is the kind of code I
>>>>>> should
>>>>>> not really write in days like this because my bugs x line ratio is
too
>>>>>> high... I can try to give it a slot, but I warned you! ;-)
>>>>>
>>>>> Sounds good to me
>>>>
>>>> Done, please review.
>>
>> That'll have to wait till I'm back at home
>>
>>>> Maybe we should also implement the methods for buffers "filters" and not
>>>> only the byte per byte method.
>>
>> Yes
>
> I just committed a new version, with optimized code for buffer operations.
> I also improved ExtraDotOutputStream to use CRLFOutputStream as the base
> class and support buffer operations.
>
>>>> I can do this, but is there a good way to test it with specific chunk
>>>> sizes?
>>
>> (not sure if this anwsers your question or not but I'm sure you'll
>> shout if it doesn't)
>>
>> I think it's best to focus on boundary conditions (corner cases) then
>> construct integratuon tests based on bulk real life data (I like
>> sonnets by the bard from Project Gutenburg).
>
> I added a byte per byte test and a test with mixed sizes for the chunks and
> having buffers ending/starting/splitting with CR LF or CRLF to both the
> CRLFOutputStream and the improved ExtraDotOutputStream.
>
>>> I also have another doubt.
>>> Currently the CRLFOutputStream needs a checkCRLFTerminator call before
>>> closing or it will not correctly fix a sequence ending in "\r" alone.
>>>
>>> This is an issue because you have to take care of this call that is not
>>> a standard for filteroutputstreams.
>>
>> I think this should be addressed
>
> I did the change but I left the checkCRLFTerminator call in place because it
> is also used to add a final CRLF when there is no newline char at the end of
> the stream. But at least even if you don't call it the resulting stream will
> always have good CRLF.
>
>>> I could change the algorythm so to replace "\r" with "\r\n", ignore "\n"
>>> if the previous one was "\r" and replace "\n" with "\r\n" otherwise.
>>>
>>> This wouldn't need the final check method call but this would mean that
>>> we pass \r\n when we only received \r: any drawback with this?
>>
>> I think I've used this strategy in the past. Anyone else see anything
>> wrong?
>
> Implemented and committed.

looks good to me :-)

does anyone have any suggestions for improvements?

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message