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 Robert Burrell Donkin <ro...@gmail.com> on 2009/08/08 11:28:26 UTC
[mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>>
>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>> equals/hashcode
>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>> bugs,
>>>>> but it would be much better to fix it (without breaking james, of
>>>>> course)...
>>>>>
>>>>>
>>>>
>>>> do you have an example of the breakage?
>>>>
>>>>
>>>
>>> From the javadoc I added:
>>>
>>> * Note that this implementation breaks the general contract of the
>>> * equals method as well as its relationship with the hashcode method,
>>> * since it can return true when compared to a String instance
>>> * (braking the symmetry property, and allowing equal objects to have
>>> * different hash codes).
>>>
>>> in other words, it can return true when compared to a String, but a
>>> String
>>> will always return false when compared to a non-String, and this breaks
>>> the
>>> contract in two different ways.
>>>
>>
>> i agree about the symmetry but IIRC when i analysed the hash code it
>> was ok(ish). given that the whole idea breaks a basic contract in
>> java, this is probably just splitting hairs. i strongly dislike this
>> design and would prefer a separate method to allow strings to be
>> explicitly checked.
>>
>> in general, MailAddress has issues, and should have been modelled as
>> an interface
>
> I still think hashCode is also broken - a MailAddress("me@here") will be
> equal to the String "ME@HERE" but they will have different hash codes.
agreed
i'm in favour of altering the equals behaviour on MailAddress but have
no idea about the possible impact
opinions?
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API documentation]
Posted by "A. Rothman" <am...@amichais.net>.
FYI, here's a summary (AIUI) to clarify:
- RFCs 821/822 have been obsoleted by RFCs 2821/2822 which in turn have
been obsoleted by RFCs 5321/5322 - might as well aim for the current
spec, no?
- MailAddress is used for SMTP envelope addresses (MAIL FROM/RCPT TO or
forward-path/reverse-path), defined in RFC xx21, whereas the xx22 RFCs
define addresses as they appear in mail headers, with somewhat different
syntax.
- JavaMail's InternetAddress class is an RFC 822 address implementation.
- The current Mailet API's MailAddress is an attempted RFC 821 address
implementation, with a contract-breaking equals method, a few
non-RFC-821 compliance bugs, and not too efficient code. I uploaded a
patch to test and fix some of the bugs, for the time being, in
https://issues.apache.org/jira/browse/MAILET-28. We are currently
discussing fixing the equals bug too.
- The 2.3.2 codebase contains 2 usages of the broken equals method as
far as I can tell, which appear to be broken regardless of the suggested
change. They can/should be fixed independently, and pave the way for
changing the equals method itself. Dunno about 3.0/trunk/external usages
- if the same usages are carried over or there are additional ones, they
should be fixed as well. A simple "fing usages" in Eclipse or other IDE
can help find these in a couple of minutes (I'm only working with 2.3.2
for now).
- I'm planning on implementing an RFC 5321-compliant address parser,
which I will likely need for one of my projects anyway. If it is agreed
that it's the right move and will be accepted, I can use it to replace
the existing MailAddress implementation.
sorry if I'm too elaborate :-)
Amichai
Stefano Bagnara wrote:
> I've not read the whole thread and evaluations, but I just want to
> point you to a bookmark of mine:
> http://www.boxbe.com/freebox.html
>
> ---
> EmailAddress.java
>
> The only more-or-less-2822-compliant Java-based email address
> extracter/verifier with some header verification as well. Reasonably,
> Javamail's InternetAddress class does not have a very robust parser,
> and often "fails" silently (technically not a failure, but not
> suitable for those that need to extract addresses accurately and
> predictably). This class can reliably extract the pertinent
> information from any well-formed address, as well as perform several
> other useful functions relating to addresses and email headers.
> ---
>
> I know it is based on regex, so I'm not sure how it performs, but it
> is a single well commented class under the apache license.
>
> Stefano
>
> 2009/9/16 Robert Burrell Donkin <ro...@gmail.com>:
>
>> On Tue, Sep 15, 2009 at 8:20 PM, Markus Wiederkehr
>> <ma...@gmail.com> wrote:
>>
>>> On Sat, Aug 8, 2009 at 7:50 PM, Norman Maurer <no...@apache.org> wrote:
>>>
>>>> Well, I think we should alter it. I never understood why james support
>>>> case sensitive email addresses at all..
>>>>
>>> AIUI the local-part of a mailbox is case sensitive whereas the domain
>>> is not (see RFC 5321 section 2.4.).
>>>
>>> This is how Mime4j's class Mailbox implements hashCode and equals, anyway.
>>>
>> yes (i remember now :-)
>>
>> IIRC the Mailbox version in Mime4J is the preferred solution so we
>> should probably use that
>>
>>
>>> In any case hashCode and equals have to be consistent with each other of course.
>>>
>> +1
>>
>> - robert
>>
>> ---------------------------------------------------------------------
>> 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: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Stefano Bagnara <ap...@bago.org>.
I've not read the whole thread and evaluations, but I just want to
point you to a bookmark of mine:
http://www.boxbe.com/freebox.html
---
EmailAddress.java
The only more-or-less-2822-compliant Java-based email address
extracter/verifier with some header verification as well. Reasonably,
Javamail's InternetAddress class does not have a very robust parser,
and often "fails" silently (technically not a failure, but not
suitable for those that need to extract addresses accurately and
predictably). This class can reliably extract the pertinent
information from any well-formed address, as well as perform several
other useful functions relating to addresses and email headers.
---
I know it is based on regex, so I'm not sure how it performs, but it
is a single well commented class under the apache license.
Stefano
2009/9/16 Robert Burrell Donkin <ro...@gmail.com>:
> On Tue, Sep 15, 2009 at 8:20 PM, Markus Wiederkehr
> <ma...@gmail.com> wrote:
>> On Sat, Aug 8, 2009 at 7:50 PM, Norman Maurer <no...@apache.org> wrote:
>>> Well, I think we should alter it. I never understood why james support
>>> case sensitive email addresses at all..
>>
>> AIUI the local-part of a mailbox is case sensitive whereas the domain
>> is not (see RFC 5321 section 2.4.).
>>
>> This is how Mime4j's class Mailbox implements hashCode and equals, anyway.
>
> yes (i remember now :-)
>
> IIRC the Mailbox version in Mime4J is the preferred solution so we
> should probably use that
>
>> In any case hashCode and equals have to be consistent with each other of course.
>
> +1
>
> - robert
>
> ---------------------------------------------------------------------
> 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: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Robert Burrell Donkin <ro...@gmail.com>.
On Tue, Sep 15, 2009 at 8:20 PM, Markus Wiederkehr
<ma...@gmail.com> wrote:
> On Sat, Aug 8, 2009 at 7:50 PM, Norman Maurer <no...@apache.org> wrote:
>> Well, I think we should alter it. I never understood why james support
>> case sensitive email addresses at all..
>
> AIUI the local-part of a mailbox is case sensitive whereas the domain
> is not (see RFC 5321 section 2.4.).
>
> This is how Mime4j's class Mailbox implements hashCode and equals, anyway.
yes (i remember now :-)
IIRC the Mailbox version in Mime4J is the preferred solution so we
should probably use that
> In any case hashCode and equals have to be consistent with each other of course.
+1
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Markus Wiederkehr <ma...@gmail.com>.
On Sat, Aug 8, 2009 at 7:50 PM, Norman Maurer <no...@apache.org> wrote:
> Well, I think we should alter it. I never understood why james support
> case sensitive email addresses at all..
AIUI the local-part of a mailbox is case sensitive whereas the domain
is not (see RFC 5321 section 2.4.).
This is how Mime4j's class Mailbox implements hashCode and equals, anyway.
In any case hashCode and equals have to be consistent with each other of course.
Bye,
Markus
>
> Bye,
> Norma
>
> 2009/8/8 Robert Burrell Donkin <ro...@gmail.com>:
>> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>>>>
>>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>>> equals/hashcode
>>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>>> bugs,
>>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>>> course)...
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> do you have an example of the breakage?
>>>>>>
>>>>>>
>>>>>
>>>>> From the javadoc I added:
>>>>>
>>>>> * Note that this implementation breaks the general contract of the
>>>>> * equals method as well as its relationship with the hashcode method,
>>>>> * since it can return true when compared to a String instance
>>>>> * (braking the symmetry property, and allowing equal objects to have
>>>>> * different hash codes).
>>>>>
>>>>> in other words, it can return true when compared to a String, but a
>>>>> String
>>>>> will always return false when compared to a non-String, and this breaks
>>>>> the
>>>>> contract in two different ways.
>>>>>
>>>>
>>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>>> was ok(ish). given that the whole idea breaks a basic contract in
>>>> java, this is probably just splitting hairs. i strongly dislike this
>>>> design and would prefer a separate method to allow strings to be
>>>> explicitly checked.
>>>>
>>>> in general, MailAddress has issues, and should have been modelled as
>>>> an interface
>>>
>>> I still think hashCode is also broken - a MailAddress("me@here") will be
>>> equal to the String "ME@HERE" but they will have different hash codes.
>>
>> agreed
>>
>> i'm in favour of altering the equals behaviour on MailAddress but have
>> no idea about the possible impact
>>
>> opinions?
>>
>> - robert
>>
>> ---------------------------------------------------------------------
>> 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
>
>
--
Always remember you're unique. Just like everyone else.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Norman Maurer <no...@apache.org>.
Well, I think we should alter it. I never understood why james support
case sensitive email addresses at all..
Bye,
Norma
2009/8/8 Robert Burrell Donkin <ro...@gmail.com>:
> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>>>
>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>> equals/hashcode
>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>> bugs,
>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>> course)...
>>>>>>
>>>>>>
>>>>>
>>>>> do you have an example of the breakage?
>>>>>
>>>>>
>>>>
>>>> From the javadoc I added:
>>>>
>>>> * Note that this implementation breaks the general contract of the
>>>> * equals method as well as its relationship with the hashcode method,
>>>> * since it can return true when compared to a String instance
>>>> * (braking the symmetry property, and allowing equal objects to have
>>>> * different hash codes).
>>>>
>>>> in other words, it can return true when compared to a String, but a
>>>> String
>>>> will always return false when compared to a non-String, and this breaks
>>>> the
>>>> contract in two different ways.
>>>>
>>>
>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>> was ok(ish). given that the whole idea breaks a basic contract in
>>> java, this is probably just splitting hairs. i strongly dislike this
>>> design and would prefer a separate method to allow strings to be
>>> explicitly checked.
>>>
>>> in general, MailAddress has issues, and should have been modelled as
>>> an interface
>>
>> I still think hashCode is also broken - a MailAddress("me@here") will be
>> equal to the String "ME@HERE" but they will have different hash codes.
>
> agreed
>
> i'm in favour of altering the equals behaviour on MailAddress but have
> no idea about the possible impact
>
> opinions?
>
> - robert
>
> ---------------------------------------------------------------------
> 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: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Norman Maurer <no...@apache.org>.
go ahead and fix it..
Bye,
Norman
2009/9/15 Robert Burrell Donkin <ro...@gmail.com>:
> On Sun, Sep 6, 2009 at 1:57 PM, A. Rothman <am...@amichais.net> wrote:
>>
>>
>> Robert Burrell Donkin wrote:
>>
>>> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>>>
>>>>>>>>
>>>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>>>> equals/hashcode
>>>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>>>> bugs,
>>>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>>>> course)...
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> do you have an example of the breakage?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> From the javadoc I added:
>>>>>>
>>>>>> * Note that this implementation breaks the general contract of the
>>>>>> * equals method as well as its relationship with the hashcode method,
>>>>>> * since it can return true when compared to a String instance
>>>>>> * (braking the symmetry property, and allowing equal objects to have
>>>>>> * different hash codes).
>>>>>>
>>>>>> in other words, it can return true when compared to a String, but a
>>>>>> String
>>>>>> will always return false when compared to a non-String, and this breaks
>>>>>> the
>>>>>> contract in two different ways.
>>>>>>
>>>>>>
>>>>>
>>>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>>>> was ok(ish). given that the whole idea breaks a basic contract in
>>>>> java, this is probably just splitting hairs. i strongly dislike this
>>>>> design and would prefer a separate method to allow strings to be
>>>>> explicitly checked.
>>>>>
>>>>> in general, MailAddress has issues, and should have been modelled as
>>>>> an interface
>>>>>
>>>>
>>>> I still think hashCode is also broken - a MailAddress("me@here") will be
>>>> equal to the String "ME@HERE" but they will have different hash codes.
>>>>
>>>
>>> agreed
>>>
>>> i'm in favour of altering the equals behaviour on MailAddress but have
>>> no idea about the possible impact
>>>
>>> opinions?
>>>
>>> - robert
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>
>>>
>>>
>>
>> I'm in favor too. I've found 2 usages relying on the broken behavior (looked
>> in JAMES 2.3.2 only): CommandListservProcessor.checkBeenThere() and
>> GenericListserv.service(). Both compare a MailAddress to a message header.
>> Actually, on closer inspection, It looks like they're broken anyway, since
>> they actually get the header as a String[] which means the equals method
>> always returns false... did this ever work? is it needed?
>>
>> Anyone have other usages or opinions?
>
> any objections?
>
> - robert
>
> ---------------------------------------------------------------------
> 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: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Robert Burrell Donkin <ro...@gmail.com>.
On Sun, Sep 6, 2009 at 1:57 PM, A. Rothman <am...@amichais.net> wrote:
>
>
> Robert Burrell Donkin wrote:
>
>> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>>
>>>>>>>
>>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>>> equals/hashcode
>>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>>> bugs,
>>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>>> course)...
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> do you have an example of the breakage?
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> From the javadoc I added:
>>>>>
>>>>> * Note that this implementation breaks the general contract of the
>>>>> * equals method as well as its relationship with the hashcode method,
>>>>> * since it can return true when compared to a String instance
>>>>> * (braking the symmetry property, and allowing equal objects to have
>>>>> * different hash codes).
>>>>>
>>>>> in other words, it can return true when compared to a String, but a
>>>>> String
>>>>> will always return false when compared to a non-String, and this breaks
>>>>> the
>>>>> contract in two different ways.
>>>>>
>>>>>
>>>>
>>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>>> was ok(ish). given that the whole idea breaks a basic contract in
>>>> java, this is probably just splitting hairs. i strongly dislike this
>>>> design and would prefer a separate method to allow strings to be
>>>> explicitly checked.
>>>>
>>>> in general, MailAddress has issues, and should have been modelled as
>>>> an interface
>>>>
>>>
>>> I still think hashCode is also broken - a MailAddress("me@here") will be
>>> equal to the String "ME@HERE" but they will have different hash codes.
>>>
>>
>> agreed
>>
>> i'm in favour of altering the equals behaviour on MailAddress but have
>> no idea about the possible impact
>>
>> opinions?
>>
>> - robert
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>>
>>
>
> I'm in favor too. I've found 2 usages relying on the broken behavior (looked
> in JAMES 2.3.2 only): CommandListservProcessor.checkBeenThere() and
> GenericListserv.service(). Both compare a MailAddress to a message header.
> Actually, on closer inspection, It looks like they're broken anyway, since
> they actually get the header as a String[] which means the equals method
> always returns false... did this ever work? is it needed?
>
> Anyone have other usages or opinions?
any objections?
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API documentation]
Posted by "A. Rothman" <am...@amichais.net>.
Robert Burrell Donkin wrote:
> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>
>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>> equals/hashcode
>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>> bugs,
>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>> course)...
>>>>>>
>>>>>>
>>>>>>
>>>>> do you have an example of the breakage?
>>>>>
>>>>>
>>>>>
>>>> From the javadoc I added:
>>>>
>>>> * Note that this implementation breaks the general contract of the
>>>> * equals method as well as its relationship with the hashcode method,
>>>> * since it can return true when compared to a String instance
>>>> * (braking the symmetry property, and allowing equal objects to have
>>>> * different hash codes).
>>>>
>>>> in other words, it can return true when compared to a String, but a
>>>> String
>>>> will always return false when compared to a non-String, and this breaks
>>>> the
>>>> contract in two different ways.
>>>>
>>>>
>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>> was ok(ish). given that the whole idea breaks a basic contract in
>>> java, this is probably just splitting hairs. i strongly dislike this
>>> design and would prefer a separate method to allow strings to be
>>> explicitly checked.
>>>
>>> in general, MailAddress has issues, and should have been modelled as
>>> an interface
>>>
>> I still think hashCode is also broken - a MailAddress("me@here") will be
>> equal to the String "ME@HERE" but they will have different hash codes.
>>
>
> agreed
>
> i'm in favour of altering the equals behaviour on MailAddress but have
> no idea about the possible impact
>
> opinions?
>
> - robert
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
>
I'm in favor too. I've found 2 usages relying on the broken behavior
(looked in JAMES 2.3.2 only): CommandListservProcessor.checkBeenThere()
and GenericListserv.service(). Both compare a MailAddress to a message
header. Actually, on closer inspection, It looks like they're broken
anyway, since they actually get the header as a String[] which means the
equals method always returns false... did this ever work? is it needed?
Anyone have other usages or opinions?
-Amichai