Add test and validation of error response

Author: bbeaudreault

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

@@ -22,8 +22,8 @@
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.PrintWriter;
+import java.net.HttpURLConnection;
 import java.net.URL;
-import java.net.URLConnection;
 import java.nio.charset.StandardCharsets;
 import java.util.Objects;
 import java.util.regex.Pattern;
@@ -41,6 +41,7 @@
 import org.apache.hadoop.security.authentication.client.AuthenticatedURL;
 import org.apache.hadoop.security.authentication.client.KerberosAuthenticator;
 import org.apache.hadoop.security.ssl.SSLFactory;
+import org.apache.hadoop.util.HttpExceptionUtils;
 import org.apache.hadoop.util.ServletUtil;
 import org.apache.hadoop.util.Tool;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -59,6 +60,8 @@ public final class LogLevel {
   public static final String PROTOCOL_HTTP = "http";
   public static final String PROTOCOL_HTTPS = "https";
 
+  public static final String READONLY_LOGGERS_CONF_KEY = "hbase.ui.logLevels.readonly.loggers";
+
   /**
    * A command line implementation
    */
@@ -247,11 +250,11 @@ private void doSetLevel() throws Exception {
      * @return a connected connection
      * @throws Exception if it can not establish a connection.
      */
-    private URLConnection connect(URL url) throws Exception {
+    private HttpURLConnection connect(URL url) throws Exception {
       AuthenticatedURL.Token token = new AuthenticatedURL.Token();
       AuthenticatedURL aUrl;
       SSLFactory clientSslFactory;
-      URLConnection connection;
+      HttpURLConnection connection;
       // If https is chosen, configures SSL client.
       if (PROTOCOL_HTTPS.equals(url.getProtocol())) {
         clientSslFactory = new SSLFactory(SSLFactory.Mode.CLIENT, this.getConf());
@@ -280,7 +283,9 @@ private void process(String urlString) throws Exception {
       URL url = new URL(urlString);
       System.out.println("Connecting to " + url);
 
-      URLConnection connection = connect(url);
+      HttpURLConnection connection = connect(url);
+
+      HttpExceptionUtils.validateResponse(connection, 200);
 
       // read from the servlet
 
@@ -317,8 +322,10 @@ public void doGet(HttpServletRequest request, HttpServletResponse response)
       Configuration conf = (Configuration) getServletContext().getAttribute(
           HttpServer.CONF_CONTEXT_ATTRIBUTE);
       if (conf.getBoolean("hbase.master.ui.readonly", false)) {
-        response.sendError(HttpServletResponse.SC_FORBIDDEN, "Modification of HBase via"
-            + " the UI is disallowed in configuration.");
+        sendError(
+          response,
+          HttpServletResponse.SC_FORBIDDEN,
+          "Modification of HBase via the UI is disallowed in configuration.");
         return;
       }
       response.setContentType("text/html");
@@ -336,7 +343,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response)
       String logName = ServletUtil.getParameter(request, "log");
       String level = ServletUtil.getParameter(request, "level");
 
-      String[] readOnlyLogLevels = conf.getStrings("hbase.ui.logLevels.readonly.loggers");
+      String[] readOnlyLogLevels = conf.getStrings(READONLY_LOGGERS_CONF_KEY);
 
       if (logName != null) {
         out.println("<p>Results:</p>");
@@ -348,8 +355,10 @@ public void doGet(HttpServletRequest request, HttpServletResponse response)
             + "Log Class: <b>" + log.getClass().getName() +"</b><br />");
         if (level != null) {
           if (!isLogLevelChangeAllowed(logName, readOnlyLogLevels)) {
-            response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED,
-              "Modification of logger " + logName + " not allowed.");
+            sendError(
+              response,
+              HttpServletResponse.SC_PRECONDITION_FAILED,
+              "Modification of logger " + logName + " is disallowed in configuration.");
             return;
           }
 
@@ -373,13 +382,19 @@ private boolean isLogLevelChangeAllowed(String logger, String[] readOnlyLogLevel
         return true;
       }
       for (String readOnlyLogLevel : readOnlyLogLevels) {
-        if (readOnlyLogLevel.startsWith(logger)) {
+        if (logger.startsWith(readOnlyLogLevel)) {
           return false;
         }
       }
       return true;
     }
 
+    private void sendError(HttpServletResponse response, int code, String message)
+      throws IOException {
+      response.setStatus(code, message);
+      response.sendError(code, message);
+    }
+
     static final String FORMS = "<div class='container-fluid content'>\n"
         + "<div class='row inner_header'>\n" + "<div class='page-header'>\n"
         + "<h1>Get/Set Log Level</h1>\n" + "</div>\n" + "</div>\n" + "Actions:" + "<p>"

hbase-http/src/test/java/org/apache/hadoop/hbase/http/log/TestLogLevel.java

@@ -24,12 +24,14 @@
 import static org.junit.Assert.fail;
 
 import java.io.File;
+import java.io.IOException;
 import java.net.BindException;
 import java.net.SocketException;
 import java.net.URI;
 import java.security.PrivilegedExceptionAction;
 import java.util.Properties;
 import javax.net.ssl.SSLException;
+import javax.servlet.http.HttpServletResponse;
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.HadoopIllegalArgumentException;
 import org.apache.hadoop.conf.Configuration;
@@ -73,6 +75,8 @@ public class TestLogLevel {
   private static Configuration clientConf;
   private static Configuration sslConf;
   private static final String logName = TestLogLevel.class.getName();
+  private static final String protectedPrefix = "protected";
+  private static final String protectedLogName = protectedPrefix + "." + logName;
   private static final org.apache.logging.log4j.Logger log =
     org.apache.logging.log4j.LogManager.getLogger(logName);
   private final static String PRINCIPAL = "loglevel.principal";
@@ -89,6 +93,7 @@ public class TestLogLevel {
   @BeforeClass
   public static void setUp() throws Exception {
     serverConf = new Configuration();
+    serverConf.setStrings(LogLevel.READONLY_LOGGERS_CONF_KEY, protectedPrefix);
     HTU = new HBaseCommonTestingUtil(serverConf);
 
     File keystoreDir = new File(HTU.getDataTestDir("keystore").toString());
@@ -259,9 +264,17 @@ private HttpServer createServer(String protocol, boolean isSpnego) throws Except
   private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol,
     final boolean isSpnego) throws Exception {
     testDynamicLogLevel(bindProtocol, connectProtocol, isSpnego,
+      logName,
       org.apache.logging.log4j.Level.DEBUG.toString());
   }
 
+  private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol,
+    final boolean isSpnego, final String newLevel) throws Exception {
+    testDynamicLogLevel(bindProtocol, connectProtocol, isSpnego,
+      logName,
+      newLevel);
+  }
+
   /**
    * Run both client and server using the given protocol.
    * @param bindProtocol specify either http or https for server
@@ -270,13 +283,14 @@ private void testDynamicLogLevel(final String bindProtocol, final String connect
    * @throws Exception if client can't accesss server.
    */
   private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol,
-    final boolean isSpnego, final String newLevel) throws Exception {
+    final boolean isSpnego, final String loggerName, final String newLevel) throws Exception {
     if (!LogLevel.isValidProtocol(bindProtocol)) {
       throw new Exception("Invalid server protocol " + bindProtocol);
     }
     if (!LogLevel.isValidProtocol(connectProtocol)) {
       throw new Exception("Invalid client protocol " + connectProtocol);
     }
+    org.apache.logging.log4j.Logger log = org.apache.logging.log4j.LogManager.getLogger(loggerName);
     org.apache.logging.log4j.Level oldLevel = log.getLevel();
     assertNotEquals("Get default Log Level which shouldn't be ERROR.",
       org.apache.logging.log4j.Level.ERROR, oldLevel);
@@ -305,8 +319,8 @@ private void testDynamicLogLevel(final String bindProtocol, final String connect
     try {
       clientUGI.doAs((PrivilegedExceptionAction<Void>) () -> {
         // client command line
-        getLevel(connectProtocol, authority);
-        setLevel(connectProtocol, authority, newLevel);
+        getLevel(connectProtocol, authority, loggerName);
+        setLevel(connectProtocol, authority, loggerName, newLevel);
         return null;
       });
     } finally {
@@ -324,7 +338,7 @@ private void testDynamicLogLevel(final String bindProtocol, final String connect
    * @param authority daemon's web UI address
    * @throws Exception if unable to connect
    */
-  private void getLevel(String protocol, String authority) throws Exception {
+  private void getLevel(String protocol, String authority, String logName) throws Exception {
     String[] getLevelArgs = { "-getlevel", authority, logName, "-protocol", protocol };
     CLI cli = new CLI(protocol.equalsIgnoreCase("https") ? sslConf : clientConf);
     cli.run(getLevelArgs);
@@ -336,13 +350,27 @@ private void getLevel(String protocol, String authority) throws Exception {
    * @param authority daemon's web UI address
    * @throws Exception if unable to run or log level does not change as expected
    */
-  private void setLevel(String protocol, String authority, String newLevel) throws Exception {
+  private void setLevel(String protocol, String authority, String logName, String newLevel) throws Exception {
     String[] setLevelArgs = { "-setlevel", authority, logName, newLevel, "-protocol", protocol };
     CLI cli = new CLI(protocol.equalsIgnoreCase("https") ? sslConf : clientConf);
     cli.run(setLevelArgs);
 
+    org.apache.logging.log4j.Logger logger = org.apache.logging.log4j.LogManager.getLogger(logName);
+
     assertEquals("new level not equal to expected: ", newLevel.toUpperCase(),
-      log.getLevel().toString());
+      logger.getLevel().toString());
+  }
+
+  @Test
+  public void testSettingProtectedLogLevel() throws Exception {
+    try {
+      testDynamicLogLevel(LogLevel.PROTOCOL_HTTP, LogLevel.PROTOCOL_HTTP, true, protectedLogName,
+        "DEBUG");
+      fail("Expected IO exception due to protected logger");
+    } catch (IOException e) {
+      assertTrue(e.getMessage().contains("" + HttpServletResponse.SC_PRECONDITION_FAILED));
+      assertTrue(e.getMessage().contains("Modification of logger " + protectedLogName + " is disallowed in configuration."));
+    }
   }
 
   /**