Fix for internalGetAttachments, ExtendedProperty, MapiTypeConverterMapEntry #80
Conversation
In ExchangeService, method internalGetAttachments:
List propsArray = new ArrayList();
for (PropertyDefinitionBase propertyDefinitionBase :
additionalProperties)
If addtionalProperties is null (and it can be), this throws a
NullPointerException.
A check for null is done here:
if (additionalProperties != null) {
request.getAdditionalProperties().addAll(propsArray);
}
This check should be done earlier.
And
propsArray.add(additionalProperties);
This will result in adding a list to another list and later on result in
a ClassCastException.
It should be:
for (PropertyDefinitionBase propertyDefinitionBase :
additionalProperties)
{
propsArray.add(propertyDefinitionBase);
}
There is a loop through an ArrayList written as: for (int index = 0; index <= array.size(); index++) It should be written as: for (int index = 0; index < array.size(); index++)
|
ExtendedProperty incorrect for loop check There is a loop through an ArrayList written as: for (int index = 0; index <= array.size(); index++) It should be written as: |
The method validateValueAsArray is completely wrong. For starters, arrays are not instances of the class Array. Secondly, extended properties of “arrays” have been implemented by the EWS API using ArrayList which is not an array, its a list.
|
In MapiTypeConverterMapEntry |
avromf
changed the title
|
Please limit pull requests to one issue/enhancement. The others would be better suited to separate pull requests. It's also encouraged to file an issue prior to a fix. Good catch on these bugs, though. |
|
+1 for separate pull request per issue. I haven't reviewed this yet, but will review the new pull requests. Feel free to close this one. |
|
Also please refer to the an open issue associated with each of the bug fixes. If there is no open issue, then please add new ones. |
There was a problem hiding this comment.
Use addAll() and Arrays.asList() instead of manually looping through and populating each element.
There was a problem hiding this comment.
Those methods cannot be used with Iterable<> as far as I know. The loop is fine, but add the properties from additionalProperties directly to the request instead of using an intermediary list which winds up populating the request anyway.
|
@avromf Is it a safe assumption that no one was calling setExtendedProperties with an "array" instead of "ArrayList"? I wonder if this scenario was completely broken or clients were able to work around it and we are not going to break them. |
There was a problem hiding this comment.
Should we check the type for all the elements?
|
@vboctor The API seems to handle them as ArrayLists when reading/writing extended properties. Any attempt to save an extended property containing an array would have thrown an exception when invoking ExtendedProperty.writeElementsToXml(). |
|
@vboctor @Noblebrown Apologies for the multiple commits. I'm inexperienced with using github. |
|
@vboctor In my case, I was using an extended property definition that was using MapiPropertyType.StringArray as its type. This was implemented internally as an ArrayList. The confusion, I think, comes from the difference in the terminology from MAPI which calls it an array and how it was implemented (a list). (I would have been happier if it used the List interface instead, but I digress.) As @Noblebrown mentioned, Array contains static methods only and you would never have a case where the object was an instance of Array. So, this never worked before. |
|
@avromf are you going to split this into multiple pull requests so we can move forward with reviewing the changes separately? |
|
@vboctor Sure, will do when I get the chance. |
|
We are also having a problem that gives a NullPointerException in our usage of the Java EWS API in some client environments. We are currently using version 1.1.5 but have tried 1.2 as well. I was hoping someone could shed some light for us on how to further investigate the underlying issue. Our product is a web application written in CFML (on top of Java) which is leveraging this Java API. I can step through our code and it looks like a connection to the EWS is being made, however when we make a call on a mailbox to "FindAttachments" it throws this exception. Is there a Java Client readily available that I can test server connection credentials with and test API method calls? I'm hoping to get to the source line in the API that is causing the issue to understand what is triggering it. |
|
Closing this pull request and resubmitting as separate requests per issue... |
4 participants
In ExchangeService, method internalGetAttachments:
List propsArray = new ArrayList();
for (PropertyDefinitionBase propertyDefinitionBase :
additionalProperties)
If addtionalProperties is null (and it can be), this throws a
NullPointerException.
A check for null is done here:
if (additionalProperties != null) {
request.getAdditionalProperties().addAll(propsArray);
}
This check should be done earlier.
And
propsArray.add(additionalProperties);
This will result in adding a list to another list and later on result in
a ClassCastException.
It should be:
for (PropertyDefinitionBase propertyDefinitionBase :
additionalProperties)
{
propsArray.add(propertyDefinitionBase);
}