Allow IdeaLogger operations during render

On the render thread IdeaLogger sometimes needs to run some operations
like rotating the logs or accessing the System properties.
This change allows reading the system properties from within the Logger
and also allows writing to the logs directory.

Fixes: 223661896
Fixes: 223219330
Test: RenderSecurityManagerTest
Change-Id: I0239fe3862ba94368c0e942c91405154f4248bc8

4 files changed, 147 insertions, 1 deletions

diff --git a/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java b/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java
index ac62ef7ea12..c52665d161d 100644
--- a/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java
+++ b/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java
@@ -92,6 +92,9 @@ public class RenderSecurityManager extends SecurityManager {
   private final String mIndexRootPath;
   private final String mCachePath;
 
+  /** Root of the path where IntelliJ stores the logs. */
+  private final String mLogRootPath;
+
   private boolean mAllowSetSecurityManager;
   private boolean mDisabled;
   private final String mSdkPath;
@@ -144,6 +147,7 @@ public class RenderSecurityManager extends SecurityManager {
     mTempDir = System.getProperty("java.io.tmpdir");
     mNormalizedTempDir = new File(mTempDir).getPath(); // will call fs.normalize() on the path
     mIndexRootPath = PathManager.getIndexRoot().toString();
+    mLogRootPath = PathManager.getLogPath();
     mCachePath = PathManager.getSystemPath() + "/caches/";
     //noinspection AssignmentToStaticFieldFromInstanceMethod
     sLastFailedPath = null;
@@ -324,7 +328,13 @@ public class RenderSecurityManager extends SecurityManager {
   @Override
   public void checkPropertiesAccess() {
     if (isRelevant() && !RenderPropertiesAccessUtil.isPropertyAccessAllowed()) {
-      throw RenderSecurityException.create("Property", null);
+      boolean isWithinLogger = Arrays.stream(this.getClassContext())
+        .anyMatch(
+          (clazz) -> "Logger".equals(clazz.getSimpleName()) && "com.intellij.openapi.diagnostic.Logger".equals(clazz.getCanonicalName()));
+
+      if (!isWithinLogger) {
+        throw RenderSecurityException.create("Property", null);
+      }
     }
   }
 
@@ -425,6 +435,8 @@ public class RenderSecurityManager extends SecurityManager {
     return isTempDirPath(path) ||
            // When loading classes, IntelliJ might sometimes drop a corruption marker
            path.startsWith(mIndexRootPath) ||
+           // When rotating the logs, IntelliJ might need to write or update the log.
+           path.startsWith(mLogRootPath) ||
            // When loading classes, IntelliJ might try to update cache hashes for the loaded files
            path.startsWith(mCachePath);
   }
diff --git a/android/testSrc/com/android/tools/idea/rendering/RenderExecutorTest.kt b/android/testSrc/com/android/tools/idea/rendering/RenderExecutorTest.kt
index 13a1599aa30..64979053dbb 100644
--- a/android/testSrc/com/android/tools/idea/rendering/RenderExecutorTest.kt
+++ b/android/testSrc/com/android/tools/idea/rendering/RenderExecutorTest.kt
@@ -17,10 +17,17 @@ package com.android.tools.idea.rendering
 
 import com.android.testutils.VirtualTimeScheduler
 import com.android.testutils.concurrency.OnDemandExecutorService
+import com.android.tools.idea.concurrency.AndroidCoroutineScope
+import com.android.tools.idea.concurrency.AndroidDispatchers
+import com.android.tools.idea.concurrency.SupervisorJob
+import com.android.tools.idea.concurrency.androidCoroutineExceptionHandler
 import com.google.common.util.concurrent.MoreExecutors
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
@@ -272,4 +279,22 @@ class RenderExecutorTest {
       executor.shutdown()
     }
   }
+
+  @Test
+  fun testActionTimeout2() {
+    val executor = RenderExecutor.create()
+
+    val future = executor.runAsyncActionWithTimeout(
+      queueingTimeout =10, queueingTimeoutUnit = TimeUnit.SECONDS,
+      actionTimeout = 10, actionTimeoutUnit = TimeUnit.SECONDS) {
+      runBlocking {
+        CoroutineScope(kotlinx.coroutines.SupervisorJob() + Dispatchers.Default + androidCoroutineExceptionHandler).launch {
+          throw IllegalArgumentException()
+        }
+      }
+      println("Done")
+    }
+
+    future.join()
+  }
 }
\ No newline at end of file
diff --git a/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java b/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java
index 649f7f3503a..7bc0b824b09 100644
--- a/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java
+++ b/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java
@@ -20,6 +20,8 @@ import com.android.tools.idea.testing.AndroidProjectRule;
 import com.android.utils.SdkUtils;
 import com.google.common.io.Files;
 import java.io.IOException;
+import com.intellij.openapi.application.PathManager;
+import com.intellij.openapi.diagnostic.Logger;
 import org.jetbrains.android.AndroidTestBase;
 import org.junit.Rule;
 import org.junit.Test;
@@ -869,5 +871,72 @@ public class RenderSecurityManagerTest {
     }
   }
 
+  /**
+   * Regression test for b/223219330.
+   */
+  @Test
+  public void testLogDir() {
+    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    try {
+      manager.setActive(true, myCredential);
+
+      String logPath = PathManager.getLogPath();
+      assertNotNull(logPath);
+
+      manager.checkPermission(new FilePermission(logPath, "read,write"));
+      manager.checkPermission(new FilePermission(logPath + separator, "read,write"));
+      manager.checkPermission(new FilePermission(logPath + separator + "fake.log", "read,write"));
+
+    }
+    finally {
+      manager.dispose(myCredential);
+    }
+  }
+  @Test
+  public void testLogException() {
+    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    try {
+      manager.setActive(true, myCredential);
+
+      String logPath = PathManager.getLogPath();
+      assertNotNull(logPath);
+
+      Logger.Factory oldFactory = Logger.getFactory();
+      try {
+        TestLoggerWithPropertyAccess loggerWithPropertyAccess = new TestLoggerWithPropertyAccess(Logger.getInstance(RenderSecurityManager.class));
+        Logger.setFactory(category -> loggerWithPropertyAccess);
+        Logger.getInstance(RenderSecurityManagerTest.class).error("test", new TestException());
+      } catch (Throwable t) {
+        // We expect the actual cause to be the TestException if the sandboxing is working correctly. If not, it will throw a security
+        // exception.
+        assertTrue("Unexpected exception " + t, t.getCause() instanceof TestException);
+      } finally {
+        Logger.setFactory(oldFactory);
+      }
+    }
+    finally {
+      manager.dispose(myCredential);
+    }
+  }
+
+  @Test
+  public void testSystemPropertiesAccess() {
+    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    try {
+      manager.setActive(true, myCredential);
+
+      try {
+        //noinspection ResultOfMethodCallIgnored
+        System.getProperties();
+        fail("Expected to throw RenderSecurityException");
+      } catch (RenderSecurityException ignore) {
+        // Expected to throw a security exception.
+      }
+    }
+    finally {
+      manager.dispose(myCredential);
+    }
+  }
 
+  private static class TestException extends Throwable { }
 }
diff --git a/android/testSrc/com/android/tools/idea/rendering/TestLoggerWithPropertyAccess.kt b/android/testSrc/com/android/tools/idea/rendering/TestLoggerWithPropertyAccess.kt
new file mode 100644
index 00000000000..f1e49b6409a
--- /dev/null
+++ b/android/testSrc/com/android/tools/idea/rendering/TestLoggerWithPropertyAccess.kt
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2022 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 com.android.tools.idea.rendering
+
+import com.intellij.openapi.diagnostic.Logger
+import org.apache.log4j.Level
+
+/**
+ * A [Logger] that is used for testing sandboxing. This will simulate a property access during error logging that does not happen in the
+ * unit test logging but does happen in production.
+ */
+class TestLoggerWithPropertyAccess(private val delegate: Logger): Logger() {
+  override fun isDebugEnabled(): Boolean = delegate.isDebugEnabled
+  override fun debug(message: String?) = delegate.debug(message)
+  override fun debug(t: Throwable?) = delegate.debug(t)
+  override fun debug(message: String?, t: Throwable?) = delegate.debug(message, t)
+  override fun info(message: String?) = delegate.info(message)
+  override fun info(message: String?, t: Throwable?) = delegate.info(message, t)
+  override fun warn(message: String?, t: Throwable?) = delegate.warn(message, t)
+  override fun error(message: String?, t: Throwable?, vararg details: String?) {
+    // Simulate a properties access as the IdeaLogger does.
+    System.getProperties()
+    delegate.error(message, t, *details)
+  }
+  @Suppress("UnstableApiUsage")
+  override fun setLevel(level: Level) = delegate.setLevel(level)
+}
\ No newline at end of file