HBASE-27240 Clean up error-prone findings in hbase-http (#4653)

Signed-off-by: Duo Zhang <zhangduo@apache.org>

hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpConfig.java

@@ -71,7 +71,7 @@ public class HttpConfig {
   }
 
   public String getSchemePrefix() {
-    return (isSecure()) ? "https://" : "http://";
+    return isSecure() ? "https://" : "http://";
   }
 
   public String getScheme(Policy policy) {

hbase-http/src/main/java/org/apache/hadoop/hbase/http/jmx/JMXJsonServlet.java

@@ -20,6 +20,8 @@ package org.apache.hadoop.hbase.http.jmx;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.lang.management.ManagementFactory;
+import java.util.Iterator;
+import java.util.List;
 import javax.management.MBeanServer;
 import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
@@ -35,6 +37,8 @@ import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.base.Splitter;
+
 /*
  * This servlet is based off of the JMXProxyServlet from Tomcat 7.0.14. It has
  * been rewritten to be read only and to output in a JSON format so it is not
@@ -173,17 +177,17 @@ public class JMXJsonServlet extends HttpServlet {
         // query per mbean attribute
         String getmethod = request.getParameter("get");
         if (getmethod != null) {
-          String[] splitStrings = getmethod.split("\\:\\:");
-          if (splitStrings.length != 2) {
+          List<String> splitStrings = Splitter.onPattern("\\:\\:").splitToList(getmethod);
+          if (splitStrings.size() != 2) {
             beanWriter.write("result", "ERROR");
             beanWriter.write("message", "query format is not as expected.");
             beanWriter.flush();
             response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
             return;
           }
+          Iterator<String> i = splitStrings.iterator();
           if (
-            beanWriter.write(this.mBeanServer, new ObjectName(splitStrings[0]), splitStrings[1],
-              description) != 0
+            beanWriter.write(this.mBeanServer, new ObjectName(i.next()), i.next(), description) != 0
           ) {
             beanWriter.flush();
             response.setStatus(HttpServletResponse.SC_BAD_REQUEST);

hbase-http/src/main/java/org/apache/hadoop/hbase/http/lib/StaticUserWebFilter.java

@@ -39,6 +39,9 @@ import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.base.Splitter;
+import org.apache.hbase.thirdparty.com.google.common.collect.Iterables;
+
 /**
  * Provides a servlet filter that pretends to authenticate a fake user (Dr.Who) so that the web UI
  * is usable for a secure cluster without authentication.
@@ -70,7 +73,7 @@ public class StaticUserWebFilter extends FilterInitializer {
     public boolean equals(Object other) {
       if (other == this) {
         return true;
-      } else if (other == null || other.getClass() != getClass()) {
+      } else if (!(other instanceof User)) {
         return false;
       }
       return ((User) other).name.equals(name);
@@ -143,8 +146,7 @@ public class StaticUserWebFilter extends FilterInitializer {
       // since we need to split out the username from the configured UGI.
       LOG.warn(
         DEPRECATED_UGI_KEY + " should not be used. Instead, use " + HBASE_HTTP_STATIC_USER + ".");
-      String[] parts = oldStyleUgi.split(",");
-      return parts[0];
+      return Iterables.get(Splitter.on(',').split(oldStyleUgi), 0);
     } else {
       return conf.get(HBASE_HTTP_STATIC_USER, DEFAULT_HBASE_HTTP_STATIC_USER);
     }

hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java

@@ -85,7 +85,7 @@ public final class LogLevel {
   }
 
   public static boolean isValidProtocol(String protocol) {
-    return ((protocol.equals(PROTOCOL_HTTP) || protocol.equals(PROTOCOL_HTTPS)));
+    return protocol.equals(PROTOCOL_HTTP) || protocol.equals(PROTOCOL_HTTPS);
   }
 
   static class CLI extends Configured implements Tool {

hbase-http/src/main/java/org/apache/hadoop/hbase/util/JSONBean.java

@@ -133,8 +133,7 @@ public class JSONBean {
   private static int write(JsonWriter writer, MBeanServer mBeanServer, ObjectName qry,
     String attribute, boolean description, ObjectName excluded) throws IOException {
     LOG.debug("Listing beans for {}", qry);
-    Set<ObjectName> names = null;
-    names = mBeanServer.queryNames(qry, null);
+    Set<ObjectName> names = mBeanServer.queryNames(qry, null);
     writer.name("beans").beginArray();
     Iterator<ObjectName> it = names.iterator();
     Pattern[] matchingPattern = null;

hbase-http/src/main/java/org/apache/hadoop/hbase/util/JSONMetricUtil.java

@@ -40,6 +40,9 @@ import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.base.Splitter;
+import org.apache.hbase.thirdparty.com.google.common.collect.Iterables;
+
 @InterfaceAudience.Private
 public final class JSONMetricUtil {
 
@@ -111,6 +114,7 @@ public final class JSONMetricUtil {
    * @param values Map values
    * @return Map or null if arrays are empty * or have different number of elements
    */
+  @SuppressWarnings("JdkObsolete") // javax requires hashtable param for ObjectName constructor
   public static Hashtable<String, String> buldKeyValueTable(String[] keys, String[] values) {
     if (keys.length != values.length) {
       LOG.error("keys and values arrays must be same size");
@@ -141,7 +145,7 @@ public final class JSONMetricUtil {
   }
 
   public static String getProcessPID() {
-    return ManagementFactory.getRuntimeMXBean().getName().split("@")[0];
+    return Iterables.get(Splitter.on('@').split(ManagementFactory.getRuntimeMXBean().getName()), 0);
   }
 
   public static String getCommmand() throws MalformedObjectNameException, IOException {

hbase-http/src/main/java/org/apache/hadoop/hbase/util/LogMonitoring.java

@@ -24,6 +24,7 @@ import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.PrintWriter;
 import java.nio.channels.FileChannel;
+import java.nio.charset.StandardCharsets;
 import java.util.Set;
 import org.apache.hadoop.hbase.logging.Log4jUtils;
 import org.apache.hadoop.io.IOUtils;
@@ -57,7 +58,7 @@ public abstract class LogMonitoring {
     try {
       FileChannel channel = fis.getChannel();
       channel.position(Math.max(0, channel.size() - tailKb * 1024));
-      r = new BufferedReader(new InputStreamReader(fis));
+      r = new BufferedReader(new InputStreamReader(fis, StandardCharsets.UTF_8));
       r.readLine(); // skip the first partial line
       String line;
       while ((line = r.readLine()) != null) {

hbase-http/src/test/java/org/apache/hadoop/hbase/http/HttpServerFunctionalTest.java

@@ -17,6 +17,9 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
@@ -29,10 +32,8 @@ import java.net.URL;
 import java.net.URLConnection;
 import java.nio.charset.StandardCharsets;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.http.HttpServer.Builder;
 import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.security.authorize.AccessControlList;
-import org.junit.Assert;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -40,7 +41,7 @@ import org.slf4j.LoggerFactory;
  * This is a base class for functional tests of the {@link HttpServer}. The methods are static for
  * other classes to import statically.
  */
-public class HttpServerFunctionalTest extends Assert {
+public class HttpServerFunctionalTest {
   private static final Logger LOG = LoggerFactory.getLogger(HttpServerFunctionalTest.class);
   /** JVM property for the webapp test dir : {@value} */
   public static final String TEST_BUILD_WEBAPPS = "test.build.webapps";
@@ -116,16 +117,14 @@ public class HttpServerFunctionalTest extends Assert {
   /**
    * Prepare the test webapp by creating the directory from the test properties fail if the
    * directory cannot be created.
+   * @throws IOException    if an error occurred
    * @throws AssertionError if a condition was not met
    */
-  protected static void prepareTestWebapp() {
+  protected static void prepareTestWebapp() throws IOException {
     String webapps = System.getProperty(TEST_BUILD_WEBAPPS, BUILD_WEBAPPS_DIR);
     File testWebappDir = new File(webapps + File.separatorChar + TEST);
-    try {
-      if (!testWebappDir.exists()) {
-        fail("Test webapp dir " + testWebappDir.getCanonicalPath() + " missing");
-      }
-    } catch (IOException e) {
+    if (!testWebappDir.exists()) {
+      fail("Test webapp dir " + testWebappDir.getCanonicalPath() + " missing");
     }
   }
 
@@ -168,7 +167,7 @@ public class HttpServerFunctionalTest extends Assert {
     return localServerBuilder(webapp).setFindPort(true).setConf(conf).setACL(adminsAcl).build();
   }
 
-  private static Builder localServerBuilder(String webapp) {
+  private static HttpServer.Builder localServerBuilder(String webapp) {
     return new HttpServer.Builder().setName(webapp).addEndpoint(URI.create("http://localhost:0"));
   }
 
@@ -232,7 +231,7 @@ public class HttpServerFunctionalTest extends Assert {
     byte[] buffer = new byte[64 * 1024];
     int len = in.read(buffer);
     while (len > 0) {
-      out.append(new String(buffer, 0, len));
+      out.append(new String(buffer, 0, len, StandardCharsets.UTF_8));
       len = in.read(buffer);
     }
     return out.toString();

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestGlobalFilter.java

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertTrue;
+
 import java.io.IOException;
 import java.util.Set;
 import java.util.TreeSet;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpServer.java

@@ -18,6 +18,10 @@
 package org.apache.hadoop.hbase.http;
 
 import static org.hamcrest.Matchers.greaterThan;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -28,6 +32,7 @@ import java.net.HttpURLConnection;
 import java.net.URI;
 import java.net.URL;
 import java.nio.CharBuffer;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -322,7 +327,8 @@ public class TestHttpServer extends HttpServerFunctionalTest {
 
   private static String readFully(final InputStream input) throws IOException {
     // TODO: when the time comes, delete me and replace with a JDK11 IO helper API.
-    try (final BufferedReader reader = new BufferedReader(new InputStreamReader(input))) {
+    try (final BufferedReader reader =
+      new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8))) {
       final StringBuilder sb = new StringBuilder();
       final CharBuffer buffer = CharBuffer.allocate(1024 * 2);
       while (reader.read(buffer) > 0) {

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpServerLifecycle.java

@@ -17,6 +17,10 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.testclassification.MiscTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
@@ -25,6 +29,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
+@Ignore("Hangs on occasion; see HBASE-14430")
 @Category({ MiscTests.class, SmallTests.class })
 public class TestHttpServerLifecycle extends HttpServerFunctionalTest {
 
@@ -51,14 +56,12 @@ public class TestHttpServerLifecycle extends HttpServerFunctionalTest {
    * Test that the server is alive once started
    * @throws Throwable on failure
    */
-  @Ignore("Hangs on occasion; see HBASE-14430")
   @Test
   public void testCreatedServerIsNotAlive() throws Throwable {
     HttpServer server = createTestServer();
     assertNotLive(server);
   }
 
-  @Ignore("Hangs on occasion; see HBASE-14430")
   @Test
   public void testStopUnstartedServer() throws Throwable {
     HttpServer server = createTestServer();
@@ -69,11 +72,9 @@ public class TestHttpServerLifecycle extends HttpServerFunctionalTest {
    * Test that the server is alive once started
    * @throws Throwable on failure
    */
-  @Ignore("Hangs on occasion; see HBASE-14430")
   @Test
   public void testStartedServerIsAlive() throws Throwable {
-    HttpServer server = null;
-    server = createTestServer();
+    HttpServer server = createTestServer();
     assertNotLive(server);
     server.start();
     assertAlive(server);
@@ -95,7 +96,6 @@ public class TestHttpServerLifecycle extends HttpServerFunctionalTest {
    * Test that the server is not alive once stopped
    * @throws Throwable on failure
    */
-  @Ignore("Hangs on occasion; see HBASE-14430")
   @Test
   public void testStoppedServerIsNotAlive() throws Throwable {
     HttpServer server = createAndStartTestServer();
@@ -108,7 +108,6 @@ public class TestHttpServerLifecycle extends HttpServerFunctionalTest {
    * Test that the server is not alive once stopped
    * @throws Throwable on failure
    */
-  @Ignore("Hangs on occasion; see HBASE-14430")
   @Test
   public void testStoppingTwiceServerIsAllowed() throws Throwable {
     HttpServer server = createAndStartTestServer();
@@ -120,15 +119,13 @@ public class TestHttpServerLifecycle extends HttpServerFunctionalTest {
   }
 
   /**
-   * Test that the server is alive once started n * on failure
+   * Test that the server is alive once started
    */
-  @Ignore("Hangs on occasion; see HBASE-14430")
   @Test
   public void testWepAppContextAfterServerStop() throws Throwable {
-    HttpServer server = null;
     String key = "test.attribute.key";
     String value = "test.attribute.value";
-    server = createTestServer();
+    HttpServer server = createTestServer();
     assertNotLive(server);
     server.start();
     server.setAttribute(key, value);

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestHttpServerWebapps.java

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.fail;
+
 import java.io.FileNotFoundException;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.testclassification.MiscTests;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestPathFilter.java

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertTrue;
+
 import java.io.IOException;
 import java.util.Set;
 import java.util.TreeSet;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestProxyUserSpnegoHttpServer.java

@@ -17,6 +17,11 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 import java.io.File;
 import java.net.HttpURLConnection;
 import java.net.URL;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSSLHttpServer.java

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertEquals;
+
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
@@ -55,9 +57,6 @@ public class TestSSLHttpServer extends HttpServerFunctionalTest {
   public static final HBaseClassTestRule CLASS_RULE =
     HBaseClassTestRule.forClass(TestSSLHttpServer.class);
 
-  private static final String BASEDIR = System.getProperty("test.build.dir", "target/test-dir")
-    + "/" + TestSSLHttpServer.class.getSimpleName();
-
   private static final Logger LOG = LoggerFactory.getLogger(TestSSLHttpServer.class);
   private static Configuration serverConf;
   private static HttpServer server;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestServletFilter.java

@@ -17,6 +17,10 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
 import java.io.IOException;
 import java.util.Random;
 import java.util.concurrent.ThreadLocalRandom;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSpnegoHttpServer.java

@@ -17,6 +17,10 @@
  */
 package org.apache.hadoop.hbase.http;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+
 import java.io.File;
 import java.io.IOException;
 import java.net.HttpURLConnection;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/jmx/TestJMXJsonServlet.java

@@ -17,6 +17,10 @@
  */
 package org.apache.hadoop.hbase.http.jmx;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.URLEncoder;

hbase-http/src/test/java/org/apache/hadoop/hbase/http/ssl/KeyStoreTestUtil.java

@@ -19,11 +19,11 @@ package org.apache.hadoop.hbase.http.ssl;
 
 import java.io.File;
 import java.io.FileOutputStream;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.io.Writer;
 import java.math.BigInteger;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 import java.security.InvalidKeyException;
 import java.security.Key;
@@ -47,6 +47,8 @@ import org.apache.hadoop.security.ssl.FileBasedKeyStoresFactory;
 import org.apache.hadoop.security.ssl.SSLFactory;
 import org.bouncycastle.x509.X509V1CertificateGenerator;
 
+import org.apache.hbase.thirdparty.com.google.common.io.Files;
+
 public final class KeyStoreTestUtil {
   private KeyStoreTestUtil() {
   }
@@ -68,6 +70,7 @@ public final class KeyStoreTestUtil {
    * @param algorithm the signing algorithm, eg "SHA1withRSA"
    * @return the self-signed certificate
    */
+  @SuppressWarnings("JavaUtilDate")
   public static X509Certificate generateCertificate(String dn, KeyPair pair, int days,
     String algorithm) throws CertificateEncodingException, InvalidKeyException,
     IllegalStateException, NoSuchProviderException, NoSuchAlgorithmException, SignatureException {
@@ -369,11 +372,8 @@ public final class KeyStoreTestUtil {
    * @throws IOException if there is an I/O error saving the file
    */
   public static void saveConfig(File file, Configuration conf) throws IOException {
-    Writer writer = new FileWriter(file);
-    try {
+    try (Writer writer = Files.newWriter(file, StandardCharsets.UTF_8)) {
       conf.writeXml(writer);
-    } finally {
-      writer.close();
     }
   }
 }