Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Zero-length string parameters passed to Java as null pointers
06-09-2011, 08:46 PM
Post: #1
Zero-length string parameters passed to Java as null pointers
If a service receives a zero-length string as one of its parameters, ohNet passes this to a Java provider as a null pointer instead of a zero-length string object. This seems wrong to me. Also, if a proxy tries to make a service invocation using a null pointer, this causes a runtime crash.

I have fixed the first of these problems by changing this code in DviSessionUpnp::InvocationReadString
Code:
Brn value = XmlParserBasic::Find(aName, iSoapRequest);
if (value.Bytes()) {
    Bwh writable(value.Bytes()+1);
    writable.Append(value);
    Converter::FromXmlEscaped(writable);
    writable.PtrZ();
    writable.TransferTo(aString);
}
to the following:
Code:
Brn value = XmlParserBasic::Find(aName, iSoapRequest);
Bwh writable(value.Bytes()+1);
if (value.Bytes()) {
    writable.Append(value);
    Converter::FromXmlEscaped(writable);
}
writable.PtrZ();
writable.TransferTo(aString);
The second problem is caused by this code in Java_org_openhome_net_controlpoint_ArgumentString_ActionArgumentCreateStringInpu​t:
Code:
ServiceParameter param = (ServiceParameter) (size_t)aParameter;
ActionArgument arg;
const char* value = (*aEnv)->GetStringUTFChars(aEnv, aValue, NULL);
If a null string is passed, aValue will be a null pointer, which causes GetStringUTFChars to crash. Either this code or the calling code in ActionArgument.java should check for a null pointer and throw a Java NullPointerException.
Find all posts by this user
07-09-2011, 10:11 AM
Post: #2
RE: Zero-length string parameters passed to Java as null pointers
Thank you for reporting this error and providing the appropriate fixes.

Your changes to DviSessionUpnp::InvocationReadString have been committed to the internal repository, as has some parameter checking for null values in the ParameterString class.

Just to be picky, I thought IllegalArgumentException would be more appropriate in this case than a NullPointerException, so IllegalArgumentException is thrown instead.
Find all posts by this user
07-09-2011, 02:04 PM
Post: #3
RE: Zero-length string parameters passed to Java as null pointers
(07-09-2011 10:11 AM)greggh Wrote:  Thank you for reporting this error and providing the appropriate fixes.

Your changes to DviSessionUpnp::InvocationReadString have been committed to the internal repository, as has some parameter checking for null values in the ParameterString class.

Thanks! I presume you meant ArgumentString, not ParameterString.

(07-09-2011 10:11 AM)greggh Wrote:  Just to be picky, I thought IllegalArgumentException would be more appropriate in this case than a NullPointerException, so IllegalArgumentException is thrown instead.

I can see the logic for doing this. However, the JDK methods seem to throw NullPointerException in this case. Here are 10 examples chosen at random:

java.lang.Class.getAnnotation()
java.lang.Class.getField()
java.lang.Class.getMethod()
java.lang.Class.getResourceAsStream()
java.util.ArrayList.addAll()
java.util.ArrayList.toArray()
java.util.Vector.containsAll()
java.util.Vector.copyInto()
java.util.Vector.removeAll()
java.util.Vector.retainAll()

See also this comment from the javadoc for java.lang.String:

Unless otherwise noted, passing a null argument to a constructor or method in this class will cause a NullPointerException to be thrown.

I think it would be better to follow the example of the JDK.
Find all posts by this user
07-09-2011, 03:22 PM
Post: #4
RE: Zero-length string parameters passed to Java as null pointers
(07-09-2011 02:04 PM)simoncn Wrote:  
(07-09-2011 10:11 AM)greggh Wrote:  Thank you for reporting this error and providing the appropriate fixes.

Your changes to DviSessionUpnp::InvocationReadString have been committed to the internal repository, as has some parameter checking for null values in the ParameterString class.

Thanks! I presume you meant ArgumentString, not ParameterString.

Sorry; you are correct. I did mean ArgumentString!

(07-09-2011 02:04 PM)simoncn Wrote:  
(07-09-2011 10:11 AM)greggh Wrote:  Just to be picky, I thought IllegalArgumentException would be more appropriate in this case than a NullPointerException, so IllegalArgumentException is thrown instead.

I can see the logic for doing this. However, the JDK methods seem to throw NullPointerException in this case. Here are 10 examples chosen at random:

java.lang.Class.getAnnotation()
java.lang.Class.getField()
java.lang.Class.getMethod()
java.lang.Class.getResourceAsStream()
java.util.ArrayList.addAll()
java.util.ArrayList.toArray()
java.util.Vector.containsAll()
java.util.Vector.copyInto()
java.util.Vector.removeAll()
java.util.Vector.retainAll()

See also this comment from the javadoc for java.lang.String:

Unless otherwise noted, passing a null argument to a constructor or method in this class will cause a NullPointerException to be thrown.

I think it would be better to follow the example of the JDK.

I was torn between throwing a NullPointerException and an IllegalArgumentException (and I know wars can be triggered over which is appropriate to use in this scenario).

In my own view on this topic, I see NullPointerException as something typically thrown by the JVM itself when an attempt is made to operate upon a null value. On the other hand, I see IllegalArgumentException as something more specific, showing an attempt has been made to validate parameters, and giving a slightly clearer indication where the source of the exception lies.

Of course, there are then counter-arguments and opposing opinions to this, such as eliminating any need to choose between the two exception types by creating a NullArgumentException class.

Regardless of opinion, I think consistency is the most important factor in these situations. As we haven't yet widely adopted any other great convention for the handling of null arguments, I think I will have to defer and follow the practice of the majority of the JDK.
Find all posts by this user
07-09-2011, 04:33 PM
Post: #5
RE: Zero-length string parameters passed to Java as null pointers
(07-09-2011 03:22 PM)greggh Wrote:  I was torn between throwing a NullPointerException and an IllegalArgumentException (and I know wars can be triggered over which is appropriate to use in this scenario).

In my own view on this topic, I see NullPointerException as something typically thrown by the JVM itself when an attempt is made to operate upon a null value. On the other hand, I see IllegalArgumentException as something more specific, showing an attempt has been made to validate parameters, and giving a slightly clearer indication where the source of the exception lies.

Of course, there are then counter-arguments and opposing opinions to this, such as eliminating any need to choose between the two exception types by creating a NullArgumentException class.

Regardless of opinion, I think consistency is the most important factor in these situations. As we haven't yet widely adopted any other great convention for the handling of null arguments, I think I will have to defer and follow the practice of the majority of the JDK.

I'd come down slightly on the side of NullPointerException even without the JDK precedent. My rationale is that IllegalArgumentException just tells me that something was wrong with the argument, whereas NullPointerException tells me specifically that the problem was a null value. I accept that this could be argued either way, and in cases like this I agree with you that consistency is a good principle to follow.
Find all posts by this user


Forum Jump: