apache license, checkstyle, pr feedback

Author: charlesconnell

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslPropertiesResolver.java

@@ -31,6 +31,8 @@
 import org.apache.hadoop.util.ReflectionUtils;
 import org.apache.hadoop.util.StringUtils;
 
+import static java.util.Objects.requireNonNull;
+
 /**
  * Provides SaslProperties to be used for a connection.
  * The default implementation is to read the values from configuration.
@@ -39,7 +41,7 @@
  *
  */
 public class SaslPropertiesResolver implements Configurable{
-  private SortedMap<String,String> properties;
+  private SortedMap<String, String> properties;
   Configuration conf;
 
   /**
@@ -83,7 +85,7 @@ public Configuration getConf() {
    * @return sasl Properties
    */
   public Map<String,String> getDefaultProperties() {
-    return new TreeMap<>(properties);
+    return cloneProperties();
   }
 
   /**
@@ -92,7 +94,7 @@ public Map<String,String> getDefaultProperties() {
    * @return the sasl properties to be used for the connection.
    */
   public Map<String, String> getServerProperties(InetAddress clientAddress){
-    return new TreeMap<>(properties);
+    return cloneProperties();
   }
 
   /**
@@ -112,7 +114,7 @@ public Map<String, String> getServerProperties(InetAddress clientAddress,
    * @return the sasl properties to be used for the connection.
    */
   public Map<String, String> getClientProperties(InetAddress serverAddress){
-    return new TreeMap<>(properties);
+    return cloneProperties();
   }
 
   /**
@@ -126,6 +128,12 @@ public Map<String, String> getClientProperties(InetAddress serverAddress,
     return getClientProperties(serverAddress);
   }
 
+  private Map<String, String> cloneProperties() {
+    requireNonNull(properties,
+        "properties map in SaslPropertiesResolver was null, did you call setConf()?");
+    return new TreeMap<>(properties);
+  }
+
   /**
    * A util function to retrieve specific additional sasl property from config.
    * Used by subclasses to read sasl properties used by themselves.

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestSaslPropertiesResolver.java

@@ -1,21 +1,38 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.hadoop.security;
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.util.Map;
 import javax.security.sasl.Sasl;
 
 import org.junit.Before;
 import org.junit.Test;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
 
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_RPC_PROTECTION;
 import static org.junit.Assert.assertEquals;
 
-public class TestSaslPropertiesResolver {
+public class TestSaslPropertiesResolver extends AbstractHadoopTestBase {
 
-  private static final SaslRpcServer.QualityOfProtection PRIVACY_QOP = SaslRpcServer.QualityOfProtection.PRIVACY;
-  private static final SaslRpcServer.QualityOfProtection AUTHENTICATION_QOP = SaslRpcServer.QualityOfProtection.AUTHENTICATION;
   private static final InetAddress LOCALHOST = new InetSocketAddress("127.0.0.1", 1).getAddress();
 
   private SaslPropertiesResolver resolver;
@@ -30,20 +47,29 @@ public void setup() {
 
   @Test
   public void testResolverDoesNotMutate() {
-    assertEquals(PRIVACY_QOP.getSaslQop(), resolver.getDefaultProperties().get(Sasl.QOP));
-    resolver.getDefaultProperties().put(Sasl.QOP, AUTHENTICATION_QOP.getSaslQop());
-    // Even after changing the map returned by SaslPropertiesResolver, it does not change its future responses
-    assertEquals(PRIVACY_QOP.getSaslQop(), resolver.getDefaultProperties().get(Sasl.QOP));
-
-    assertEquals(PRIVACY_QOP.getSaslQop(), resolver.getClientProperties(LOCALHOST).get(Sasl.QOP));
-    resolver.getDefaultProperties().put(Sasl.QOP, AUTHENTICATION_QOP.getSaslQop());
-    // Even after changing the map returned by SaslPropertiesResolver, it does not change its future responses
-    assertEquals(PRIVACY_QOP.getSaslQop(), resolver.getClientProperties(LOCALHOST).get(Sasl.QOP));
-
-    assertEquals(PRIVACY_QOP.getSaslQop(), resolver.getServerProperties(LOCALHOST).get(Sasl.QOP));
-    resolver.getDefaultProperties().put(Sasl.QOP, AUTHENTICATION_QOP.getSaslQop());
-    // Even after changing the map returned by SaslPropertiesResolver, it does not change its future responses
-    assertEquals(PRIVACY_QOP.getSaslQop(), resolver.getServerProperties(LOCALHOST).get(Sasl.QOP));
+    assertPrivacyQop(resolver.getDefaultProperties());
+    setAuthenticationQop(resolver.getDefaultProperties());
+    // After changing the map returned by SaslPropertiesResolver, future maps are not affected
+    assertPrivacyQop(resolver.getDefaultProperties());
+
+    assertPrivacyQop(resolver.getClientProperties(LOCALHOST));
+    setAuthenticationQop(resolver.getClientProperties(LOCALHOST));
+    // After changing the map returned by SaslPropertiesResolver, future maps are not affected
+    assertPrivacyQop(resolver.getClientProperties(LOCALHOST));
+
+    assertPrivacyQop(resolver.getServerProperties(LOCALHOST));
+    setAuthenticationQop(resolver.getServerProperties(LOCALHOST));
+    // After changing the map returned by SaslPropertiesResolver, future maps are not affected
+    assertPrivacyQop(resolver.getServerProperties(LOCALHOST));
+  }
+
+  private static void setAuthenticationQop(Map<String, String> saslProperties) {
+    saslProperties.put(Sasl.QOP, SaslRpcServer.QualityOfProtection.AUTHENTICATION.getSaslQop());
+  }
+
+  private static void assertPrivacyQop(Map<String, String> saslProperties) {
+    assertEquals(SaslRpcServer.QualityOfProtection.PRIVACY.getSaslQop(),
+        saslProperties.get(Sasl.QOP));
   }
 
 }