Small bug in org.jboss.mail.mailbox.MessageData

[sappenin]

I found a small bug in org.jboss.mail.mailbox.MessageData.semiColize().

Basically, this function takes a MailAddress array, and is supposed to return a string containing the addresses. The function appears to work properly when an array of length 1 is passed in. However, if the array has more than 1 address, the returned value is actually a semi-colon followed by the toString representation of the array class, generally something like:
"; [Lorg.jboss.mail.message.MailAddress;@1ca8d36". This seems to primarily affect the contents of the "toAddress" field in the Messagedata table, only when a msg has more than 1 recipient.

Here is the orig code...

/**
 * take a set of addresses and turn them into a semi colon seperated string
 *
 * @param address
 * @return string containing the addresses
 */
 private String semiColize(MailAddress[] address) {
 String retval = "";
 for (int i = 0; i < address.length; i++) {
 retval = retval.length() == 0 ? address.toString() : ("; " + address.toString());
 }
 return retval;
 }

And a suggested fix:

 /**
 * take a set of addresses and turn them into a semi colon seperated string
 *
 * @param address
 * @return string containing the addresses
 */
 private String semiColize(MailAddress[] address) {
 String retval = "";
 for (int i = 0; i < address.length; i++) {

//Append into retval instead of assignment

 retval += retval.length() == 0 ? address.toString(): ("; " + address.toString()); //Read the value of the i'th address, instead of the array itself.
 }
 return retval;
 }

[sappenin]

Hmmm....the forum is stripping out my array brackets.

The fix should look like this:

 /**
 * take a set of addresses and turn them into a semi colon seperated string
 *
 * @param address
 * @return string containing the addresses
 */
 private String semiColize(MailAddress[] address) {
 String retval = "";
 for (int i = 0; i < address.length; i++) {
 retval += retval.length() == 0 ? address{i}.toString() : ("; " + address{i}.toString());
}
 return retval;
 }

[sappenin]

Thanks for adding the chage in CVS. Although, it looks like only half of the changes made it in. In its current state, the "for" loop is going to replace the value of retval with a new string on every iteration.

Instead, each new address from the address[] array should be appended, so that all addresses are returned by the function.

Current CVS code:

//Bug: 'retval' will end up with only the last address
// since an assignment operation is being performed
// instead of an append
retval = retval.length() == 0 ? address{i}.toString()
 : ("; " + address{i}.toString());

Suggested Fix:

//Append into 'retval' using '+=' instead of just '='
//or perhaps by using a StringBuffer(?) for better efficiency
retval += retval.length() == 0 ? address{i}.toString()
 : ("; " + address{i}.toString());

[acoliver]

doh.. . I'm an idiot. Thanks for checking me :-)

[f00bar]

I had to de-type the flags field in MessageData in order to get it to compile. I just did a cvs upd to pull the source tree. Is it possible that you didn't commit a file called Flag.java in the same namespace as MessageData, or am I missing something?

BTW, is there a better way to report stuff like this than post to the BB?

[acoliver]

jira. Yes...if I failed to commit flag to head then that is my bad. I'll check.