james-mime4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Markus Wiederkehr <markus.wiederk...@gmail.com>
Subject Re: Address list parser expects unfolded field body. Does that make sense?
Date Sun, 29 Nov 2009 19:22:42 GMT
On Sat, Nov 28, 2009 at 12:19 AM, Oleg Kalnichevski <olegk@apache.org> wrote:
> Markus Wiederkehr wrote:
>>
>> On Fri, Nov 27, 2009 at 10:21 PM, Oleg Kalnichevski <olegk@apache.org>
>> wrote:
>>>
>>> Markus Wiederkehr wrote:
>>>>
>>>> On Fri, Nov 27, 2009 at 2:37 PM, Oleg Kalnichevski <olegk@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

Mime
View raw message