Mailing-List: contact dev-help@activemq.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@activemq.apache.org
From: mtaylor <git@git.apache.org>
To: dev@activemq.apache.org
Reply-To: dev@activemq.apache.org
References: <git-pr-632-activemq-artemis@git.apache.org>
In-Reply-To: <git-pr-632-activemq-artemis@git.apache.org>
Subject: [GitHub] activemq-artemis pull request #632: ARTEMIS-604 - Message Serialization Impr...
Content-Type: text/plain
Message-Id: <20160718164010.B98C7DFAF5@git1-us-west.apache.org>
Date: Mon, 18 Jul 2016 16:40:10 +0000 (UTC)
archived-at: Mon, 18 Jul 2016 16:40:12 -0000

Github user mtaylor commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/632#discussion_r71182493
  
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/ObjectInputStreamWithClassLoader.java ---
    @@ -25,23 +25,110 @@
     import java.security.AccessController;
     import java.security.PrivilegedActionException;
     import java.security.PrivilegedExceptionAction;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Iterator;
    +import java.util.List;
     
     public class ObjectInputStreamWithClassLoader extends ObjectInputStream {
     
        // Constants ------------------------------------------------------------------------------------
     
    +   /**
    +    * Value used to indicate that all classes should be white or black listed,
    +    */
    +   public static final String CATCH_ALL_WILDCARD = "*";
    +
    +   public static final String WHITELIST_PROPERTY = "org.apache.activemq.artemis.jms.deserialization.whitelist";
    +   public static final String BLACKLIST_PROPERTY = "org.apache.activemq.artemis.jms.deserialization.blacklist";
    +
        // Attributes -----------------------------------------------------------------------------------
     
    +   private List<String> whiteList = new ArrayList<>();
    +   private List<String> blackList = new ArrayList<>();
    +
        // Static ---------------------------------------------------------------------------------------
     
        // Constructors ---------------------------------------------------------------------------------
     
        public ObjectInputStreamWithClassLoader(final InputStream in) throws IOException {
           super(in);
    +      String whiteList = System.getProperty(WHITELIST_PROPERTY, null);
    +      setWhiteList(whiteList);
    +
    +      String blackList = System.getProperty(BLACKLIST_PROPERTY, null);
    +      setBlackList(blackList);
        }
     
        // Public ---------------------------------------------------------------------------------------
     
    +   /**
    +    * @return the whiteList configured on this policy instance.
    +    */
    +   public String getWhiteList() {
    +      Iterator<String> entries = whiteList.iterator();
    +      StringBuilder builder = new StringBuilder();
    +
    +      while (entries.hasNext()) {
    +         builder.append(entries.next());
    +         if (entries.hasNext()) {
    +            builder.append(",");
    +         }
    +      }
    +      return builder.toString();
    +   }
    +
    +   /**
    +    * @return the blackList configured on this policy instance.
    +    */
    +   public String getBlackList() {
    +      Iterator<String> entries = blackList.iterator();
    +      StringBuilder builder = new StringBuilder();
    +
    +      while (entries.hasNext()) {
    +         builder.append(entries.next());
    +         if (entries.hasNext()) {
    +            builder.append(",");
    +         }
    +      }
    +
    +      return builder.toString();
    +   }
    +
    +   /**
    +    * Replaces the currently configured whiteList with a comma separated
    +    * string containing the new whiteList. Null or empty string denotes
    +    * no whiteList entries, {@value #CATCH_ALL_WILDCARD} indicates that
    +    * all classes are whiteListed.
    +    *
    +    * @param whiteList the whiteList that this policy is configured to recognize.
    +    */
    +   public void setWhiteList(String whiteList) {
    +      ArrayList<String> list = new ArrayList<>();
    +      if (whiteList != null && !whiteList.isEmpty()) {
    +         list.addAll(Arrays.asList(whiteList.split(",")));
    +      }
    +
    +      this.whiteList = list;
    +   }
    +
    +   /**
    +    * Replaces the currently configured blackList with a comma separated
    +    * string containing the new blackList. Null or empty string denotes
    +    * no blacklist entries, {@value #CATCH_ALL_WILDCARD} indicates that
    +    * all classes are blacklisted.
    +    *
    +    * @param blackList the blackList that this policy is configured to recognize.
    +    */
    +   public void setBlackList(String blackList) {
    +      ArrayList<String> list = new ArrayList<>();
    +      if (blackList != null && !blackList.isEmpty()) {
    +         list.addAll(Arrays.asList(blackList.split(",")));
    +      }
    +
    +      this.blackList = list;
    +   }
    +
    --- End diff --
    
    Same comment as above.  You could move the list validation and parse into a separation method to avoid duplication.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---