You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mime4j-dev@james.apache.org by Markus Wiederkehr <ma...@gmail.com> on 2009/11/27 19:58:12 UTC
Re: Address list parser expects unfolded field body. Does that make
sense?
On Fri, Nov 27, 2009 at 2:37 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> Oleg Kalnichevski wrote:
>>
>> Markus et al
>>
>> The address list parser currently chokes on folded field values that are
>> otherwise perfectly valid. That seems somewhat illogical to me. It really
>> took me a while to figure out what was wrong with the address list until I
>> stumbled upon a commend about the parser expecting unfolded fields. The very
>> cryptic exception message did not really help either.
>>
>> What is the reason for this restriction? It is because folded values are
>> difficult to parse with jjtree? Should not we unfold field values
>> automatically prior to feeding them to the parser?
As for the reason, I don't know, that was before my time..
>> Oleg
>>
>
> DelegatingFieldParser#parse method does not automatically unfold the field
> body, which actually seems like a bug to me. What is the expected behavior
> of this method?
>From what I see the unfolding happens in
AbstractField.parse(ByteSequence, String) at line 155. The call
hierarchy leads to AbstractField.parse(ByteSequence) and
MessageBuilder.field(Field)..
I guess you should use AbstractField.parse(ByteSequence).
Markus
Re: Address list parser expects unfolded field body. Does that make
sense?
Posted by Markus Wiederkehr <ma...@gmail.com>.
On Sat, Nov 28, 2009 at 12:19 AM, Oleg Kalnichevski <ol...@apache.org> wrote:
> Markus Wiederkehr wrote:
>>
>> On Fri, Nov 27, 2009 at 10:21 PM, Oleg Kalnichevski <ol...@apache.org>
>> wrote:
>>>
>>> Markus Wiederkehr wrote:
>>>>
>>>> On Fri, Nov 27, 2009 at 2:37 PM, Oleg Kalnichevski <ol...@apache.org>
>>>> wrote:
>>>>>
>>>>> Oleg Kalnichevski wrote:
>>>>>>
>>>>>> Markus et al
>>>>>>
>>>>>> The address list parser currently chokes on folded field values that
>>>>>> are
>>>>>> otherwise perfectly valid. That seems somewhat illogical to me. It
>>>>>> really
>>>>>> took me a while to figure out what was wrong with the address list
>>>>>> until
>>>>>> I
>>>>>> stumbled upon a commend about the parser expecting unfolded fields.
>>>>>> The
>>>>>> very
>>>>>> cryptic exception message did not really help either.
>>>>>>
>>>>>> What is the reason for this restriction? It is because folded values
>>>>>> are
>>>>>> difficult to parse with jjtree? Should not we unfold field values
>>>>>> automatically prior to feeding them to the parser?
>>>>
>>>> As for the reason, I don't know, that was before my time..
>>>>
>>> And also before mine
>>>
>>>
>>>>>> Oleg
>>>>>>
>>>>> DelegatingFieldParser#parse method does not automatically unfold the
>>>>> field
>>>>> body, which actually seems like a bug to me. What is the expected
>>>>> behavior
>>>>> of this method?
>>>>> From what I see the unfolding happens in
>>>>
>>>> AbstractField.parse(ByteSequence, String) at line 155. The call
>>>> hierarchy leads to AbstractField.parse(ByteSequence) and
>>>> MessageBuilder.field(Field)..
>>>>
>>>> I guess you should use AbstractField.parse(ByteSequence).
>>>>
>>> That still leaves us with DefaultFieldParser and DelegatingFieldParser
>>> that
>>> produce incorrect results, at least in my opinion.
>>
>> Are you referring to DelegatingFieldParser.parse(String, String,
>> ByteSequence)? That method implements FieldParser#parse which can only
>> be considered to be internal API in my opinion.
>
> That would not be a problem if DefaultFieldParser did not subclass
> DelegatingFieldParser
>
>
> Well maybe not quite,
>>
>> a user might want to implement a custom FieldParser but I don't think
>> this method is intended to be invoked directly.
>>
>> Generally you don't have the name, body and raw byte sequence at hand.
>
> When working with MimeTokenStream generally you do.
Okay, that makes sense.
Feel free to change the behaviour if you like (unless someone else
objects). Maybe it would also be sufficient to provide good
documentation for the current behaviour instead. I mean all you'd have
to do to fix your code is add one MimeUtil.unfold() call.
Markus
>
>> Usually name and body are computed from the raw byte sequence or vice
>> versa. So what's wrong with using AbstractField.parse(ByteSequence)
>> instead?
>>
>
> There is nothing wrong with it but inconveniently there is a public class
> called DefaultFieldParser, which seems to suggest that this is what users
> are supposed to use per default. Why would a user pick AbstractField over
> DefaultFieldParser to parse fields?
>
>
>>> Are there any objections to changing the behavior of these classes?
>>
>> I don't see the point. What's your use case?
>>
>
> MimeTokenStream mimestream = new MimeTokenStream();
> mimestream.parse(instream);
>
> FieldParser parser = new DefaultFieldParser();
> boolean completed = false;
> while (!completed) {
> int state = mimestream.getState();
> switch (state) {
> case MimeTokenStream.T_FIELD:
> Field field = mimestream.getField();
> String name = field.getName();
> String body = field.getBody();
> ByteSequence raw = field.getRaw();
> ParsedField parsedField = parser.parse(name, body, raw);
> break;
> case MimeTokenStream.T_END_OF_STREAM:
> completed = true;
> break;
> }
> if (!completed) {
> mimestream.next();
> }
> }
>
> Oleg
>
>> Markus
>>
>>> Oleg
>>>
>>>> Markus
Re: Address list parser expects unfolded field body. Does that make
sense?
Posted by Oleg Kalnichevski <ol...@apache.org>.
Markus Wiederkehr wrote:
> On Fri, Nov 27, 2009 at 10:21 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
>> Markus Wiederkehr wrote:
>>> On Fri, Nov 27, 2009 at 2:37 PM, Oleg Kalnichevski <ol...@apache.org>
>>> wrote:
>>>> Oleg Kalnichevski wrote:
>>>>> Markus et al
>>>>>
>>>>> The address list parser currently chokes on folded field values that are
>>>>> otherwise perfectly valid. That seems somewhat illogical to me. It
>>>>> really
>>>>> took me a while to figure out what was wrong with the address list until
>>>>> I
>>>>> stumbled upon a commend about the parser expecting unfolded fields. The
>>>>> very
>>>>> cryptic exception message did not really help either.
>>>>>
>>>>> What is the reason for this restriction? It is because folded values are
>>>>> difficult to parse with jjtree? Should not we unfold field values
>>>>> automatically prior to feeding them to the parser?
>>> As for the reason, I don't know, that was before my time..
>>>
>> And also before mine
>>
>>
>>>>> Oleg
>>>>>
>>>> DelegatingFieldParser#parse method does not automatically unfold the
>>>> field
>>>> body, which actually seems like a bug to me. What is the expected
>>>> behavior
>>>> of this method?
>>>> From what I see the unfolding happens in
>>> AbstractField.parse(ByteSequence, String) at line 155. The call
>>> hierarchy leads to AbstractField.parse(ByteSequence) and
>>> MessageBuilder.field(Field)..
>>>
>>> I guess you should use AbstractField.parse(ByteSequence).
>>>
>> That still leaves us with DefaultFieldParser and DelegatingFieldParser that
>> produce incorrect results, at least in my opinion.
>
> Are you referring to DelegatingFieldParser.parse(String, String,
> ByteSequence)? That method implements FieldParser#parse which can only
> be considered to be internal API in my opinion.
That would not be a problem if DefaultFieldParser did not subclass
DelegatingFieldParser
Well maybe not quite,
> a user might want to implement a custom FieldParser but I don't think
> this method is intended to be invoked directly.
>
> Generally you don't have the name, body and raw byte sequence at hand.
When working with MimeTokenStream generally you do.
> Usually name and body are computed from the raw byte sequence or vice
> versa. So what's wrong with using AbstractField.parse(ByteSequence)
> instead?
>
There is nothing wrong with it but inconveniently there is a public
class called DefaultFieldParser, which seems to suggest that this is
what users are supposed to use per default. Why would a user pick
AbstractField over DefaultFieldParser to parse fields?
>> Are there any objections to changing the behavior of these classes?
>
> I don't see the point. What's your use case?
>
MimeTokenStream mimestream = new MimeTokenStream();
mimestream.parse(instream);
FieldParser parser = new DefaultFieldParser();
boolean completed = false;
while (!completed) {
int state = mimestream.getState();
switch (state) {
case MimeTokenStream.T_FIELD:
Field field = mimestream.getField();
String name = field.getName();
String body = field.getBody();
ByteSequence raw = field.getRaw();
ParsedField parsedField = parser.parse(name, body, raw);
break;
case MimeTokenStream.T_END_OF_STREAM:
completed = true;
break;
}
if (!completed) {
mimestream.next();
}
}
Oleg
> Markus
>
>> Oleg
>>
>>> Markus
>
Re: Address list parser expects unfolded field body. Does that make
sense?
Posted by Markus Wiederkehr <ma...@gmail.com>.
On Fri, Nov 27, 2009 at 10:21 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> Markus Wiederkehr wrote:
>>
>> On Fri, Nov 27, 2009 at 2:37 PM, Oleg Kalnichevski <ol...@apache.org>
>> wrote:
>>>
>>> Oleg Kalnichevski wrote:
>>>>
>>>> Markus et al
>>>>
>>>> The address list parser currently chokes on folded field values that are
>>>> otherwise perfectly valid. That seems somewhat illogical to me. It
>>>> really
>>>> took me a while to figure out what was wrong with the address list until
>>>> I
>>>> stumbled upon a commend about the parser expecting unfolded fields. The
>>>> very
>>>> cryptic exception message did not really help either.
>>>>
>>>> What is the reason for this restriction? It is because folded values are
>>>> difficult to parse with jjtree? Should not we unfold field values
>>>> automatically prior to feeding them to the parser?
>>
>> As for the reason, I don't know, that was before my time..
>>
>
> And also before mine
>
>
>>>> Oleg
>>>>
>>> DelegatingFieldParser#parse method does not automatically unfold the
>>> field
>>> body, which actually seems like a bug to me. What is the expected
>>> behavior
>>> of this method?
>>
>>> From what I see the unfolding happens in
>>
>> AbstractField.parse(ByteSequence, String) at line 155. The call
>> hierarchy leads to AbstractField.parse(ByteSequence) and
>> MessageBuilder.field(Field)..
>>
>> I guess you should use AbstractField.parse(ByteSequence).
>>
>
> That still leaves us with DefaultFieldParser and DelegatingFieldParser that
> produce incorrect results, at least in my opinion.
Are you referring to DelegatingFieldParser.parse(String, String,
ByteSequence)? That method implements FieldParser#parse which can only
be considered to be internal API in my opinion. Well maybe not quite,
a user might want to implement a custom FieldParser but I don't think
this method is intended to be invoked directly.
Generally you don't have the name, body and raw byte sequence at hand.
Usually name and body are computed from the raw byte sequence or vice
versa. So what's wrong with using AbstractField.parse(ByteSequence)
instead?
> Are there any objections to changing the behavior of these classes?
I don't see the point. What's your use case?
Markus
> Oleg
>
>> Markus
Re: Address list parser expects unfolded field body. Does that make
sense?
Posted by Oleg Kalnichevski <ol...@apache.org>.
Markus Wiederkehr wrote:
> On Fri, Nov 27, 2009 at 2:37 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
>> Oleg Kalnichevski wrote:
>>> Markus et al
>>>
>>> The address list parser currently chokes on folded field values that are
>>> otherwise perfectly valid. That seems somewhat illogical to me. It really
>>> took me a while to figure out what was wrong with the address list until I
>>> stumbled upon a commend about the parser expecting unfolded fields. The very
>>> cryptic exception message did not really help either.
>>>
>>> What is the reason for this restriction? It is because folded values are
>>> difficult to parse with jjtree? Should not we unfold field values
>>> automatically prior to feeding them to the parser?
>
> As for the reason, I don't know, that was before my time..
>
And also before mine
>>> Oleg
>>>
>> DelegatingFieldParser#parse method does not automatically unfold the field
>> body, which actually seems like a bug to me. What is the expected behavior
>> of this method?
>
>>>From what I see the unfolding happens in
> AbstractField.parse(ByteSequence, String) at line 155. The call
> hierarchy leads to AbstractField.parse(ByteSequence) and
> MessageBuilder.field(Field)..
>
> I guess you should use AbstractField.parse(ByteSequence).
>
That still leaves us with DefaultFieldParser and DelegatingFieldParser
that produce incorrect results, at least in my opinion.
Are there any objections to changing the behavior of these classes?
Oleg
> Markus
>