You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Markus Wiederkehr <ma...@gmail.com> on 2008/11/08 14:31:51 UTC
[mime4j] MimeException in Field.parse()
For MIME4J-73 we changed the IllegalArgumentException in Field.parse()
to a MimeException because loading an invalid message from an
InputStream should not throw a runtime exception.
While this was correct it leads to another problem. Loading a message
from an InputStream is just one scenario. The other one is creating a
new message from scratch. If you know for certain that the header
field to parse is well-formed the MimeException is nothing but a
nuisance.
So maybe what we need is a checked exception for loading a message and
a runtime exception for creating a message programatically. Maybe
Field.parse() should throw an IllegalArgumentException and the code
that loads a message should convert it to a MimeException.
But there is something else. Consider parsing a message that contains
an invalid Date header field. Mime4j does _not_ throw a MimeException
in this case. Instead it returns a DateTimeField instance that
represents the invalid date. This DateTimeField also stores a
ParseException. I think the idea behind this is that it should still
be possible to parse a message even if some header fields are
incorrent.
In this spirit, should Field.parse() throw an exception at all? Or
would it be better to return a Field instance that represents an
invalid header field instead?
What do you think?
Markus
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mime4j] MimeException in Field.parse()
Posted by Robert Burrell Donkin <ro...@gmail.com>.
On Sun, Nov 9, 2008 at 9:50 PM, Markus Wiederkehr
<ma...@gmail.com> wrote:
> On Sun, Nov 9, 2008 at 10:35 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
>> I personally prefer a subclass of Throwable to express some kind of
>> exceptional condition, but could live with InvalidField or some such.
>
> Mime4j does not throw an exception if a date header field is
> malformed. The same is true for erroneous address lists, mailboxes and
> mailbox lists.
the mime4j API is inconsistent. the other fields expose any exception
using a getter. i suspect that this design was adopted to allow the
caller to decide whether they wanted to ignore unparsable fields or
not.
the consistent way to remove the throws clause would be to pull the
getter up from the subclasses. the major problem with this approach
would be to break callers relying on catch blocks which would need to
be replaced with a getter check.
> Wouldn't it be consequent not to throw an exception if a header field
> does not match the required regex?
>
> Another possible solution would be:
>
> public static Field parseUnchecked(final String raw) throws
> IllegalArgumentException {
> // ...
> }
>
> public static Field parse(final String raw) throws MimeException {
> try {
> return parseUnchecked(raw);
> } catch (IllegalArgumentException e) {
> throw new MimeException("Invalid header field", e);
> }
> }
>
> I'm not sure if I like that either..
i would prefer not to use a RuntimeException. a getter would be more
consistent with the rest of the API.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mime4j] MimeException in Field.parse()
Posted by Markus Wiederkehr <ma...@gmail.com>.
On Sun, Nov 9, 2008 at 10:35 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> I personally prefer a subclass of Throwable to express some kind of
> exceptional condition, but could live with InvalidField or some such.
Mime4j does not throw an exception if a date header field is
malformed. The same is true for erroneous address lists, mailboxes and
mailbox lists.
Wouldn't it be consequent not to throw an exception if a header field
does not match the required regex?
Another possible solution would be:
public static Field parseUnchecked(final String raw) throws
IllegalArgumentException {
// ...
}
public static Field parse(final String raw) throws MimeException {
try {
return parseUnchecked(raw);
} catch (IllegalArgumentException e) {
throw new MimeException("Invalid header field", e);
}
}
I'm not sure if I like that either..
Markus
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mime4j] MimeException in Field.parse()
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2008-11-09 at 20:46 +0100, Markus Wiederkehr wrote:
> On Sun, Nov 9, 2008 at 8:33 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> > On Sat, 2008-11-08 at 14:31 +0100, Markus Wiederkehr wrote:
> >> For MIME4J-73 we changed the IllegalArgumentException in Field.parse()
> >> to a MimeException because loading an invalid message from an
> >> InputStream should not throw a runtime exception.
> >>
> >> While this was correct it leads to another problem. Loading a message
> >> from an InputStream is just one scenario. The other one is creating a
> >> new message from scratch. If you know for certain that the header
> >> field to parse is well-formed the MimeException is nothing but a
> >> nuisance.
> >>
> >> So maybe what we need is a checked exception for loading a message and
> >> a runtime exception for creating a message programatically. Maybe
> >> Field.parse() should throw an IllegalArgumentException and the code
> >> that loads a message should convert it to a MimeException.
> >>
> >
> > While Field.parse() throwing a checked exception can be a nuisance, I
> > personally can live with that. The idea of rethrowing
> > IllegalArgumentException as a checked exception does not sound quite
> > right to me.
>
> Throw an IllegalArgumentException as a checked exception? I'm not sure
> I understand..
>
I was trying to say that RE-THROWING IllegalArgumentException as a
checked exception, such as MimeException, did not seem right to me.
> What I meant is Field.parse() is called by Header.Header(InputStream),
> MessageBuilder.field() and SimpleContentHandler.field(). Field.parse()
> could throw an IllegalArgumentException which gets caught in these
> three methods and rethrown as a MimeException.
>
> What would you think of not throwing an exception at all?
>
> The exception gets thrown if the header does not match the
> FIELD_NAME_PATTERN. In this case Field.parse() could return a
> "InvalidField" instance, for example. InvalidField.getName() could
> return the raw value and getBody() could return either null or an
> empty string.
>
I personally prefer a subclass of Throwable to express some kind of
exceptional condition, but could live with InvalidField or some such.
Oleg
> Markus
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mime4j] MimeException in Field.parse()
Posted by Markus Wiederkehr <ma...@gmail.com>.
On Sun, Nov 9, 2008 at 8:46 PM, Markus Wiederkehr
<ma...@gmail.com> wrote:
> On Sun, Nov 9, 2008 at 8:33 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
>> While Field.parse() throwing a checked exception can be a nuisance, I
>> personally can live with that. The idea of rethrowing
>> IllegalArgumentException as a checked exception does not sound quite
>> right to me.
>
> Throw an IllegalArgumentException as a checked exception? I'm not sure
> I understand..
Sorry, you wrote _re_throw. I should learn to read sometime..
Anyway I admit that rethrowing a runtime exception at three different
locations in the API is not very beautiful and error prone in case of
future changes.
But I really hate those "this-cannot-happen" catch blocks and would be
glad if we could find a solution for this.
Markus
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mime4j] MimeException in Field.parse()
Posted by Markus Wiederkehr <ma...@gmail.com>.
On Sun, Nov 9, 2008 at 8:33 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Sat, 2008-11-08 at 14:31 +0100, Markus Wiederkehr wrote:
>> For MIME4J-73 we changed the IllegalArgumentException in Field.parse()
>> to a MimeException because loading an invalid message from an
>> InputStream should not throw a runtime exception.
>>
>> While this was correct it leads to another problem. Loading a message
>> from an InputStream is just one scenario. The other one is creating a
>> new message from scratch. If you know for certain that the header
>> field to parse is well-formed the MimeException is nothing but a
>> nuisance.
>>
>> So maybe what we need is a checked exception for loading a message and
>> a runtime exception for creating a message programatically. Maybe
>> Field.parse() should throw an IllegalArgumentException and the code
>> that loads a message should convert it to a MimeException.
>>
>
> While Field.parse() throwing a checked exception can be a nuisance, I
> personally can live with that. The idea of rethrowing
> IllegalArgumentException as a checked exception does not sound quite
> right to me.
Throw an IllegalArgumentException as a checked exception? I'm not sure
I understand..
What I meant is Field.parse() is called by Header.Header(InputStream),
MessageBuilder.field() and SimpleContentHandler.field(). Field.parse()
could throw an IllegalArgumentException which gets caught in these
three methods and rethrown as a MimeException.
What would you think of not throwing an exception at all?
The exception gets thrown if the header does not match the
FIELD_NAME_PATTERN. In this case Field.parse() could return a
"InvalidField" instance, for example. InvalidField.getName() could
return the raw value and getBody() could return either null or an
empty string.
Markus
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mime4j] MimeException in Field.parse()
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sat, 2008-11-08 at 14:31 +0100, Markus Wiederkehr wrote:
> For MIME4J-73 we changed the IllegalArgumentException in Field.parse()
> to a MimeException because loading an invalid message from an
> InputStream should not throw a runtime exception.
>
> While this was correct it leads to another problem. Loading a message
> from an InputStream is just one scenario. The other one is creating a
> new message from scratch. If you know for certain that the header
> field to parse is well-formed the MimeException is nothing but a
> nuisance.
>
> So maybe what we need is a checked exception for loading a message and
> a runtime exception for creating a message programatically. Maybe
> Field.parse() should throw an IllegalArgumentException and the code
> that loads a message should convert it to a MimeException.
>
While Field.parse() throwing a checked exception can be a nuisance, I
personally can live with that. The idea of rethrowing
IllegalArgumentException as a checked exception does not sound quite
right to me.
I will not stand in the way if someone wants to implement a different
approach, though.
Oleg
> But there is something else. Consider parsing a message that contains
> an invalid Date header field. Mime4j does _not_ throw a MimeException
> in this case. Instead it returns a DateTimeField instance that
> represents the invalid date. This DateTimeField also stores a
> ParseException. I think the idea behind this is that it should still
> be possible to parse a message even if some header fields are
> incorrent.
>
> In this spirit, should Field.parse() throw an exception at all? Or
> would it be better to return a Field instance that represents an
> invalid header field instead?
>
> What do you think?
>
> Markus
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org