From 8208bddbefa9613422b9c6a19ce39a24391beec3 Mon Sep 17 00:00:00 2001
From: Andreas Gampe <agampe@google.com>
Date: Mon, 27 Apr 2015 17:26:37 -0700
Subject: [PATCH] ART: Fix constructor access checking

Constructor access must be checked.

(cherry picked from commit 0dd76cd3f09f495a1b9a0e4f8712c09ff885c6fd)

Bug: 20639158
Change-Id: I3c586e9572a748d208bea43aa2349c3ef52a2ee5
---
 runtime/native/java_lang_reflect_Constructor.cc |  9 +++------
 test/100-reflect2/src/Main.java                 | 15 +++++++++++++++
 test/100-reflect2/src/sub/PPClass.java          | 23 +++++++++++++++++++++++
 3 files changed, 41 insertions(+), 6 deletions(-)
 create mode 100644 test/100-reflect2/src/sub/PPClass.java

diff --git a/runtime/native/java_lang_reflect_Constructor.cc b/runtime/native/java_lang_reflect_Constructor.cc
index 04d2e5e32..810b354b3 100644
--- a/runtime/native/java_lang_reflect_Constructor.cc
+++ b/runtime/native/java_lang_reflect_Constructor.cc
@@ -30,10 +30,7 @@
 namespace art {
 
 /*
- * We get here through Constructor.newInstance().  The Constructor object
- * would not be available if the constructor weren't public (per the
- * definition of Class.getConstructor), so we can skip the method access
- * check.  We can also safely assume the constructor isn't associated
+ * We can also safely assume the constructor isn't associated
  * with an interface, array, or primitive class. If this is coming from
  * native, it is OK to avoid access checks since JNI does not enforce them.
  */
@@ -48,8 +45,8 @@ static jobject Constructor_newInstance(JNIEnv* env, jobject javaMethod, jobjectA
                                    PrettyDescriptor(c.Get()).c_str());
     return nullptr;
   }
-  // Verify that we can access the class (only for debug since the above comment).
-  if (kIsDebugBuild && !c->IsPublic()) {
+  // Verify that we can access the class.
+  if (!c->IsPublic()) {
     auto* caller = GetCallingClass(soa.Self(), 1);
     // If caller is null, then we called from JNI, just avoid the check since JNI avoids most
     // access checks anyways. TODO: Investigate if this the correct behavior.
diff --git a/test/100-reflect2/src/Main.java b/test/100-reflect2/src/Main.java
index 0cc148812..86a5ef89d 100644
--- a/test/100-reflect2/src/Main.java
+++ b/test/100-reflect2/src/Main.java
@@ -266,9 +266,24 @@ class Main {
     show(ctor.newInstance(new char[] { 'x', 'y', 'z', '!' }, 1, 2));
   }
 
+  private static void testPackagePrivate() {
+    try {
+      Class<?> c = Class.forName("sub.PPClass");
+      Constructor cons = c.getConstructor();
+      cons.newInstance();
+      throw new RuntimeException("Expected IllegalAccessException.");
+    } catch (IllegalAccessException e) {
+      // Expected.
+    } catch (Exception e) {
+      // Error.
+      e.printStackTrace();
+    }
+  }
+
   public static void main(String[] args) throws Exception {
     testFieldReflection();
     testMethodReflection();
     testConstructorReflection();
+    testPackagePrivate();
   }
 }
diff --git a/test/100-reflect2/src/sub/PPClass.java b/test/100-reflect2/src/sub/PPClass.java
new file mode 100644
index 000000000..d972287fc
--- /dev/null
+++ b/test/100-reflect2/src/sub/PPClass.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package sub;
+
+// A package-private class with a public constructor.
+class PPClass {
+    public PPClass() {
+    }
+}
\ No newline at end of file
-- 
2.11.0