Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-4487672

(proxy) Proxy constructor should check for null argument

    Details

    • Subcomponent:
    • Resolved In Build:
      b91
    • CPU:
      generic
    • OS:
      generic
    • Verification:
      Verified

      Description



      Name: bsC130419 Date: 08/02/2001


      java version "1.4.0-beta"
      Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.0-beta-b65)
      Java HotSpot(TM) Client VM (build 1.4.0-beta-b65, mixed mode)

      The specification for java.lang.reflect.Proxy.newProxyInstance requires a
      NullPointerException if the InvocationHandler argument is null. However, this
      same restriction does not currently apply to proxy instances constructed by
      reflection. I propose that the Proxy constructor be modified to throw a
      NullPointerException if parameter h is null and isProxyInstance() returns true,
      so that it is impossible to create a proxy instance without a handler, rather
      than the current problem of failing at an undetermined point later on when the
      handler is first used.

      This class shows the difference:
      import java.lang.reflect.*;
      class Foo {
        public static void main(String[] args) {
          Object o = null;
          try {
            o = Proxy.newProxyInstance(Foo.class.getClassLoader(),
                                       new Class[0], null);
          } catch (NullPointerException e) {
            System.out.println("newProxyInstance OK");
          }
          try {
            o = Proxy.getProxyClass(Foo.class.getClassLoader(),
                                    new Class[0]).
                  getConstructor(new Class[] {InvocationHandler.class}).
                  newInstance(new Object[1]);
            System.out.println("Oops, created proxy instance without handler");
          } catch (NullPointerException e) {
            System.out.println("Proxy constructor OK");
          } catch (Exception e) {
            System.out.println("shouldn't get here");
          }
          if (o != null)
            try {
              System.out.println(o);
            } catch (NullPointerException e) {
              System.out.println("Delayed failure is BAD");
              e.printStackTrace();
            }
        }
      }

      Implementation wise, all you need do is change Proxy(InvocationHandler h),
      adding a check:
      if (h == null && isProxyInstance(this)) throw new NullPointerException();
      (Review ID: 129305)
      ======================================================================

        Activity

        Hide
        peterjones Peter Jones added a comment -
        BT2:EVALUATION

        [First, for what it's worth, even though the customer's release field indicates "merlin-beta", this issue dates from 1.3.0 (when the class Proxy was first introduced), so this bug should not be considered something that has been introduced in Merlin.]

        I agree with the submitter in principle; it would be desirable to fix this issue at some point. But it also is a fairly minor issue, only affecting code that would otherwise fail with a NullPointerException anyway, just at a different point (assuming that application code does not catch NullPointerException from a proxy class's constructor as part of normal control flow, which would be poor style). The "workaround" is what correct code should already be doing (never passing null to a proxy class's constructor to begin with)-- the issue is really more about providing a better failure mode for incorrect code.

        A fix would require a very minor spec update-- a clarification in the behavior of a case that correct code should never encounter anyway-- but still a spec change, thus requiring due consideration and process (see Comments for a draft of the required spec addition). It is in all likelihood too late in the Merlin release cycle to consider a spec change like this when the apparent severity of the issue is low (there is a straightforward "workaround", which is what application code should be doing anyway), but the fix seems like a good candidate for consideration in the Tiger release.

        peter.c.jones@east 2001-08-06
        Show
        peterjones Peter Jones added a comment - BT2:EVALUATION [First, for what it's worth, even though the customer's release field indicates "merlin-beta", this issue dates from 1.3.0 (when the class Proxy was first introduced), so this bug should not be considered something that has been introduced in Merlin.] I agree with the submitter in principle; it would be desirable to fix this issue at some point. But it also is a fairly minor issue, only affecting code that would otherwise fail with a NullPointerException anyway, just at a different point (assuming that application code does not catch NullPointerException from a proxy class's constructor as part of normal control flow, which would be poor style). The "workaround" is what correct code should already be doing (never passing null to a proxy class's constructor to begin with)-- the issue is really more about providing a better failure mode for incorrect code. A fix would require a very minor spec update-- a clarification in the behavior of a case that correct code should never encounter anyway-- but still a spec change, thus requiring due consideration and process (see Comments for a draft of the required spec addition). It is in all likelihood too late in the Merlin release cycle to consider a spec change like this when the apparent severity of the issue is low (there is a straightforward "workaround", which is what application code should be doing anyway), but the fix seems like a good candidate for consideration in the Tiger release. peter.c.jones@east 2001-08-06
        Hide
        defectconv Defect Conversion BT2 (Inactive) added a comment -
        BT2:WORK AROUND



        Name: bsC130419 Date: 08/02/2001


        When using proxy classes, manually ensure the handler is never null, or always
        use the newProxyInstance factory method.
        ======================================================================
        Show
        defectconv Defect Conversion BT2 (Inactive) added a comment - BT2:WORK AROUND Name: bsC130419 Date: 08/02/2001 When using proxy classes, manually ensure the handler is never null, or always use the newProxyInstance factory method. ======================================================================
        Hide
        darcy Joe Darcy added a comment -
        Changing assignee to engineer currently working in the area.
        Show
        darcy Joe Darcy added a comment - Changing assignee to engineer currently working in the area.
        Hide
        mchung Mandy Chung added a comment -
        Suggested fix:

             /**
              * Constructs a new {@code Proxy} instance from a subclass
              * (typically, a dynamic proxy class) with the specified value
              * for its invocation handler.
              *
              * @param h the invocation handler for this proxy instance
        + *
        + * @throws NullPointerException if the given invocation handler, {@code h},
        + * is {@code null}.
              */
             protected Proxy(InvocationHandler h) {
        + Objects.requireNonNull(h);
                 this.h = h;
             }
         
        Show
        mchung Mandy Chung added a comment - Suggested fix:      /**       * Constructs a new {@code Proxy} instance from a subclass       * (typically, a dynamic proxy class) with the specified value       * for its invocation handler.       *       * @param h the invocation handler for this proxy instance + * + * @throws NullPointerException if the given invocation handler, {@code h}, + * is {@code null}.       */      protected Proxy(InvocationHandler h) { + Objects.requireNonNull(h);          this.h = h;      }  
        Hide
        mchung Mandy Chung added a comment -
        Suggested text to be included in Release Note:

        SE API spec change:
          java.lang.reflect.Proxy(InvocationHandler h) constructor will throw a NullPointerException if the given InvocationHandler parameter is null.

        Compatibility Concern:
          This change may cause behavioral incompatibility. Existing code that constructs a dynamic proxy instance with null argument will get NullPointerException after this change.
        Such usage is expected to rarely exist since such proxy instance has no use at all and it'll throw NPE when its method is invoked anyway.
        Show
        mchung Mandy Chung added a comment - Suggested text to be included in Release Note: SE API spec change:   java.lang.reflect.Proxy(InvocationHandler h) constructor will throw a NullPointerException if the given InvocationHandler parameter is null. Compatibility Concern:   This change may cause behavioral incompatibility. Existing code that constructs a dynamic proxy instance with null argument will get NullPointerException after this change. Such usage is expected to rarely exist since such proxy instance has no use at all and it'll throw NPE when its method is invoked anyway.
        Hide
        hgupdate HG Updates added a comment -
        URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5e8959ab64af
        User: mchung
        Date: 2013-05-16 22:09:26 +0000
        Show
        hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5e8959ab64af User: mchung Date: 2013-05-16 22:09:26 +0000
        Hide
        hgupdate HG Updates added a comment -
        URL: http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/5e8959ab64af
        User: lana
        Date: 2013-05-21 18:20:30 +0000
        Show
        hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/5e8959ab64af User: lana Date: 2013-05-21 18:20:30 +0000
        Hide
        ewang Eric Wang (Inactive) added a comment -
        verified in b94
        Show
        ewang Eric Wang (Inactive) added a comment - verified in b94

          People

          • Assignee:
            mchung Mandy Chung
            Reporter:
            bstrathesunw Bill Strathearn (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Imported:
              Indexed: