CERT
Skip to end of metadata
Go to start of metadata

The Java compiler type checks the arguments to each varargs method to ensure that the arguments are of the same type or object reference. However, the compile-time checking is ineffective when Object or generic T parameter types are used [Bloch 2008]. Another requirement for providing strong compile-time type checking of variable argument methods is to be as specific as possible when declaring the type of the method parameter.

Noncompliant Code Example (Object)

This noncompliant code example declares a vararg method using Object. It accepts an arbitrary mix of parameters of any object type. Legitimate uses of such declarations are rare. (See exception below).

Noncompliant Code Example (Generic Type)

This noncompliant code example declares a vararg method using a generic type parameter. It accepts a variable number of parameters that are all of the same object type. Again, legitimate uses of such declarations are rare.

Compliant Solution

Be as specific as possible when declaring parameter types; avoid Object and imprecise generic types in varargs.

Retrofitting old methods containing final array parameters with generically-typed varargs is not always a good idea. For example, given a method that does not accept an argument of a particular type, it could be possible to override the compile-time checking — through the use of generic varargs parameters — so that the method would compile cleanly rather than correctly, causing a compile-time error [Bloch 2008].

Also, note that autoboxing does not allow strong compile-time type checking of primitive types and their corresponding wrapper classes.

Exceptions

DCL09-EX1: Varargs signatures using Object and imprecise generic types are acceptable when the body of the method does not use casts or auto-boxing and compiles without error. Consider the following example, which operates correctly for all object types and type checks successfully.

Risk Assessment

Unmindful use of the varargs feature prevents strong compile-time type checking, creates ambiguity, and diminishes code readability.

Guideline

Severity

Likelihood

Remediation Cost

Priority

Level

DCL09-J

low

unlikely

medium

P2

L3

Automated Detection

Automated detection appears to be straightforward.

Bibliography

[Bloch 2008]

Item 42: "Use Varargs Judiciously"

[Steinberg 2005]

"Using the Varargs Language Feature"

[Sun 2006]

varargs


      01. Declarations and Initialization (DCL)      

14 Comments

  1. Why is the second NCCE (the int autoboxing NCCE example) bad?

    1. Consider doSomething() removes elements from a list that holds objects of type Integer. The elements are removed depending on their index (that is, doSomething(2) will remove the third element) or by element type (that is, doSomething((Integer)1) will remove the element with value 1). When instead of int, the method parameter is declared as Integer, both forms are allowed because of autoboxing. This is not always desirable (in reality, the List interface became troublesome when it was generified). That said, I don't think this is limited to varargs methods only. IMO the preferable way is:

      as opposed to:

      In general, reference types do not go very well with primitive types. When doSomething((Integer)1) is called, then only the particular NCE is valid; not when doSomething(1) is called.

      1. I've been working some more with the autoboxing example, and it looks like the CE may not work as expected. The following code compiles without error:

        The output is:

        It looks like Java is auto-unboxing the '((Integer) 1)' and then passing '((int) 1)' to test(). This means that even when you declare test as 'test(int... i)', test() still takes either Integer or int arguments, the same as if test is declared as 'test(Integer... i)'. The only difference is in whether autoboxing or auto-unboxing is applied to int or Integer type arguments.

        Interestingly, the Java compiler rejected your example with polymorphic test() methods when I made both test() methods take varargs rather than a single argument. It looks like polymorphic methods/functions with varargs whose different types can be autoboxed/auto-unboxed to eachother are not allowed.

        1. Hm. The example as it stands now does not sound too convincing. Here's another attempt -

          If only one doSomething() exists, and performs only one function (either removes elements from a List by index <when a primitive parameter is used> or by value <when a boxed primitive parameter is used>), two cases arise:

          1. When the user wants to remove an element by index and consequently uses doSomething(1) and the parameter type is int and not Integer: If someone uses doSomething((Integer)1) erroneously, a tool may warn the user that the code might not behave the expected way (eg. it is designed to remove elements by index as opposed to by value).

          2. When the user wants to remove an element by value and consequently uses doSomething((Integer)1) and the parameter type is Integer and not int: If someone uses doSomething(1) erroneously, a tool may warn the user that the code might not behave the expected way (eg. it is designed to remove elements by value as opposed to by index).

          As you note, the potential for confusion is eliminated in vararg methods because of the compiler error. This is only when we have two doSomething() implementations. If we have just one, the above applies. In summary, we are warning the user about auto(un)boxing so that it does not override his thinking. It is also apparent that a user who does this may not have been referring to the documentation for doSomething().

        2. In Kirk's final paragraph, I think you mean overloaded rather than polymorphic, but the point remains.

          1. http://java.sun.com/j2se/1.5.0/docs/guide/language/varargs.html says: "Generally speaking, you should not overload a varargs method, or it will be difficult for programmers to figure out which overloading gets called." This seems sensible.

            1. Precisely. DCL08-J. Do not overload variable argument methods.
              To my last comment, I'd just add: the present NCE violates case 2. I should have avoided bringing in overloaded vararg methods into this discussion.

              1. My main point is that because of auto-unboxing, the autoboxing CE still does not enforce strong compile-time type checking. doSomething(int... x) can still be called with primitive int or Integer object arguments, just as the NCCE doSomething(Integer... x) can be called with primitive int or Integer object arguments. This autoboxing/auto-unboxing behavior happens with any functions/methods that have a vararg argument of a primitive type with a corresponding wrapper class (like int, char, or byte) or a vararg argument whose type is one of the wrapper classes for a primitive type (like Integer, Character, or Byte).

                For example:

                Prints:

                Basically, due to auto-unboxing you cannot write a vararg function/method that is only accepts primitive types as arguments. Would it be worth it to specifically point out this autoboxing/auto-unboxing behavior in this recommendation?

                1. You're right that the NCE/CS do not enforce strong compile time type checking, which is the intent of this recommendation. There is also no real CS for the conditions I stated in my comment because auto (un)boxing is a language feature designed to eliminate the need of incessant casting, whereas a CS would suggest using casts. We are dealing with a problem for which there is no CS. Such discussions are usually not included. (note that the practice is still questionable because autoboxing may decrease performance when used in a loop and so on, but this is not a direct security vuln). At best, we can use a line that says autoboxing does not allow strong compile time type checking of primitive types and their corresponding wrapper classes and one must refer to the documentation for the method to confirm the semantics.

  2. While a good SE guideline, I don't see how violating this rule yields a security flaw.

    Your NCCE code has two 'invalid' signatures (they should prob each be their own NCCE). I presume that any type checking would be done at runtime. EG if I pass a Foo object in to suspect1, I will get a ClassCastException at runtime rather than a compile error. I'm not sure that that is a security violation (it depends on your security policy, and how you handle exceptions, I guess).

    1. The List<E> interface suffers from this pitfall. Removing elements from a list can silently fail at runtime. Refer to MET01-J. Avoid ambiguous uses of overloading for more details.

      You may or may not get a ClassCastException depending on what you are doing. For example, if you are attempting to remove an element (of type Bar) from a list (that contains Foo elements) and returning a boolean indicating whether the operation succeeded, you're quite doomed. The operation will fail silently. I can elaborate on this in the guideline to put the security aspect into perspective. Does this convince you?

      1. I would be convinced by a NCCE that fails silently (eg it does not do what it promises, yet throws no exception nor gives any indicator (eg returning false) of failure..

  3. I think java.util.Formatter may also qualify as an exception to this rule. Sometimes you have to use Object... as an argument type.

  4. I'm wondering if we should add an example, or modify an existing example, to have additional fixed parameter types to show that it is not only this specific method signature but any use of a generic or Object type in a vararg method?