You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2018/11/16 00:47:01 UTC

[geode] branch feature/GEODE-5076 created (now 50b191e)

This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a change to branch feature/GEODE-5076
in repository https://gitbox.apache.org/repos/asf/geode.git.


      at 50b191e  GEODE-5076 jmx client should not access or modify internal regions

This branch includes the following new commits:

     new 50b191e  GEODE-5076 jmx client should not access or modify internal regions

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[geode] 01/01: GEODE-5076 jmx client should not access or modify internal regions

Posted by bs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-5076
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 50b191e8edb591ab2d3cb6052030a32a3c300bcc
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Thu Nov 15 16:45:27 2018 -0800

    GEODE-5076 jmx client should not access or modify internal regions
    
    Modified the management packages to use a filtered cache that doesn't
    allow users access to internal regions.
---
 .../apache/geode/internal/cache/GemFireCacheImpl.java |  3 ++-
 .../apache/geode/internal/cache/InternalCache.java    |  2 +-
 .../internal/cache/InternalCacheForClientAccess.java  | 19 ++++++++++++++-----
 .../geode/internal/cache/xmlcache/CacheCreation.java  |  3 ++-
 .../geode/management/internal/FederatingManager.java  |  9 +++++----
 .../geode/management/internal/JmxManagerLocator.java  |  4 +---
 .../geode/management/internal/LocalManager.java       |  4 ++--
 .../geode/management/internal/ManagementAgent.java    | 13 +++++++++++--
 .../org/apache/geode/management/internal/Manager.java |  5 +++--
 .../management/internal/beans/BeanUtilFuncs.java      |  4 ++--
 .../management/internal/beans/CacheServerBridge.java  |  4 ++--
 .../management/internal/beans/QueryDataFunction.java  | 16 ++++------------
 .../management/internal/cli/modes/CommandModes.java   |  8 ++++++--
 .../internal/configuration/domain/XmlEntity.java      |  3 ++-
 14 files changed, 57 insertions(+), 40 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index ce0195e..0b0b2ed 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -265,6 +265,7 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
   public static final boolean DEFAULT_COPY_ON_READ = false;
 
   /**
+   * getcachefor
    * The default amount of time to wait for a {@code netSearch} to complete
    */
   public static final int DEFAULT_SEARCH_TIMEOUT =
@@ -5329,7 +5330,7 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
       new InternalCacheForClientAccess(this);
 
   @Override
-  public InternalCache getCacheForProcessingClientRequests() {
+  public InternalCacheForClientAccess getCacheForProcessingClientRequests() {
     return cacheForClients;
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java
index 0f97fbf..d44f7ca 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java
@@ -367,5 +367,5 @@ public interface InternalCache extends Cache, Extensible<Cache>, CacheTime {
    * application visible regions. Any regions created internally
    * by Geode will not be accessible from the returned cache.
    */
-  InternalCache getCacheForProcessingClientRequests();
+  InternalCacheForClientAccess getCacheForProcessingClientRequests();
 }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java
index 0b5bc81..f5657fb 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java
@@ -138,10 +138,9 @@ public class InternalCacheForClientAccess implements InternalCache {
   }
 
   /**
-   * Server-side code using an InternalCacheForClientAccess may need to
-   * get an Internal Region not normally accesible and may use this method to
-   * do so. The REST API, for instance, needs to get at a Query store
-   * region that is not otherwise accessible through the getRegion methods.
+   * This method can be used to locate an internal region.
+   * It should not be invoked with a region name obtained
+   * from a client.
    */
   public <K, V> Region<K, V> getInternalRegion(String path) {
     Region<K, V> result = delegate.getRegion(path);
@@ -228,6 +227,16 @@ public class InternalCacheForClientAccess implements InternalCache {
     return delegate.createVMRegion(name, p_attrs, internalRegionArgs);
   }
 
+  /**
+   * This method allows server-side code to create an internal region. It should
+   * not be invoked with a region name obtained from a client.
+   */
+  public <K, V> Region<K, V> createInternalRegion(String name, RegionAttributes<K, V> p_attrs,
+      InternalRegionArguments internalRegionArgs)
+      throws RegionExistsException, TimeoutException, IOException, ClassNotFoundException {
+    return delegate.createVMRegion(name, p_attrs, internalRegionArgs);
+  }
+
   @Override
   public Cache getReconnectedCache() {
     Cache reconnectedCache = delegate.getReconnectedCache();
@@ -1201,7 +1210,7 @@ public class InternalCacheForClientAccess implements InternalCache {
   }
 
   @Override
-  public InternalCache getCacheForProcessingClientRequests() {
+  public InternalCacheForClientAccess getCacheForProcessingClientRequests() {
     return this;
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java
index 2ee6126..9e9f9ed 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java
@@ -119,6 +119,7 @@ import org.apache.geode.internal.cache.FilterProfile;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.InitialImageOperation;
 import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.cache.InternalCacheForClientAccess;
 import org.apache.geode.internal.cache.InternalRegion;
 import org.apache.geode.internal.cache.InternalRegionArguments;
 import org.apache.geode.internal.cache.LocalRegion;
@@ -2407,7 +2408,7 @@ public class CacheCreation implements InternalCache {
   }
 
   @Override
-  public InternalCache getCacheForProcessingClientRequests() {
+  public InternalCacheForClientAccess getCacheForProcessingClientRequests() {
     throw new UnsupportedOperationException("Should not be invoked");
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java b/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
index 8b231ad..801dd6b 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
@@ -357,8 +357,9 @@ public class FederatingManager extends Manager {
         String monitoringRegionName = ManagementConstants.MONITORING_REGION + "_" + appender;
         String notificationRegionName = ManagementConstants.NOTIFICATION_REGION + "_" + appender;
 
-        if (cache.getRegion(monitoringRegionName) != null
-            && cache.getRegion(notificationRegionName) != null) {
+
+        if (cache.getInternalRegion(monitoringRegionName) != null
+            && cache.getInternalRegion(notificationRegionName) != null) {
           return member;
         }
 
@@ -415,7 +416,7 @@ public class FederatingManager extends Manager {
                 return null;
               }
               proxyMonitoringRegion =
-                  cache.createVMRegion(monitoringRegionName, monitoringRegionAttrs,
+                  cache.createInternalRegion(monitoringRegionName, monitoringRegionAttrs,
                       internalRegionArguments);
               proxyMonitoringRegionCreated = true;
 
@@ -434,7 +435,7 @@ public class FederatingManager extends Manager {
                 return null;
               }
               proxyNotificationRegion =
-                  cache.createVMRegion(notificationRegionName, notifRegionAttrs,
+                  cache.createInternalRegion(notificationRegionName, notifRegionAttrs,
                       internalRegionArguments);
               proxyNotificationRegionCreated = true;
             } catch (TimeoutException | RegionExistsException | IOException
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocator.java b/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocator.java
index a0c0202..bd294bc 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocator.java
@@ -21,8 +21,6 @@ import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.CancelException;
 import org.apache.geode.SystemFailure;
-import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.GemFireCache;
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.FunctionService;
@@ -203,7 +201,7 @@ public class JmxManagerLocator implements TcpHandler {
     @Override
     public void execute(FunctionContext context) {
       try {
-        Cache cache = CacheFactory.getAnyInstance();
+        InternalCache cache = ManagementAgent.getCache();
         if (cache != null) {
           ManagementService ms = ManagementService.getExistingManagementService(cache);
           if (ms != null) {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java b/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java
index 95f656d..c36479e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/LocalManager.java
@@ -151,7 +151,7 @@ public class LocalManager extends Manager {
 
         try {
           repo.setLocalMonitoringRegion(
-              cache.createVMRegion(ManagementConstants.MONITORING_REGION + "_" + appender,
+              cache.createInternalRegion(ManagementConstants.MONITORING_REGION + "_" + appender,
                   monitoringRegionAttrs, internalArgs));
           monitoringRegionCreated = true;
 
@@ -167,7 +167,7 @@ public class LocalManager extends Manager {
 
         try {
           repo.setLocalNotificationRegion(
-              cache.createVMRegion(ManagementConstants.NOTIFICATION_REGION + "_" + appender,
+              cache.createInternalRegion(ManagementConstants.NOTIFICATION_REGION + "_" + appender,
                   notifRegionAttrs, internalArgs));
           notifRegionCreated = true;
         } catch (TimeoutException e) {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
index 31737fe..ddd60a9 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
@@ -55,6 +55,7 @@ import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.GemFireVersion;
 import org.apache.geode.internal.admin.SSLConfig;
 import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.cache.InternalCacheForClientAccess;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.net.SSLConfigurationFactory;
 import org.apache.geode.internal.net.SocketCreator;
@@ -142,6 +143,14 @@ public class ManagementAgent {
         && !cache.isClient());
   }
 
+  public static InternalCacheForClientAccess getCache() {
+    InternalCache cache = (InternalCache) CacheFactory.getAnyInstance();
+    if (cache != null) {
+      return cache.getCacheForProcessingClientRequests();
+    }
+    return null;
+  }
+
   public synchronized void startAgent(InternalCache cache) {
     // Do not start Management REST service if developer REST service is already
     // started.
@@ -191,7 +200,7 @@ public class ManagementAgent {
 
   private void startHttpService(boolean isServer) {
     final SystemManagementService managementService = (SystemManagementService) ManagementService
-        .getManagementService(CacheFactory.getAnyInstance());
+        .getManagementService(getCache());
 
     final ManagerMXBean managerBean = managementService.getManagerMXBean();
 
@@ -305,7 +314,7 @@ public class ManagementAgent {
 
           // set cache property for developer REST service running
           if (isRestWebAppAdded) {
-            InternalCache cache = (InternalCache) CacheFactory.getAnyInstance();
+            InternalCache cache = getCache();
             cache.setRESTServiceRunning(true);
 
             // create region to hold query information (queryId, queryString).
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/Manager.java b/geode-core/src/main/java/org/apache/geode/management/internal/Manager.java
index 916cbd7..20ea590 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/Manager.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/Manager.java
@@ -16,6 +16,7 @@ package org.apache.geode.management.internal;
 
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.cache.InternalCacheForClientAccess;
 
 /**
  * The Manager is a 7.0 JMX Agent which is hosted within a GemFire process. Only one instance is
@@ -26,7 +27,7 @@ import org.apache.geode.internal.cache.InternalCache;
  */
 public abstract class Manager {
 
-  protected InternalCache cache;
+  protected InternalCacheForClientAccess cache;
 
   /**
    * depicts whether this node is a Managing node or not
@@ -51,7 +52,7 @@ public abstract class Manager {
   public Manager(ManagementResourceRepo repo, InternalDistributedSystem system,
       InternalCache cache) {
     this.repo = repo;
-    this.cache = cache;
+    this.cache = cache.getCacheForProcessingClientRequests();
     this.system = system;
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
index fab0bbd..14d9005 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
@@ -26,11 +26,11 @@ import java.util.Set;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
-import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.management.GemFireProperties;
+import org.apache.geode.management.internal.ManagementAgent;
 import org.apache.geode.management.internal.cli.CliUtil;
 
 /**
@@ -123,7 +123,7 @@ public class BeanUtilFuncs {
     DistributedMember memberFound = null;
 
     if (memberNameOrId != null) {
-      InternalCache cache = (InternalCache) CacheFactory.getAnyInstance();
+      InternalCache cache = ManagementAgent.getCache();
       Set<DistributedMember> memberSet = CliUtil.getAllMembers(cache);
       for (DistributedMember member : memberSet) {
         if (memberNameOrId.equals(member.getId()) || memberNameOrId.equals(member.getName())) {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
index 36bc96e..bab3269 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
@@ -24,7 +24,6 @@ import java.util.Map;
 
 import org.apache.logging.log4j.Logger;
 
-import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.query.CqClosedException;
 import org.apache.geode.cache.query.CqException;
@@ -57,6 +56,7 @@ import org.apache.geode.internal.process.ProcessUtils;
 import org.apache.geode.management.ClientHealthStatus;
 import org.apache.geode.management.ClientQueueDetail;
 import org.apache.geode.management.ServerLoadData;
+import org.apache.geode.management.internal.ManagementAgent;
 import org.apache.geode.management.internal.ManagementConstants;
 import org.apache.geode.management.internal.beans.stats.MBeanStatsMonitor;
 import org.apache.geode.management.internal.beans.stats.StatType;
@@ -405,7 +405,7 @@ public class CacheServerBridge extends ServerBridge {
   }
 
   public Version getClientVersion(ClientConnInfo connInfo) {
-    InternalCache cache = (InternalCache) CacheFactory.getAnyInstance();
+    InternalCache cache = ManagementAgent.getCache();
 
     if (cache.getCacheServers().size() == 0) {
       return null;
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java
index a8a5bfc..7d2dc35 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java
@@ -30,7 +30,6 @@ import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.SystemFailure;
-import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.execute.Function;
@@ -57,6 +56,7 @@ import org.apache.geode.internal.cache.execute.InternalFunction;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.DistributedRegionMXBean;
 import org.apache.geode.management.ManagementService;
+import org.apache.geode.management.internal.ManagementAgent;
 import org.apache.geode.management.internal.ManagementConstants;
 import org.apache.geode.management.internal.SystemManagementService;
 import org.apache.geode.management.internal.cli.CliUtil;
@@ -126,7 +126,7 @@ public class QueryDataFunction implements Function, InternalEntity {
   private QueryDataFunctionResult selectWithType(final FunctionContext context, String queryString,
       final boolean showMember, final String regionName, final int limit,
       final int queryResultSetLimit, final int queryCollectionsDepth) throws Exception {
-    InternalCache cache = getCache();
+    InternalCache cache = ManagementAgent.getCache();
     Function localQueryFunc = new LocalQueryFunction("LocalQueryFunction", regionName, showMember)
         .setOptimizeForWrite(true);
     queryString = applyLimitClause(queryString, limit, queryResultSetLimit);
@@ -346,7 +346,7 @@ public class QueryDataFunction implements Function, InternalEntity {
       }
     }
 
-    InternalCache cache = (InternalCache) CacheFactory.getAnyInstance();
+    InternalCache cache = ManagementAgent.getCache();
     try {
 
       SystemManagementService service =
@@ -434,10 +434,6 @@ public class QueryDataFunction implements Function, InternalEntity {
     }
   }
 
-  private InternalCache getCache() {
-    return (InternalCache) CacheFactory.getAnyInstance();
-  }
-
   private static class JsonisedErrorMessage {
 
     private static String message = "message";
@@ -525,7 +521,7 @@ public class QueryDataFunction implements Function, InternalEntity {
 
     @Override
     public void execute(final FunctionContext context) {
-      InternalCache cache = getCache();
+      InternalCache cache = ManagementAgent.getCache();
       QueryService queryService = cache.getQueryService();
       String qstr = (String) context.getArguments();
       Region r = cache.getRegion(regionName);
@@ -545,10 +541,6 @@ public class QueryDataFunction implements Function, InternalEntity {
       }
     }
 
-    private InternalCache getCache() {
-      return (InternalCache) CacheFactory.getAnyInstance();
-    }
-
     @Override
     public String getId() {
       return this.id;
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/modes/CommandModes.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/modes/CommandModes.java
index c50fbf3..d2ccee8 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/modes/CommandModes.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/modes/CommandModes.java
@@ -27,7 +27,8 @@ import org.json.JSONObject;
 
 import org.apache.geode.LogWriter;
 import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.internal.ManagementAgent;
 
 public class CommandModes {
 
@@ -86,7 +87,10 @@ public class CommandModes {
   }
 
   private void logException(Exception e) {
-    Cache cache = CacheFactory.getAnyInstance();
+    Cache cache = ManagementAgent.getCache();
+    if (cache != null && cache instanceof InternalCache) {
+      cache = ((InternalCache) cache).getCacheForProcessingClientRequests();
+    }
     LogWriter logger = cache.getLogger();
     logger.warning("Error parsing command mode descriptor", e);
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java
index 9fa2e9d..dc38aeb 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java
@@ -90,7 +90,8 @@ public class XmlEntity implements VersionedDataSerializable {
   }
 
   private static CacheProvider createDefaultCacheProvider() {
-    return () -> (InternalCache) CacheFactory.getAnyInstance();
+    return () -> ((InternalCache) CacheFactory.getAnyInstance())
+        .getCacheForProcessingClientRequests();
   }
 
   /**