DO NOT REPLY [Bug 17161] - LocalSender removes attachments from request message

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17161>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17161

LocalSender removes attachments from request message

Eric.D.Friedman@wellsfargo.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |



------- Additional Comments From Eric.D.Friedman@wellsfargo.com  2003-03-19 19:52 -------
LocalSender throws a NullPointerException because of this patch.

The reason is that this code:

serverRequest.getAttachmentsImpl().
        setAttachmentParts(clientRequest.getAttachmentsImpl().
                           getAttachments());

does not check whether the return value of getAttachmentsImpl() on either
serverRequest or clientRequest is null, which it can be if attachment support is
not enabled.

Two possible fixes:

1. just check for null in both cases and do not copy attachments if either value
is null;

2. add a method to Message: 

public boolean isAttachmentSupportEnabled() {
  return mAttachments != null;
}

This has the advantage of hiding the fact that a null value on message's member
variable has more semantic meaning than one might otherwise guess.

It also raises a question:  what to do when the client message has attachments
and the server doesn't support them:  raise an exception?  silently drop them?

In short, the minimum fix is this:

Attachments serverAttachments = serverRequest.getAttachmentsImpl();
Attachments clientAttachments = clientRequest.getAttachmentsImpl();

if (null != clientAttachments && null != serverAttachments) {
  serverAttachments.setAttachmentParts(clientAttachments.getAttachments());
}

A better fix would be this:

[ in Message.java ]

public boolean isAttachmentSupportEnabled() {
  return null != mAttachments;
}

and in LocalSender:

if (clientRequest.isAttachmentsSupportEnabled()
    && serverRequest.isAttachmentsSupportEnabled()) {
  serverRequest.getAttachmentsImpl().
        setAttachmentParts(clientRequest.getAttachmentsImpl().
                           getAttachments());
}

the above portion might change pending the decision about what to happen when
there's a mismatch between the client/server support for attachments -- probably
not a big deal for LocalSender, but definitely a concern elsewhere.