Incorrect line length limitation while parsing headers

[Jesse Long <je...@virtualpostman.co.za>]

Hi,

Please see the following code in mime4j 0.6:

http://james.apache.org/mime4j/xref/org/apache/james/mime4j/parser/AbstractEntity.html#133

The for loop checks for excess line length in headers based on the 
complete length of the header, not on each individual line in the header.

So, my header like this:

X-Header-Name: <980 chars><CRLF>
<20 chars><CRLF>
<25 chars><CRLF>

fails, because the complete length of the header is > 1000, however, no 
line in the mime message is longer than 1000 chars...

I suggest just removing the check for excess line length in the 
fillFieldBuffer() method.

Cheers,
Jesse

Re: Field / Address model refactoring; was Re: 0.6.2 or 0.7 any time soon?

[Stefano Bagnara <ap...@bago.org>]

2011/1/12 Oleg Kalnichevski <ol...@apache.org>:
> ...
>
>> Why can't you simply work on trunk or on a branch here in our svn? I
>> think I encouraged you multiple times to simply do this, I don't
>> really understand why you aswer me like I'm trying to stop you.
>> My preference is for you to use trunk. We use CTR, so go ahead and I
>> hope you will be happy if I review what you do.
>
>
> Stefano et al
>
> I committed the first round of (potentially less contentious) changes to
> the DOM API refactoring branch [1]. Please review and let me know if you
> can live with these changes. The main difference with the current trunk
> is that address formatting code was factored out into a separate utility
> class akin AddressBuilder and MailboxImpl / GroupImpl classes were made
> unnecessary.

Works for me.

> If there are no vocal objections to these changes, I would like to merge
> them down to trunk and proceed with potentially more inflammatory
> changes to MessageImpl, HeaderImpl, and friends.

+1

Stefano

Field / Address model refactoring; was Re: 0.6.2 or 0.7 any time soon?

[Oleg Kalnichevski <ol...@apache.org>]

...

> Why can't you simply work on trunk or on a branch here in our svn? I
> think I encouraged you multiple times to simply do this, I don't
> really understand why you aswer me like I'm trying to stop you.
> My preference is for you to use trunk. We use CTR, so go ahead and I
> hope you will be happy if I review what you do.


Stefano et al

I committed the first round of (potentially less contentious) changes to
the DOM API refactoring branch [1]. Please review and let me know if you
can live with these changes. The main difference with the current trunk
is that address formatting code was factored out into a separate utility
class akin AddressBuilder and MailboxImpl / GroupImpl classes were made
unnecessary. 

If there are no vocal objections to these changes, I would like to merge
them down to trunk and proceed with potentially more inflammatory
changes to MessageImpl, HeaderImpl, and friends.

Oleg 

[1]
https://svn.apache.org/repos/asf/james/mime4j/branches/dom-api-refactoring

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Norman Maurer <no...@apache.org>]

2011/1/26 Stefano Bagnara <ap...@bago.org>:
> 2011/1/26 Oleg Kalnichevski <ol...@apache.org>:
>>
>>> The fact is that probably ServiceLoader was required in the previous
>>> design, using Abstract classes. That design is the only I'm aware of
>>> that would have allowed future releases to add more methods to each
>>> class without breaking compatibility with older sources. Now that you
>>> removed this possibility by switching everything to interfaces I
>>> understand that the ServiceLoader seems "horrid". Most stuff I did in
>>> the dom package has been done with this precise goal, so I can't
>>> "defend" that code if we remove the goal :-)  That said you also
>>> should understand that the ServiceLoader, like most SPI model out
>>> there, is simply an abstraction layer. You are free to directly
>>> instantiate the implementation.
>>>
>>>
>>> I didn't (and I currently don't) know a better way to create a forward
>>> compatible, yet extensible, API, but I'm far from being an expert API
>>> designer.
>>>
>>
>> Stefano
>>
>> I am sorry I do not see how the service locator pattern can help with
>> maintaining forward compatibility between releases. I always thought
>> that it's goal was primarily to decouple layers and to enable multiple
>> implementations of a service interface.
>>
>> http://en.wikipedia.org/wiki/Service_locator_pattern
>
> I wrote "the ServiceLoader, ..., is simply an abstraction layer." so
> I'm in line with wikipedia, I'm sorry if I mislead you with some other
> sentence.
>
> I try to explain with other words:
> - abstract model was there to allow forward compatibility (it is the
> only way I know that allows adding a method in the model without
> breaking everything)
> - service locator adds "indirection" (decoupling)
> You need both if you want to "publish" a single package and you don't
> want this package to have direct dependencies on the implementation.
> The 2 patterns are extensively used in the standard java class library
> and in many  common libraries (I can't take the merit for mixing them
> ;-) ).
>
> I'm not sure I understand what part of the ServiceLocator is "horrid"
> in your opinion (I don't want to force you explaining anything or to
> waste your time, but I'm always interested learning from other
> developers).
>
>>> > How do you want me to proceed now?
>>>
>>> I'm happy to see you pushing. *Go* *ahead* :-)  I just explained why I
>>> did things in a given way and how I evaluate changes. This was not a
>>> comment to stop you, but just something to let you be more informed.
>>> As long as 0.7 will support the Monitor object, the fixed headless
>>> parsing and the fixed field folding I will be happy (I currently have
>>> 0.7-SNAPSHOT as dependency for this and not for the DOM abstraction).
>>>
>>> Stefano
>>
>> I do not want to 'push' if there is no consensus that I am pushing in
>> the right direction.
>
> You should ;-)
> - You have the time to prepare a next release
> - You are a project committer
> - No one is vetoing your changes
>
> The fact that I prefer the "previous" design and I don't see critical
> "inconsistencies" in that design is just a comment (an IMHO). I accept
> to loose that stuff as in turn you'll give me back a release soon. So
> I think for both me and the community having a release with the many
> bugfix/features introduced in trunk is a big win. (hope this makes
> sense)
>
> Stefano
>

+1 Let's keep pushing for a new mime4j release :)

Bye,
Norman

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Stefano Bagnara <ap...@bago.org>]

2011/1/26 Oleg Kalnichevski <ol...@apache.org>:
> [...]
> Fine. The problem is that Service Locator is considered an anti pattern
> by many, an opinion I fully subscribe to.

I don't have a strong opinion on this. IMHO it depends on the context
(also, an antipattern is usually compared to a better alternative to
reach the same goal).

If you want to remove it from svn feel free to do that. I just care
about the operational fixes/improvements I did there and don't care
about the locator or the dom api (sooner or later it would be good to
have this api, but it is not an high priority IMO).

Consider it a test. I did it in trunk simply because no one else was
working there and we usually prefer coding to happen in trunk (in fact
the locator+factories is something that took few hours, nothing more
than an experiment and we probably wasting too much time "discussing"
it when it can either be improved or replaced by alternatives)

I'm not against removing the locator, removing the "abstract classes
style" and so on (and you should feel free doing this, really).

The main "design" thing I care is about package dependencies: I don't
want dependency cycles between packages (I know some developers don't
care about this, but in my experience it is very important and I try
to defend this as I can: I'm aware people think I'm extremist about
this).

> [...]
> What is wrong with plain old java classes? What is it _exactly_ that was
> wrong with Message classes in 0.6?

goals in my mind when I did the change:
1) having a single package exposing dom functionality/classes so
client classes simply have "import mime4j.dom.*;" and not some import
from parser some from stream some from message and some from field.
2) make sure this package had no direct dependencies on the parsing
package (DOM should not depend on a parsing implementation: DOM is
about structure, not parsing)
3) allow future release to be able to add new methods/behaviours
without breaking compatibilty.

I remember 0.6 had many other issue I had to refactor in order to come
up with clean dependency tree and fix some bug regarding consistency
(headless parsing, header folding, empty lines), but if you want to
backport fixes and keep the "Message" style from 0.6 I can live with
it too. I'm sorry but my memory don't let me remember all of the issue
I saw in 0.6 (I don't even remember what I ate yesterday). Another
motivation was using an API java developers were used to (Most java
developers used/understand xml api so it was a good match).

> I am not sure. Now we have a situation when neither you nor I like the
> existing DOM API that much.

If I understand the way you liked the 0.6 version then it should be
simple to make you like trunk again.
1) Just leave interfaces in dom.
2) Remove the factories and service locator from dom (or ignore they
exists, they are just something "additional" you are not forced to use
them).
3) Tell people to instantiate Message implementation from message
package (MimeMessage / MessageImpl).

How is so much different from 0.6 with dom interfaces extracted to
another package?
Are you instead saying you don't want the "dom" package at all?

While I try to remember something else from the upgrade of my "client
code" from 0.6 to 0.7-SNAPSHOT I personally prefer the way some
configuration is exposed via the new factories instead of the 0.6
method of passing a MimeEntityConfig whenever you wanted to alter the
default behaviour (I think config objects like this are unexpected
from a java library user).

jDKIM with mime4j 0.6:
http://svn.apache.org/viewvc/james/jdkim/trunk/main/src/main/java/org/apache/james/jdkim/impl/Message.java?revision=824254&view=markup
You can see I had to extend MimeTokenStream  just to be able to
configure a new max line length.
Also I had to use the low level token stream for a simple task like
parsing message headers.

jDKIM with mime4j 0.7-SNAP (didn't update to the current trunk, it is
still against trunk before you worked on it):
http://svn.apache.org/viewvc/james/jdkim/trunk/main/src/main/java/org/apache/james/jdkim/impl/Message.java?revision=998320&view=markup
It still uses MimeEntityConfig but you can see the comment where I was
planning to have similar configurations exposed as factory properties:
--
MessageBuilderFactory mbf = MessageBuilderFactory.newInstance();
68 	mbf.setAttribute("MimeEntityConfig", mec);
69 	// mbf.setProperty("MaxLineLength", 10000);
---
Once MaxLineLength was a factory configuration property the jDKIM
would be using only dom classes (and not message/parser/stream lower
level packages).

Also the code moved from 184 to 130 LOC.

Unfortunately my experience with mime4j <=0.6 is limited to jdkim
because the only other project where I used mime4j used 0.7-SNAP since
the first version because I use the validation and the Monitor to
report errors found while parsing (not available in 0.6).

> Anyway, I'll stick to my original plan, bring mime4j to a state in which
> it could be released as 0.7 and step back again.

OK (and feel free to drop my trunk tests if you have ideas, really)

Stefano

PS: This long messages are not to "defend" my opinion, but just to
explain trunk code and historical steps.

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Oleg Kalnichevski <ol...@apache.org>]

> > I am sorry I do not see how the service locator pattern can help with
> > maintaining forward compatibility between releases. I always thought
> > that it's goal was primarily to decouple layers and to enable multiple
> > implementations of a service interface.
> >
> > http://en.wikipedia.org/wiki/Service_locator_pattern
> 
> I wrote "the ServiceLoader, ..., is simply an abstraction layer." so
> I'm in line with wikipedia, I'm sorry if I mislead you with some other
> sentence.
> 

Fine. The problem is that Service Locator is considered an anti pattern
by many, an opinion I fully subscribe to.


> I try to explain with other words:
> - abstract model was there to allow forward compatibility (it is the
> only way I know that allows adding a method in the model without
> breaking everything)


What is wrong with plain old java classes? What is it _exactly_ that was
wrong with Message classes in 0.6?


> I'm not sure I understand what part of the ServiceLocator is "horrid"
> in your opinion (I don't want to force you explaining anything or to
> waste your time, but I'm always interested learning from other
> developers).
> 

See above.


> The fact that I prefer the "previous" design and I don't see critical
> "inconsistencies" in that design is just a comment (an IMHO). I accept
> to loose that stuff as in turn you'll give me back a release soon. So
> I think for both me and the community having a release with the many
> bugfix/features introduced in trunk is a big win. (hope this makes
> sense)
> 

I am not sure. Now we have a situation when neither you nor I like the
existing DOM API that much.

Anyway, I'll stick to my original plan, bring mime4j to a state in which
it could be released as 0.7 and step back again.

Oleg

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Stefano Bagnara <ap...@bago.org>]

2011/1/26 Oleg Kalnichevski <ol...@apache.org>:
>
>> The fact is that probably ServiceLoader was required in the previous
>> design, using Abstract classes. That design is the only I'm aware of
>> that would have allowed future releases to add more methods to each
>> class without breaking compatibility with older sources. Now that you
>> removed this possibility by switching everything to interfaces I
>> understand that the ServiceLoader seems "horrid". Most stuff I did in
>> the dom package has been done with this precise goal, so I can't
>> "defend" that code if we remove the goal :-)  That said you also
>> should understand that the ServiceLoader, like most SPI model out
>> there, is simply an abstraction layer. You are free to directly
>> instantiate the implementation.
>>
>>
>> I didn't (and I currently don't) know a better way to create a forward
>> compatible, yet extensible, API, but I'm far from being an expert API
>> designer.
>>
>
> Stefano
>
> I am sorry I do not see how the service locator pattern can help with
> maintaining forward compatibility between releases. I always thought
> that it's goal was primarily to decouple layers and to enable multiple
> implementations of a service interface.
>
> http://en.wikipedia.org/wiki/Service_locator_pattern

I wrote "the ServiceLoader, ..., is simply an abstraction layer." so
I'm in line with wikipedia, I'm sorry if I mislead you with some other
sentence.

I try to explain with other words:
- abstract model was there to allow forward compatibility (it is the
only way I know that allows adding a method in the model without
breaking everything)
- service locator adds "indirection" (decoupling)
You need both if you want to "publish" a single package and you don't
want this package to have direct dependencies on the implementation.
The 2 patterns are extensively used in the standard java class library
and in many  common libraries (I can't take the merit for mixing them
;-) ).

I'm not sure I understand what part of the ServiceLocator is "horrid"
in your opinion (I don't want to force you explaining anything or to
waste your time, but I'm always interested learning from other
developers).

>> > How do you want me to proceed now?
>>
>> I'm happy to see you pushing. *Go* *ahead* :-)  I just explained why I
>> did things in a given way and how I evaluate changes. This was not a
>> comment to stop you, but just something to let you be more informed.
>> As long as 0.7 will support the Monitor object, the fixed headless
>> parsing and the fixed field folding I will be happy (I currently have
>> 0.7-SNAPSHOT as dependency for this and not for the DOM abstraction).
>>
>> Stefano
>
> I do not want to 'push' if there is no consensus that I am pushing in
> the right direction.

You should ;-)
- You have the time to prepare a next release
- You are a project committer
- No one is vetoing your changes

The fact that I prefer the "previous" design and I don't see critical
"inconsistencies" in that design is just a comment (an IMHO). I accept
to loose that stuff as in turn you'll give me back a release soon. So
I think for both me and the community having a release with the many
bugfix/features introduced in trunk is a big win. (hope this makes
sense)

Stefano

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Oleg Kalnichevski <ol...@apache.org>]

> The fact is that probably ServiceLoader was required in the previous
> design, using Abstract classes. That design is the only I'm aware of
> that would have allowed future releases to add more methods to each
> class without breaking compatibility with older sources. Now that you
> removed this possibility by switching everything to interfaces I
> understand that the ServiceLoader seems "horrid". Most stuff I did in
> the dom package has been done with this precise goal, so I can't
> "defend" that code if we remove the goal :-)  That said you also
> should understand that the ServiceLoader, like most SPI model out
> there, is simply an abstraction layer. You are free to directly
> instantiate the implementation.
>
>
> I didn't (and I currently don't) know a better way to create a forward
> compatible, yet extensible, API, but I'm far from being an expert API
> designer.
> 

Stefano

I am sorry I do not see how the service locator pattern can help with
maintaining forward compatibility between releases. I always thought
that it's goal was primarily to decouple layers and to enable multiple
implementations of a service interface.   

http://en.wikipedia.org/wiki/Service_locator_pattern


> >
> > How do you want me to proceed now?
> 
> I'm happy to see you pushing. *Go* *ahead* :-)  I just explained why I
> did things in a given way and how I evaluate changes. This was not a
> comment to stop you, but just something to let you be more informed.
> As long as 0.7 will support the Monitor object, the fixed headless
> parsing and the fixed field folding I will be happy (I currently have
> 0.7-SNAPSHOT as dependency for this and not for the DOM abstraction).
> 
> Stefano
> 

I do not want to 'push' if there is no consensus that I am pushing in
the right direction.

Oleg 

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Markus Wiederkehr <ma...@gmail.com>]

On Tue, Jan 25, 2011 at 10:09 PM, Stefano Bagnara <ap...@bago.org> wrote:
> 2011/1/25 Oleg Kalnichevski <ol...@apache.org>:
>> On Tue, 2011-01-25 at 17:49 +0100, Stefano Bagnara wrote:
>>> 2011/1/25 Oleg Kalnichevski <ol...@apache.org>:
>>> > * MessageBuilder redesigned and changed to an interface
>>> > * Moved #writeTo method to MessageWriter interface
>>>
>>> Moving "writeTo" from each "DOM node" to a MessageWriter interface
>>> have the drawback that you cannot anymore serialize just part of a
>>> message (just a subtree, an header, a content) because your
>>> MessageWriter serializes only full messages. On the other side I see
>>> the advantage of having the writer separated from the element itself
>>> (reading and writing are different tasks).
>>>
>>
>> I am sorry but that was not done by me.
>>
>> http://svn.apache.org/viewvc?view=revision&revision=739822
>
> So it seems the "feature" has been removed long ago! Thanks for the link.

It is still possible to serialize parts of a Message because
MessageWriter (MimeWriter in Oleg's branch) publicly exposes the
methods to do so (writeMultipart(), etc).

See http://svn.apache.org/viewvc/james/mime4j/trunk/dom/src/main/java/org/apache/james/mime4j/message/MessageWriter.java?view=markup

IIRC the reason for that change was that there were different
strategies for writing messages (strict, lenient) and before the
change the code for writing and these strategies in particular was
scattered around the various message classes (Header, Entity,
Multipart, ..). Now it's all in one place. See also MIME4J-110..

Later, in the course of MIME4J-118, the different writing modes were
removed and now there is only one MessageWriter instance called
DEFAULT.

Cheers,
Markus

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Stefano Bagnara <ap...@bago.org>]

2011/1/25 Oleg Kalnichevski <ol...@apache.org>:
> On Tue, 2011-01-25 at 17:49 +0100, Stefano Bagnara wrote:
>> 2011/1/25 Oleg Kalnichevski <ol...@apache.org>:
>> > ...
>> >
>> >>
>> >> >> [...] BTW I'm just loud thinking, just go ahead with
>> >> >> your plan :-)
>> >> >
>> >> > I am going to get there shortly. I just prefer to make potentially
>> >> > contentious changes in small, incremental steps, so that they are easier
>> >> > to review and agree / disagree upon.
>> >>
>> >> I understand. I'm impatient to see your changes :-)
>> >>
>> >> Happy hacking
>> >> Stefano
>> >>
>> >
>> > Stefano et al
>> >
>> > I just completed refactoring of MessageBuilder and related classes on
>> > the refactoring branch. Here is a short summary of changes:
>> >
>> > * MessageBuilder redesigned and changed to an interface
>> > * Moved #writeTo method to MessageWriter interface
>>
>> Moving "writeTo" from each "DOM node" to a MessageWriter interface
>> have the drawback that you cannot anymore serialize just part of a
>> message (just a subtree, an header, a content) because your
>> MessageWriter serializes only full messages. On the other side I see
>> the advantage of having the writer separated from the element itself
>> (reading and writing are different tasks).
>>
>
> I am sorry but that was not done by me.
>
> http://svn.apache.org/viewvc?view=revision&revision=739822

So it seems the "feature" has been removed long ago! Thanks for the link.

>> I haven't carefully looked at the code to see if you preserved some of
>> the optimizations about unparsed content streamed as-is in writeTo
>> instead of being recreated (this was good for performances and also to
>> keep input formatting when writing out a parsed message with no
>> changes).
>>
>> > * MessageBuilderFactory renamed to MessageServiceFactory and can now be
>> > used to instantiate message writers and builders. Additional factory
>> > methods can be introduced in the future.
>> >
>> > Please review and let me know if you can live with these changes.
>>
>> I hoped you wanted to deal also with DOM altering methods as I thought
>> you was not happy with dom mainly because it was an incomplete api (so
>> I probably didn't understand you well). I don't see "improvements"
>> over previous trunk
>
> My main problem was the lack of consistency in your code more than
> anything else. I never claimed I was able to create a "better" /
> "enhanced" / "more complete" API. I was merely trying to resolve
> inconsistencies while preserving the initial design and the general
> approach that you had started. If I had my way I would simply throw away
> every single class in the 'dom' packages, especially that horrid
> ServiceLoader, and reverted back to the simpler Message API that had
> existed before.

The fact is that probably ServiceLoader was required in the previous
design, using Abstract classes. That design is the only I'm aware of
that would have allowed future releases to add more methods to each
class without breaking compatibility with older sources. Now that you
removed this possibility by switching everything to interfaces I
understand that the ServiceLoader seems "horrid". Most stuff I did in
the dom package has been done with this precise goal, so I can't
"defend" that code if we remove the goal :-)  That said you also
should understand that the ServiceLoader, like most SPI model out
there, is simply an abstraction layer. You are free to directly
instantiate the implementation.

I didn't (and I currently don't) know a better way to create a forward
compatible, yet extensible, API, but I'm far from being an expert API
designer.

>> (But maybe this is mainly a matter of style /
>> different approaches to API design), but it is anyway much better than
>> 0.6 so I can live with it (power to active people).
>>
>> To avoid misunderstandings: I'm not "sad" accepting this changes, on
>> the contrary I'm happy to see someone pushing mime4j again and I just
>> try to find the time to review and explain why things were done in
>> that way.
>>
>
> How do you want me to proceed now?

I'm happy to see you pushing. *Go* *ahead* :-)  I just explained why I
did things in a given way and how I evaluate changes. This was not a
comment to stop you, but just something to let you be more informed.
As long as 0.7 will support the Monitor object, the fixed headless
parsing and the fixed field folding I will be happy (I currently have
0.7-SNAPSHOT as dependency for this and not for the DOM abstraction).

Stefano

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Oleg Kalnichevski <ol...@apache.org>]

On Tue, 2011-01-25 at 17:49 +0100, Stefano Bagnara wrote:
> 2011/1/25 Oleg Kalnichevski <ol...@apache.org>:
> > ...
> >
> >>
> >> >> [...] BTW I'm just loud thinking, just go ahead with
> >> >> your plan :-)
> >> >
> >> > I am going to get there shortly. I just prefer to make potentially
> >> > contentious changes in small, incremental steps, so that they are easier
> >> > to review and agree / disagree upon.
> >>
> >> I understand. I'm impatient to see your changes :-)
> >>
> >> Happy hacking
> >> Stefano
> >>
> >
> > Stefano et al
> >
> > I just completed refactoring of MessageBuilder and related classes on
> > the refactoring branch. Here is a short summary of changes:
> >
> > * MessageBuilder redesigned and changed to an interface
> > * Moved #writeTo method to MessageWriter interface
> 
> Moving "writeTo" from each "DOM node" to a MessageWriter interface
> have the drawback that you cannot anymore serialize just part of a
> message (just a subtree, an header, a content) because your
> MessageWriter serializes only full messages. On the other side I see
> the advantage of having the writer separated from the element itself
> (reading and writing are different tasks).
> 

I am sorry but that was not done by me.

http://svn.apache.org/viewvc?view=revision&revision=739822


> I haven't carefully looked at the code to see if you preserved some of
> the optimizations about unparsed content streamed as-is in writeTo
> instead of being recreated (this was good for performances and also to
> keep input formatting when writing out a parsed message with no
> changes).
> 
> > * MessageBuilderFactory renamed to MessageServiceFactory and can now be
> > used to instantiate message writers and builders. Additional factory
> > methods can be introduced in the future.
> >
> > Please review and let me know if you can live with these changes.
> 
> I hoped you wanted to deal also with DOM altering methods as I thought
> you was not happy with dom mainly because it was an incomplete api (so
> I probably didn't understand you well). I don't see "improvements"
> over previous trunk 

My main problem was the lack of consistency in your code more than
anything else. I never claimed I was able to create a "better" /
"enhanced" / "more complete" API. I was merely trying to resolve
inconsistencies while preserving the initial design and the general
approach that you had started. If I had my way I would simply throw away
every single class in the 'dom' packages, especially that horrid
ServiceLoader, and reverted back to the simpler Message API that had
existed before.


> (But maybe this is mainly a matter of style /
> different approaches to API design), but it is anyway much better than
> 0.6 so I can live with it (power to active people).
> 
> To avoid misunderstandings: I'm not "sad" accepting this changes, on
> the contrary I'm happy to see someone pushing mime4j again and I just
> try to find the time to review and explain why things were done in
> that way.
> 

How do you want me to proceed now?

Cheers,

Oleg

Re: MessageBuilder refactoring; was Re: dom / message refactoring

[Stefano Bagnara <ap...@bago.org>]

2011/1/25 Oleg Kalnichevski <ol...@apache.org>:
> ...
>
>>
>> >> [...] BTW I'm just loud thinking, just go ahead with
>> >> your plan :-)
>> >
>> > I am going to get there shortly. I just prefer to make potentially
>> > contentious changes in small, incremental steps, so that they are easier
>> > to review and agree / disagree upon.
>>
>> I understand. I'm impatient to see your changes :-)
>>
>> Happy hacking
>> Stefano
>>
>
> Stefano et al
>
> I just completed refactoring of MessageBuilder and related classes on
> the refactoring branch. Here is a short summary of changes:
>
> * MessageBuilder redesigned and changed to an interface
> * Moved #writeTo method to MessageWriter interface

Moving "writeTo" from each "DOM node" to a MessageWriter interface
have the drawback that you cannot anymore serialize just part of a
message (just a subtree, an header, a content) because your
MessageWriter serializes only full messages. On the other side I see
the advantage of having the writer separated from the element itself
(reading and writing are different tasks).

I haven't carefully looked at the code to see if you preserved some of
the optimizations about unparsed content streamed as-is in writeTo
instead of being recreated (this was good for performances and also to
keep input formatting when writing out a parsed message with no
changes).

> * MessageBuilderFactory renamed to MessageServiceFactory and can now be
> used to instantiate message writers and builders. Additional factory
> methods can be introduced in the future.
>
> Please review and let me know if you can live with these changes.

I hoped you wanted to deal also with DOM altering methods as I thought
you was not happy with dom mainly because it was an incomplete api (so
I probably didn't understand you well). I don't see "improvements"
over previous trunk (But maybe this is mainly a matter of style /
different approaches to API design), but it is anyway much better than
0.6 so I can live with it (power to active people).

To avoid misunderstandings: I'm not "sad" accepting this changes, on
the contrary I'm happy to see someone pushing mime4j again and I just
try to find the time to review and explain why things were done in
that way.

Thank you,
Stefano

MessageBuilder refactoring; was Re: dom / message refactoring

[Oleg Kalnichevski <ol...@apache.org>]

...

> 
> >> [...] BTW I'm just loud thinking, just go ahead with
> >> your plan :-)
> >
> > I am going to get there shortly. I just prefer to make potentially
> > contentious changes in small, incremental steps, so that they are easier
> > to review and agree / disagree upon.
> 
> I understand. I'm impatient to see your changes :-)
> 
> Happy hacking
> Stefano
> 

Stefano et al

I just completed refactoring of MessageBuilder and related classes on
the refactoring branch. Here is a short summary of changes: 

* MessageBuilder redesigned and changed to an interface
* Moved #writeTo method to MessageWriter interface
* MessageBuilderFactory renamed to MessageServiceFactory and can now be
used to instantiate message writers and builders. Additional factory
methods can be introduced in the future.

Please review and let me know if you can live with these changes.

I am _almost_ done. As the very last step I would like to try to
decouple 'message' and 'storage' classes, but I could also live with
things in their present form.

Oleg

Re: dom / message refactoring; was Re: 0.6.2 or 0.7 any time soon?

[Stefano Bagnara <ap...@bago.org>]

2011/1/18 Oleg Kalnichevski <ol...@apache.org>:
> I understand the downside of having a pure interface based API. However,
> you seem to be very keen to position mime4j not only as a utility for
> parsing and building mime messages but also as an abstract Document
> Object Model with different / alternative implementations. I just do not
> see how one can honestly call the existing code in trunk an abstract
> Object Model if the only aspect that is abstract is field parsing. We
> should either drop the pretense at having an abstract DOM in mime4j and
> revert to the state existed before your changes or actually make an
> effort to define a truly abstract model even if that means more effort.
>
> I am very fine with just reverting to the old state, by the way.
>
> So, what shall I do?

Go ahead with your plan.

I agree that DOM is incomplete now.

I don't think that we'll see alternative implementations, but having a
good DOM api is also a way to stabilize an interface and put a clear
limit in what can be changed and what cannot be changed.

I would like before 1.0 to tell people feel free to refer .dom.
classes, don't touch .field./.message./.storage./ .

Having no direct dependencies from dom to the implementation is a way
to be sure this happen.

>> [...] BTW I'm just loud thinking, just go ahead with
>> your plan :-)
>
> I am going to get there shortly. I just prefer to make potentially
> contentious changes in small, incremental steps, so that they are easier
> to review and agree / disagree upon.

I understand. I'm impatient to see your changes :-)

Happy hacking
Stefano

Re: dom / message refactoring; was Re: 0.6.2 or 0.7 any time soon?

[Oleg Kalnichevski <ol...@apache.org>]

On Tue, 2011-01-18 at 17:46 +0100, Stefano Bagnara wrote:
> Il 17 gennaio 2011 17.43.37 UTC+1, Oleg Kalnichevski
> <ol...@apache.org> ha scritto:
> > Stefano et al
> >
> > Please review another round of API changes on the refactoring branch.
> > Here is the gist of most important changes
> >
> > * Most of the object model implementation logic moved from 'dom' to
> > 'message'. 'dom' consists mostly of pure interfaces now that do not
> > impose particular internal structures, a particular implementation or a
> > particular inheritance hierarchy.
> 
> I think we loose an opportunity to keep backward compatibility in
> future when we want to add methods to the 4 classes you transformed in
> interfaces and I don't see advantages in turn, but, on the other side
> I think we should feel free to break backward compatibility even in
> future versions so this is not a big drawback and I can live with the
> change (just hoping this won't be a blocker in future).
> 

Stefano

I understand the downside of having a pure interface based API. However,
you seem to be very keen to position mime4j not only as a utility for
parsing and building mime messages but also as an abstract Document
Object Model with different / alternative implementations. I just do not
see how one can honestly call the existing code in trunk an abstract
Object Model if the only aspect that is abstract is field parsing. We
should either drop the pretense at having an abstract DOM in mime4j and
revert to the state existed before your changes or actually make an
effort to define a truly abstract model even if that means more effort.

I am very fine with just reverting to the old state, by the way.

So, what shall I do?

> > * All dom element copy / parse logic moved from individual classes to a
> > utility class currently called MimeBuilder (I would prefer this class to
> > be called MessageBuilder but this name is already taken)
> 
> Maybe we should expose some of the MimeBuilder methods via the
> MessageBuilder "public" factory: the bigger limit of the current DOM
> api is that it doesn't expose methods to create a structured Message
> from scratch (it let you set a new Body but it doesn't give a way to
> create a Body). So the MimeBuilder methods could already be moved to
> MessageBuilderImpl. BTW I'm just loud thinking, just go ahead with
> your plan :-)
> 

I am going to get there shortly. I just prefer to make potentially
contentious changes in small, incremental steps, so that they are easier
to review and agree / disagree upon.

Oleg

Re: dom / message refactoring; was Re: 0.6.2 or 0.7 any time soon?

[Stefano Bagnara <io...@bago.org>]

Il 17 gennaio 2011 17.43.37 UTC+1, Oleg Kalnichevski
<ol...@apache.org> ha scritto:
> Stefano et al
>
> Please review another round of API changes on the refactoring branch.
> Here is the gist of most important changes
>
> * Most of the object model implementation logic moved from 'dom' to
> 'message'. 'dom' consists mostly of pure interfaces now that do not
> impose particular internal structures, a particular implementation or a
> particular inheritance hierarchy.

I think we loose an opportunity to keep backward compatibility in
future when we want to add methods to the 4 classes you transformed in
interfaces and I don't see advantages in turn, but, on the other side
I think we should feel free to break backward compatibility even in
future versions so this is not a big drawback and I can live with the
change (just hoping this won't be a blocker in future).

> * All dom element copy / parse logic moved from individual classes to a
> utility class currently called MimeBuilder (I would prefer this class to
> be called MessageBuilder but this name is already taken)

Maybe we should expose some of the MimeBuilder methods via the
MessageBuilder "public" factory: the bigger limit of the current DOM
api is that it doesn't expose methods to create a structured Message
from scratch (it let you set a new Body but it doesn't give a way to
create a Body). So the MimeBuilder methods could already be moved to
MessageBuilderImpl. BTW I'm just loud thinking, just go ahead with
your plan :-)

> Please let me know if you can live with these changes and whether it is
> ok to merge them down to trunk.

ok.

Stefano

dom / message refactoring; was Re: 0.6.2 or 0.7 any time soon?

[Oleg Kalnichevski <ol...@apache.org>]

On Tue, 2011-01-11 at 21:13 +0100, Oleg Kalnichevski wrote:

...

> 
> Stefano, 
> 
> I already explained why I no longer feel comfortable committing to
> trunk. There is no point repeating that. 
> 
> > We will ask questions when we need answers :-)
> > 
> > Stefano
> > 
> > PS: if something I said/did makes you angry just explain me please. I
> > hope this is just something related to "translation" and the fact we
> > don't speak the same language natively.
> > 
> 
> I am not angry. I just find it regrettable that the project was
> effectively paralyzed for a full year.
> 
> I'll start working on a branch. 
> 
> Cheers
> 
> Oleg 
> 

Stefano et al

Please review another round of API changes on the refactoring branch.
Here is the gist of most important changes

* Most of the object model implementation logic moved from 'dom' to
'message'. 'dom' consists mostly of pure interfaces now that do not
impose particular internal structures, a particular implementation or a
particular inheritance hierarchy. 

* All dom element copy / parse logic moved from individual classes to a
utility class currently called MimeBuilder (I would prefer this class to
be called MessageBuilder but this name is already taken)

Please let me know if you can live with these changes and whether it is
ok to merge them down to trunk.

Oleg

Re: 0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Oleg Kalnichevski <ol...@apache.org>]

> Why can't you simply work on trunk or on a branch here in our svn? I
> think I encouraged you multiple times to simply do this, I don't
> really understand why you aswer me like I'm trying to stop you.
> My preference is for you to use trunk. We use CTR, so go ahead and I
> hope you will be happy if I review what you do.
> 
> You are a committer, so we already trust you, so you should no fear
> working in trunk. I do this when I have ideas and I expect others to
> simply do the same. If what I committed in trunk is blocking evolution
> then just revert it, otherwise make your changes: whatever you feel
> appropriate.
> 

Stefano, 

I already explained why I no longer feel comfortable committing to
trunk. There is no point repeating that. 

> We will ask questions when we need answers :-)
> 
> Stefano
> 
> PS: if something I said/did makes you angry just explain me please. I
> hope this is just something related to "translation" and the fact we
> don't speak the same language natively.
> 

I am not angry. I just find it regrettable that the project was
effectively paralyzed for a full year.

I'll start working on a branch. 

Cheers

Oleg 

Re: 0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Norman Maurer <no...@apache.org>]

Com'on guys calm down..

@Oleg:
Please just start your work on trunk or a branch inside apache. This
way we will still be able to see your commits as notifications etc. I
don't see why you want to take the code to github. Just work here..

Bye,
Norman

2011/1/11 Stefano Bagnara <ap...@bago.org>:
> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
>> On Tue, 2011-01-11 at 15:18 +0100, Stefano Bagnara wrote:
>>> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
>>> > On Tue, 2011-01-11 at 14:01 +0100, Stefano Bagnara wrote:
>>> >> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
>>> >> > On Tue, 2011-01-11 at 09:27 +0100, Norman Maurer wrote:
>>> >> >> Hi there,
>>> >> >>
>>> >> >> I think if it worth it we should release 0.6.2. Release often release
>>> >> >> early, you know ;)
>>> >> >>
>>> >> >> Bye,
>>> >> >> Norman
>>> >> >>
>>> >> >
>>> >> > Folks
>>> >> >
>>> >> > I also would like to port another fix, should we decide to do another
>>> >> > release off the 0.6.x branch.
>>> >>
>>> >> What is the other fix? This one is not critical (I see it just like a
>>> >> documentation bug: either way we need that check on that field to work
>>> >> that way, we can't simply check the line length).
>>> >>
>>> >
>>> > Parsing of folded fields. The default field parser in 0.6 chokes on
>>> > perfectly valid fields if their body is folded.
>>> >
>>> >
>>> >> > I am also willing to make a push toward the 0.7 release, if no one is
>>> >> > going to pick up work on the API changes stared by Stefano on the trunk.
>>> >>
>>> >> I had really few time but I think I also slowed down because I never
>>> >> understood if what I was doing was liked or not. It takes a lot to
>>> >> complete stuff, so I would have liked to understand what others thinks
>>> >> we should do in 0.7.
>>> >>
>>> >> As an example I see sometimes we talk as 0.x versions we can do
>>> >> backward incompatible changes trying to reach a good api, other times
>>> >> it seems we instead are "stuck" to the 0.6 version because 0.6 has
>>> >> already a lot of users so compatibility is brought to the table.
>>> >>
>>> >> That said, I say my opinion and I expect others to say their opinions
>>> >> so that we can see where we can find a consensus.
>>> >> - I think 0.6 is not "great" as API, so I would happily break
>>> >> compatibility in order to provide a better api. Main thing is that the
>>> >> 0.6 API does not accept evolution (every non trivial feature will
>>> >> require a backward incompatible change).
>>> >> - IMO current trunk could be released as 0.7.0 with very minor change:
>>> >> it is far from exposing a complete api, but I find it already better
>>> >> than 0.6 and I have already some product depending on current trunk.
>>> >> We saw we proceed at a slow speed, so we should be prepared improving
>>> >> the API while we reach 1.0.
>>> >> - I guess most of changes we have in trunk are not backportable to 0.6
>>> >> because they have been possible by the major refactorings, but I'm not
>>> >> against this, if anyone sees a way.
>>> >>
>>> >> Can you state yours and also tell something more about "your" 0.7 plan?
>>> >>
>>> >
>>> > I think we discussed this on more than one occasion in the past. While I
>>> > think mime4j 'core' in 0.7 is fine, the 'dom' / 'message' stuff is not,
>>>
>>> Yes, we discussed a couple of times, but we didn't find a solution (at
>>> least not one I understood)
>>>
>>> > and the whole library is not in a releasable state at the moment.
>>>
>>> Got it: hope you will review trunk soon to understand what changes you
>>> propose to make it releasable.
>>>
>>> > And there is "my" plan:
>>> >
>>> > (1) ask people to go over issues in JIRA and decide what is in scope for
>>> > 0.7 and what can wait until a better day (0.8)
>>>
>>> +1
>>>
>>> The main causes I use trunk in production instead of 0.6 are:
>>> MIME4J-158 - Reduce usage of commons-logging in favor of a "Monitor" service.
>>> https://issues.apache.org/jira/browse/MIME4J-158
>>> MIME4J-58 - Lenient dealing with headless messages or malformed
>>> header/body separation
>>> https://issues.apache.org/jira/browse/MIME4J-58
>>> MIME4J-153 - Headless inconsistency between MimeTokenStream and MimeStreamParser
>>> https://issues.apache.org/jira/browse/MIME4J-153
>>>
>>> Also the folded header stuff you mentioned (MIME4J-141 - MIME4J-146)
>>>
>>> > (2) revisit the 'dom' and 'message' packages and try to figure out
>>> > whether 'model' and 'implementation' classes in their present form make
>>> > sense.
>>>
>>> +1
>>>
>>> > In my option, many of them do not.
>>>
>>> They are the result of my limited use case: they work fine in my use
>>> cases (jDKIM + proprietary product).
>>> We need more real use cases to "shape" them, but I trust you (and you
>>> probably have good uses cases too), so I will wait for your review.
>>>
>>> Stefano
>>>
>>
>> Stefano, for the love of God Almighty, what else am I supposed to do?
>
> I don't get what I said to upset you. I'm sorry if you thought I'm on
> the way. I keep repeating you just commit your changes, just do
> whatever you want on svn and I will be happy. You don't commit
> anything so I try discussing. I don't know what you prefer me to do.
> I'm just trying to understand how to evolve mime4j.
>
>> I
>> pointed out a number of times those things that do not seem to make any
>> sense what so ever, like HeaderImpl extending Header, which is a
>> CONCRETE class, or abstract Multipart where the only abstract aspects
>> are preamble / epilogue related methods.
>>
>> OK. I will create a copy of mime4j on github and make _minimal_ changes
>> to your code just to resolve the most glaring WTFs in the API and
>> present it for review. Simply listing things that I disagree with does
>> not seem to bring us anywhere anymore.
>
> Why can't you simply work on trunk or on a branch here in our svn? I
> think I encouraged you multiple times to simply do this, I don't
> really understand why you aswer me like I'm trying to stop you.
> My preference is for you to use trunk. We use CTR, so go ahead and I
> hope you will be happy if I review what you do.
>
> You are a committer, so we already trust you, so you should no fear
> working in trunk. I do this when I have ideas and I expect others to
> simply do the same. If what I committed in trunk is blocking evolution
> then just revert it, otherwise make your changes: whatever you feel
> appropriate.
>
> We will ask questions when we need answers :-)
>
> Stefano
>
> PS: if something I said/did makes you angry just explain me please. I
> hope this is just something related to "translation" and the fact we
> don't speak the same language natively.
>

Re: 0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Stefano Bagnara <ap...@bago.org>]

2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
> On Tue, 2011-01-11 at 15:18 +0100, Stefano Bagnara wrote:
>> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
>> > On Tue, 2011-01-11 at 14:01 +0100, Stefano Bagnara wrote:
>> >> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
>> >> > On Tue, 2011-01-11 at 09:27 +0100, Norman Maurer wrote:
>> >> >> Hi there,
>> >> >>
>> >> >> I think if it worth it we should release 0.6.2. Release often release
>> >> >> early, you know ;)
>> >> >>
>> >> >> Bye,
>> >> >> Norman
>> >> >>
>> >> >
>> >> > Folks
>> >> >
>> >> > I also would like to port another fix, should we decide to do another
>> >> > release off the 0.6.x branch.
>> >>
>> >> What is the other fix? This one is not critical (I see it just like a
>> >> documentation bug: either way we need that check on that field to work
>> >> that way, we can't simply check the line length).
>> >>
>> >
>> > Parsing of folded fields. The default field parser in 0.6 chokes on
>> > perfectly valid fields if their body is folded.
>> >
>> >
>> >> > I am also willing to make a push toward the 0.7 release, if no one is
>> >> > going to pick up work on the API changes stared by Stefano on the trunk.
>> >>
>> >> I had really few time but I think I also slowed down because I never
>> >> understood if what I was doing was liked or not. It takes a lot to
>> >> complete stuff, so I would have liked to understand what others thinks
>> >> we should do in 0.7.
>> >>
>> >> As an example I see sometimes we talk as 0.x versions we can do
>> >> backward incompatible changes trying to reach a good api, other times
>> >> it seems we instead are "stuck" to the 0.6 version because 0.6 has
>> >> already a lot of users so compatibility is brought to the table.
>> >>
>> >> That said, I say my opinion and I expect others to say their opinions
>> >> so that we can see where we can find a consensus.
>> >> - I think 0.6 is not "great" as API, so I would happily break
>> >> compatibility in order to provide a better api. Main thing is that the
>> >> 0.6 API does not accept evolution (every non trivial feature will
>> >> require a backward incompatible change).
>> >> - IMO current trunk could be released as 0.7.0 with very minor change:
>> >> it is far from exposing a complete api, but I find it already better
>> >> than 0.6 and I have already some product depending on current trunk.
>> >> We saw we proceed at a slow speed, so we should be prepared improving
>> >> the API while we reach 1.0.
>> >> - I guess most of changes we have in trunk are not backportable to 0.6
>> >> because they have been possible by the major refactorings, but I'm not
>> >> against this, if anyone sees a way.
>> >>
>> >> Can you state yours and also tell something more about "your" 0.7 plan?
>> >>
>> >
>> > I think we discussed this on more than one occasion in the past. While I
>> > think mime4j 'core' in 0.7 is fine, the 'dom' / 'message' stuff is not,
>>
>> Yes, we discussed a couple of times, but we didn't find a solution (at
>> least not one I understood)
>>
>> > and the whole library is not in a releasable state at the moment.
>>
>> Got it: hope you will review trunk soon to understand what changes you
>> propose to make it releasable.
>>
>> > And there is "my" plan:
>> >
>> > (1) ask people to go over issues in JIRA and decide what is in scope for
>> > 0.7 and what can wait until a better day (0.8)
>>
>> +1
>>
>> The main causes I use trunk in production instead of 0.6 are:
>> MIME4J-158 - Reduce usage of commons-logging in favor of a "Monitor" service.
>> https://issues.apache.org/jira/browse/MIME4J-158
>> MIME4J-58 - Lenient dealing with headless messages or malformed
>> header/body separation
>> https://issues.apache.org/jira/browse/MIME4J-58
>> MIME4J-153 - Headless inconsistency between MimeTokenStream and MimeStreamParser
>> https://issues.apache.org/jira/browse/MIME4J-153
>>
>> Also the folded header stuff you mentioned (MIME4J-141 - MIME4J-146)
>>
>> > (2) revisit the 'dom' and 'message' packages and try to figure out
>> > whether 'model' and 'implementation' classes in their present form make
>> > sense.
>>
>> +1
>>
>> > In my option, many of them do not.
>>
>> They are the result of my limited use case: they work fine in my use
>> cases (jDKIM + proprietary product).
>> We need more real use cases to "shape" them, but I trust you (and you
>> probably have good uses cases too), so I will wait for your review.
>>
>> Stefano
>>
>
> Stefano, for the love of God Almighty, what else am I supposed to do?

I don't get what I said to upset you. I'm sorry if you thought I'm on
the way. I keep repeating you just commit your changes, just do
whatever you want on svn and I will be happy. You don't commit
anything so I try discussing. I don't know what you prefer me to do.
I'm just trying to understand how to evolve mime4j.

> I
> pointed out a number of times those things that do not seem to make any
> sense what so ever, like HeaderImpl extending Header, which is a
> CONCRETE class, or abstract Multipart where the only abstract aspects
> are preamble / epilogue related methods.
>
> OK. I will create a copy of mime4j on github and make _minimal_ changes
> to your code just to resolve the most glaring WTFs in the API and
> present it for review. Simply listing things that I disagree with does
> not seem to bring us anywhere anymore.

Why can't you simply work on trunk or on a branch here in our svn? I
think I encouraged you multiple times to simply do this, I don't
really understand why you aswer me like I'm trying to stop you.
My preference is for you to use trunk. We use CTR, so go ahead and I
hope you will be happy if I review what you do.

You are a committer, so we already trust you, so you should no fear
working in trunk. I do this when I have ideas and I expect others to
simply do the same. If what I committed in trunk is blocking evolution
then just revert it, otherwise make your changes: whatever you feel
appropriate.

We will ask questions when we need answers :-)

Stefano

PS: if something I said/did makes you angry just explain me please. I
hope this is just something related to "translation" and the fact we
don't speak the same language natively.

Re: 0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Oleg Kalnichevski <ol...@apache.org>]

On Tue, 2011-01-11 at 15:18 +0100, Stefano Bagnara wrote:
> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
> > On Tue, 2011-01-11 at 14:01 +0100, Stefano Bagnara wrote:
> >> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
> >> > On Tue, 2011-01-11 at 09:27 +0100, Norman Maurer wrote:
> >> >> Hi there,
> >> >>
> >> >> I think if it worth it we should release 0.6.2. Release often release
> >> >> early, you know ;)
> >> >>
> >> >> Bye,
> >> >> Norman
> >> >>
> >> >
> >> > Folks
> >> >
> >> > I also would like to port another fix, should we decide to do another
> >> > release off the 0.6.x branch.
> >>
> >> What is the other fix? This one is not critical (I see it just like a
> >> documentation bug: either way we need that check on that field to work
> >> that way, we can't simply check the line length).
> >>
> >
> > Parsing of folded fields. The default field parser in 0.6 chokes on
> > perfectly valid fields if their body is folded.
> >
> >
> >> > I am also willing to make a push toward the 0.7 release, if no one is
> >> > going to pick up work on the API changes stared by Stefano on the trunk.
> >>
> >> I had really few time but I think I also slowed down because I never
> >> understood if what I was doing was liked or not. It takes a lot to
> >> complete stuff, so I would have liked to understand what others thinks
> >> we should do in 0.7.
> >>
> >> As an example I see sometimes we talk as 0.x versions we can do
> >> backward incompatible changes trying to reach a good api, other times
> >> it seems we instead are "stuck" to the 0.6 version because 0.6 has
> >> already a lot of users so compatibility is brought to the table.
> >>
> >> That said, I say my opinion and I expect others to say their opinions
> >> so that we can see where we can find a consensus.
> >> - I think 0.6 is not "great" as API, so I would happily break
> >> compatibility in order to provide a better api. Main thing is that the
> >> 0.6 API does not accept evolution (every non trivial feature will
> >> require a backward incompatible change).
> >> - IMO current trunk could be released as 0.7.0 with very minor change:
> >> it is far from exposing a complete api, but I find it already better
> >> than 0.6 and I have already some product depending on current trunk.
> >> We saw we proceed at a slow speed, so we should be prepared improving
> >> the API while we reach 1.0.
> >> - I guess most of changes we have in trunk are not backportable to 0.6
> >> because they have been possible by the major refactorings, but I'm not
> >> against this, if anyone sees a way.
> >>
> >> Can you state yours and also tell something more about "your" 0.7 plan?
> >>
> >
> > I think we discussed this on more than one occasion in the past. While I
> > think mime4j 'core' in 0.7 is fine, the 'dom' / 'message' stuff is not,
> 
> Yes, we discussed a couple of times, but we didn't find a solution (at
> least not one I understood)
> 
> > and the whole library is not in a releasable state at the moment.
> 
> Got it: hope you will review trunk soon to understand what changes you
> propose to make it releasable.
> 
> > And there is "my" plan:
> >
> > (1) ask people to go over issues in JIRA and decide what is in scope for
> > 0.7 and what can wait until a better day (0.8)
> 
> +1
> 
> The main causes I use trunk in production instead of 0.6 are:
> MIME4J-158 - Reduce usage of commons-logging in favor of a "Monitor" service.
> https://issues.apache.org/jira/browse/MIME4J-158
> MIME4J-58 - Lenient dealing with headless messages or malformed
> header/body separation
> https://issues.apache.org/jira/browse/MIME4J-58
> MIME4J-153 - Headless inconsistency between MimeTokenStream and MimeStreamParser
> https://issues.apache.org/jira/browse/MIME4J-153
> 
> Also the folded header stuff you mentioned (MIME4J-141 - MIME4J-146)
> 
> > (2) revisit the 'dom' and 'message' packages and try to figure out
> > whether 'model' and 'implementation' classes in their present form make
> > sense.
> 
> +1
> 
> > In my option, many of them do not.
> 
> They are the result of my limited use case: they work fine in my use
> cases (jDKIM + proprietary product).
> We need more real use cases to "shape" them, but I trust you (and you
> probably have good uses cases too), so I will wait for your review.
> 
> Stefano
> 

Stefano, for the love of God Almighty, what else am I supposed to do? I
pointed out a number of times those things that do not seem to make any
sense what so ever, like HeaderImpl extending Header, which is a
CONCRETE class, or abstract Multipart where the only abstract aspects
are preamble / epilogue related methods.  

OK. I will create a copy of mime4j on github and make _minimal_ changes
to your code just to resolve the most glaring WTFs in the API and
present it for review. Simply listing things that I disagree with does
not seem to bring us anywhere anymore.

Cheers

Oleg

Re: 0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Stefano Bagnara <ap...@bago.org>]

2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
> On Tue, 2011-01-11 at 14:01 +0100, Stefano Bagnara wrote:
>> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
>> > On Tue, 2011-01-11 at 09:27 +0100, Norman Maurer wrote:
>> >> Hi there,
>> >>
>> >> I think if it worth it we should release 0.6.2. Release often release
>> >> early, you know ;)
>> >>
>> >> Bye,
>> >> Norman
>> >>
>> >
>> > Folks
>> >
>> > I also would like to port another fix, should we decide to do another
>> > release off the 0.6.x branch.
>>
>> What is the other fix? This one is not critical (I see it just like a
>> documentation bug: either way we need that check on that field to work
>> that way, we can't simply check the line length).
>>
>
> Parsing of folded fields. The default field parser in 0.6 chokes on
> perfectly valid fields if their body is folded.
>
>
>> > I am also willing to make a push toward the 0.7 release, if no one is
>> > going to pick up work on the API changes stared by Stefano on the trunk.
>>
>> I had really few time but I think I also slowed down because I never
>> understood if what I was doing was liked or not. It takes a lot to
>> complete stuff, so I would have liked to understand what others thinks
>> we should do in 0.7.
>>
>> As an example I see sometimes we talk as 0.x versions we can do
>> backward incompatible changes trying to reach a good api, other times
>> it seems we instead are "stuck" to the 0.6 version because 0.6 has
>> already a lot of users so compatibility is brought to the table.
>>
>> That said, I say my opinion and I expect others to say their opinions
>> so that we can see where we can find a consensus.
>> - I think 0.6 is not "great" as API, so I would happily break
>> compatibility in order to provide a better api. Main thing is that the
>> 0.6 API does not accept evolution (every non trivial feature will
>> require a backward incompatible change).
>> - IMO current trunk could be released as 0.7.0 with very minor change:
>> it is far from exposing a complete api, but I find it already better
>> than 0.6 and I have already some product depending on current trunk.
>> We saw we proceed at a slow speed, so we should be prepared improving
>> the API while we reach 1.0.
>> - I guess most of changes we have in trunk are not backportable to 0.6
>> because they have been possible by the major refactorings, but I'm not
>> against this, if anyone sees a way.
>>
>> Can you state yours and also tell something more about "your" 0.7 plan?
>>
>
> I think we discussed this on more than one occasion in the past. While I
> think mime4j 'core' in 0.7 is fine, the 'dom' / 'message' stuff is not,

Yes, we discussed a couple of times, but we didn't find a solution (at
least not one I understood)

> and the whole library is not in a releasable state at the moment.

Got it: hope you will review trunk soon to understand what changes you
propose to make it releasable.

> And there is "my" plan:
>
> (1) ask people to go over issues in JIRA and decide what is in scope for
> 0.7 and what can wait until a better day (0.8)

+1

The main causes I use trunk in production instead of 0.6 are:
MIME4J-158 - Reduce usage of commons-logging in favor of a "Monitor" service.
https://issues.apache.org/jira/browse/MIME4J-158
MIME4J-58 - Lenient dealing with headless messages or malformed
header/body separation
https://issues.apache.org/jira/browse/MIME4J-58
MIME4J-153 - Headless inconsistency between MimeTokenStream and MimeStreamParser
https://issues.apache.org/jira/browse/MIME4J-153

Also the folded header stuff you mentioned (MIME4J-141 - MIME4J-146)

> (2) revisit the 'dom' and 'message' packages and try to figure out
> whether 'model' and 'implementation' classes in their present form make
> sense.

+1

> In my option, many of them do not.

They are the result of my limited use case: they work fine in my use
cases (jDKIM + proprietary product).
We need more real use cases to "shape" them, but I trust you (and you
probably have good uses cases too), so I will wait for your review.

Stefano

Re: 0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Oleg Kalnichevski <ol...@apache.org>]

On Tue, 2011-01-11 at 14:01 +0100, Stefano Bagnara wrote:
> 2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
> > On Tue, 2011-01-11 at 09:27 +0100, Norman Maurer wrote:
> >> Hi there,
> >>
> >> I think if it worth it we should release 0.6.2. Release often release
> >> early, you know ;)
> >>
> >> Bye,
> >> Norman
> >>
> >
> > Folks
> >
> > I also would like to port another fix, should we decide to do another
> > release off the 0.6.x branch.
> 
> What is the other fix? This one is not critical (I see it just like a
> documentation bug: either way we need that check on that field to work
> that way, we can't simply check the line length).
> 

Parsing of folded fields. The default field parser in 0.6 chokes on
perfectly valid fields if their body is folded. 


> > I am also willing to make a push toward the 0.7 release, if no one is
> > going to pick up work on the API changes stared by Stefano on the trunk.
> 
> I had really few time but I think I also slowed down because I never
> understood if what I was doing was liked or not. It takes a lot to
> complete stuff, so I would have liked to understand what others thinks
> we should do in 0.7.
> 
> As an example I see sometimes we talk as 0.x versions we can do
> backward incompatible changes trying to reach a good api, other times
> it seems we instead are "stuck" to the 0.6 version because 0.6 has
> already a lot of users so compatibility is brought to the table.
> 
> That said, I say my opinion and I expect others to say their opinions
> so that we can see where we can find a consensus.
> - I think 0.6 is not "great" as API, so I would happily break
> compatibility in order to provide a better api. Main thing is that the
> 0.6 API does not accept evolution (every non trivial feature will
> require a backward incompatible change).
> - IMO current trunk could be released as 0.7.0 with very minor change:
> it is far from exposing a complete api, but I find it already better
> than 0.6 and I have already some product depending on current trunk.
> We saw we proceed at a slow speed, so we should be prepared improving
> the API while we reach 1.0.
> - I guess most of changes we have in trunk are not backportable to 0.6
> because they have been possible by the major refactorings, but I'm not
> against this, if anyone sees a way.
> 
> Can you state yours and also tell something more about "your" 0.7 plan?
> 

I think we discussed this on more than one occasion in the past. While I
think mime4j 'core' in 0.7 is fine, the 'dom' / 'message' stuff is not,
and the whole library is not in a releasable state at the moment. 

And there is "my" plan:

(1) ask people to go over issues in JIRA and decide what is in scope for
0.7 and what can wait until a better day (0.8)
(2) revisit the 'dom' and 'message' packages and try to figure out
whether 'model' and 'implementation' classes in their present form make
sense. In my option, many of them do not.

Oleg

Re: 0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Stefano Bagnara <ap...@bago.org>]

2011/1/11 Oleg Kalnichevski <ol...@apache.org>:
> On Tue, 2011-01-11 at 09:27 +0100, Norman Maurer wrote:
>> Hi there,
>>
>> I think if it worth it we should release 0.6.2. Release often release
>> early, you know ;)
>>
>> Bye,
>> Norman
>>
>
> Folks
>
> I also would like to port another fix, should we decide to do another
> release off the 0.6.x branch.

What is the other fix? This one is not critical (I see it just like a
documentation bug: either way we need that check on that field to work
that way, we can't simply check the line length).

> I am also willing to make a push toward the 0.7 release, if no one is
> going to pick up work on the API changes stared by Stefano on the trunk.

I had really few time but I think I also slowed down because I never
understood if what I was doing was liked or not. It takes a lot to
complete stuff, so I would have liked to understand what others thinks
we should do in 0.7.

As an example I see sometimes we talk as 0.x versions we can do
backward incompatible changes trying to reach a good api, other times
it seems we instead are "stuck" to the 0.6 version because 0.6 has
already a lot of users so compatibility is brought to the table.

That said, I say my opinion and I expect others to say their opinions
so that we can see where we can find a consensus.
- I think 0.6 is not "great" as API, so I would happily break
compatibility in order to provide a better api. Main thing is that the
0.6 API does not accept evolution (every non trivial feature will
require a backward incompatible change).
- IMO current trunk could be released as 0.7.0 with very minor change:
it is far from exposing a complete api, but I find it already better
than 0.6 and I have already some product depending on current trunk.
We saw we proceed at a slow speed, so we should be prepared improving
the API while we reach 1.0.
- I guess most of changes we have in trunk are not backportable to 0.6
because they have been possible by the major refactorings, but I'm not
against this, if anyone sees a way.

Can you state yours and also tell something more about "your" 0.7 plan?

Stefano

0.6.2 or 0.7 any time soon? was Re: Incorrect line length limitation while parsing headers

[Oleg Kalnichevski <ol...@apache.org>]

On Tue, 2011-01-11 at 09:27 +0100, Norman Maurer wrote:
> Hi there,
> 
> I think if it worth it we should release 0.6.2. Release often release
> early, you know ;)
> 
> Bye,
> Norman
> 

Folks

I also would like to port another fix, should we decide to do another
release off the 0.6.x branch.

I am also willing to make a push toward the 0.7 release, if no one is
going to pick up work on the API changes stared by Stefano on the trunk.

Oleg 

Re: Incorrect line length limitation while parsing headers

[Norman Maurer <no...@apache.org>]

Hi there,

I think if it worth it we should release 0.6.2. Release often release
early, you know ;)

Bye,
Norman

2011/1/11 Stefano Bagnara <ap...@bago.org>:
> Hi,
>
> in trunk we changed the exception to "max header length exception": as
> we use a single buffer to store a whole header (even if it is folded
> in 100 lines) we need to protect from memory usage using such limit.
> We just released 0.6.1, not sure if we will release 0.6.2 or wait for
> 0.7. In case we will backport the change from trunk and change the
> exception description instead of changing the logic.
>
> Please file an issue at http://issues.apache.org/jira/browse/MIME4J
>
> Stefano
>
> 2011/1/11 Jesse Long <je...@virtualpostman.co.za>:
>> Hi,
>>
>> Please see the following code in mime4j 0.6:
>>
>> http://james.apache.org/mime4j/xref/org/apache/james/mime4j/parser/AbstractEntity.html#133
>>
>> The for loop checks for excess line length in headers based on the complete
>> length of the header, not on each individual line in the header.
>>
>> So, my header like this:
>>
>> X-Header-Name: <980 chars><CRLF>
>> <20 chars><CRLF>
>> <25 chars><CRLF>
>>
>> fails, because the complete length of the header is > 1000, however, no line
>> in the mime message is longer than 1000 chars...
>>
>> I suggest just removing the check for excess line length in the
>> fillFieldBuffer() method.
>>
>> Cheers,
>> Jesse
>>
>

Re: Incorrect line length limitation while parsing headers

[Stefano Bagnara <ap...@bago.org>]

2011/1/12 Jesse Long <jp...@unknown.za.net>:
> Hi All,
>
> With regards to the bug - Just changing the wording of the exception is not
> really sufficient. Yes, it will now correctly describe the error, but my
> problem persists. I have a, say, 2300 byte header - I cannot parse it. I
> understand that we must protect against arbitrarily long headers, like a
> 50Mb header, but surely we must also allow a header that spans multiple
> lines. The whole point of header folding is to allow headers longer that
> 1000 chars. So, we must make the header size limit greater than 1000 chars.
> Maybe 10000 chars is an acceptable default header size limit?
>
> I had a look at trunk, and the code has change somewhat. I cant tell quickly
> if the code in trunk is using the 1000 char limit for header size or not.
>
> My request: please perform a release on the 0.6 branch (0.6.2) that allows
> me to parse headers in excess of 1000 chars when using code like:
>
> Message m = new Message(inputStream);

Having a better default makes sense, please file a JIRA issue.
http://issues.apache.org/jira/browse/MIME4J

> On 11/01/2011 10:24, Stefano Bagnara wrote:
>>
>> Hi,
>>
>> in trunk we changed the exception to "max header length exception": as
>> we use a single buffer to store a whole header (even if it is folded
>> in 100 lines) we need to protect from memory usage using such limit.
>> We just released 0.6.1, not sure if we will release 0.6.2 or wait for
>> 0.7. In case we will backport the change from trunk and change the
>> exception description instead of changing the logic.
>>
>> Please file an issue at http://issues.apache.org/jira/browse/MIME4J
>>
>> Stefano
>>
>> 2011/1/11 Jesse Long<je...@virtualpostman.co.za>:
>>>
>>> Hi,
>>>
>>> Please see the following code in mime4j 0.6:
>>>
>>>
>>> http://james.apache.org/mime4j/xref/org/apache/james/mime4j/parser/AbstractEntity.html#133
>>>
>>> The for loop checks for excess line length in headers based on the
>>> complete
>>> length of the header, not on each individual line in the header.
>>>
>>> So, my header like this:
>>>
>>> X-Header-Name:<980 chars><CRLF>
>>> <20 chars><CRLF>
>>> <25 chars><CRLF>
>>>
>>> fails, because the complete length of the header is>  1000, however, no
>>> line
>>> in the mime message is longer than 1000 chars...
>>>
>>> I suggest just removing the check for excess line length in the
>>> fillFieldBuffer() method.
>>>
>>> Cheers,
>>> Jesse
>>>
>
>

Re: Incorrect line length limitation while parsing headers

[Jesse Long <jp...@unknown.za.net>]

Hi All,

With regards to the bug - Just changing the wording of the exception is 
not really sufficient. Yes, it will now correctly describe the error, 
but my problem persists. I have a, say, 2300 byte header - I cannot 
parse it. I understand that we must protect against arbitrarily long 
headers, like a 50Mb header, but surely we must also allow a header that 
spans multiple lines. The whole point of header folding is to allow 
headers longer that 1000 chars. So, we must make the header size limit 
greater than 1000 chars. Maybe 10000 chars is an acceptable default 
header size limit?

I had a look at trunk, and the code has change somewhat. I cant tell 
quickly if the code in trunk is using the 1000 char limit for header 
size or not.

My request: please perform a release on the 0.6 branch (0.6.2) that 
allows me to parse headers in excess of 1000 chars when using code like:

Message m = new Message(inputStream);

Thanks,
Jesse

On 11/01/2011 10:24, Stefano Bagnara wrote:
> Hi,
>
> in trunk we changed the exception to "max header length exception": as
> we use a single buffer to store a whole header (even if it is folded
> in 100 lines) we need to protect from memory usage using such limit.
> We just released 0.6.1, not sure if we will release 0.6.2 or wait for
> 0.7. In case we will backport the change from trunk and change the
> exception description instead of changing the logic.
>
> Please file an issue at http://issues.apache.org/jira/browse/MIME4J
>
> Stefano
>
> 2011/1/11 Jesse Long<je...@virtualpostman.co.za>:
>> Hi,
>>
>> Please see the following code in mime4j 0.6:
>>
>> http://james.apache.org/mime4j/xref/org/apache/james/mime4j/parser/AbstractEntity.html#133
>>
>> The for loop checks for excess line length in headers based on the complete
>> length of the header, not on each individual line in the header.
>>
>> So, my header like this:
>>
>> X-Header-Name:<980 chars><CRLF>
>> <20 chars><CRLF>
>> <25 chars><CRLF>
>>
>> fails, because the complete length of the header is>  1000, however, no line
>> in the mime message is longer than 1000 chars...
>>
>> I suggest just removing the check for excess line length in the
>> fillFieldBuffer() method.
>>
>> Cheers,
>> Jesse
>>

Re: Incorrect line length limitation while parsing headers

[Stefano Bagnara <ap...@bago.org>]

Hi,

in trunk we changed the exception to "max header length exception": as
we use a single buffer to store a whole header (even if it is folded
in 100 lines) we need to protect from memory usage using such limit.
We just released 0.6.1, not sure if we will release 0.6.2 or wait for
0.7. In case we will backport the change from trunk and change the
exception description instead of changing the logic.

Please file an issue at http://issues.apache.org/jira/browse/MIME4J

Stefano

2011/1/11 Jesse Long <je...@virtualpostman.co.za>:
> Hi,
>
> Please see the following code in mime4j 0.6:
>
> http://james.apache.org/mime4j/xref/org/apache/james/mime4j/parser/AbstractEntity.html#133
>
> The for loop checks for excess line length in headers based on the complete
> length of the header, not on each individual line in the header.
>
> So, my header like this:
>
> X-Header-Name: <980 chars><CRLF>
> <20 chars><CRLF>
> <25 chars><CRLF>
>
> fails, because the complete length of the header is > 1000, however, no line
> in the mime message is longer than 1000 chars...
>
> I suggest just removing the check for excess line length in the
> fillFieldBuffer() method.
>
> Cheers,
> Jesse
>