This is an automated email from the ASF dual-hosted git repository. kgyrtkirk pushed a commit to branch branch-3 in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/branch-3 by this push: new 1087c4e HIVE-25844: Exception deserialization error-s may cause beeline to terminate immediately (backport HIVE-24772) (#2918) (Zoltan Haindrich reviewed by Krisztian Kasa) 1087c4e is described below commit 1087c4e14a2668eab914939f1f92dae2f049cf7e Author: Zoltan Haindrich <k...@rxd.hu> AuthorDate: Tue Feb 22 15:29:52 2022 +0100 HIVE-25844: Exception deserialization error-s may cause beeline to terminate immediately (backport HIVE-24772) (#2918) (Zoltan Haindrich reviewed by Krisztian Kasa) --- .../apache/hive/minikdc/TestJdbcWithMiniKdc.java | 28 +--- .../apache/hive/service/cli/HiveSQLException.java | 167 +++++---------------- .../hive/service/cli/thrift/ThriftCLIService.java | 67 +++++---- .../hive/service/cli/TestHiveSQLException.java | 112 +------------- 4 files changed, 79 insertions(+), 295 deletions(-) diff --git a/itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestJdbcWithMiniKdc.java b/itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestJdbcWithMiniKdc.java index e526b48..353d157 100644 --- a/itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestJdbcWithMiniKdc.java +++ b/itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestJdbcWithMiniKdc.java @@ -221,22 +221,15 @@ public class TestJdbcWithMiniKdc { * it's not allowed to impersonate * @throws Exception */ - @Test + @Test(expected = HiveSQLException.class) public void testNegativeTokenAuth() throws Exception { miniHiveKdc.loginUser(MiniHiveKdc.HIVE_TEST_SUPER_USER); hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL()); try { // retrieve token and store in the cache - String token = ((HiveConnection)hs2Conn).getDelegationToken( + ((HiveConnection)hs2Conn).getDelegationToken( MiniHiveKdc.HIVE_TEST_USER_2, MiniHiveKdc.HIVE_SERVICE_PRINCIPAL); - - fail(MiniHiveKdc.HIVE_TEST_SUPER_USER + " shouldn't be allowed to retrieve token for " + - MiniHiveKdc.HIVE_TEST_USER_2); - } catch (SQLException e) { - // Expected error - assertEquals("Unexpected type of exception class thrown", HiveSQLException.class, e.getClass()); - assertTrue(e.getCause().getCause().getMessage().contains("is not allowed to impersonate")); } finally { hs2Conn.close(); } @@ -260,22 +253,11 @@ public class TestJdbcWithMiniKdc { * impersonate the given user * @throws Exception */ - @Test + @Test(expected = SQLException.class) public void testNegativeProxyAuth() throws Exception { miniHiveKdc.loginUser(MiniHiveKdc.HIVE_TEST_SUPER_USER); - try { - hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL("default", - ";hive.server2.proxy.user=" + MiniHiveKdc.HIVE_TEST_USER_2)); - verifyProperty(SESSION_USER_NAME, MiniHiveKdc.HIVE_TEST_USER_2); - fail(MiniHiveKdc.HIVE_TEST_SUPER_USER + " shouldn't be allowed proxy connection for " - + MiniHiveKdc.HIVE_TEST_USER_2); - } catch (SQLException e) { - // Expected error - e.printStackTrace(); - assertTrue(e.getMessage().contains("Failed to validate proxy privilege")); - assertTrue(e.getCause().getCause().getCause().getMessage() - .contains("is not allowed to impersonate")); - } + hs2Conn = DriverManager + .getConnection(miniHS2.getJdbcURL("default", ";hive.server2.proxy.user=" + MiniHiveKdc.HIVE_TEST_USER_2)); } /** diff --git a/service/src/java/org/apache/hive/service/cli/HiveSQLException.java b/service/src/java/org/apache/hive/service/cli/HiveSQLException.java index 5f9ff43..f180830 100644 --- a/service/src/java/org/apache/hive/service/cli/HiveSQLException.java +++ b/service/src/java/org/apache/hive/service/cli/HiveSQLException.java @@ -19,39 +19,52 @@ package org.apache.hive.service.cli; import java.sql.SQLException; -import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.apache.hive.service.rpc.thrift.TStatus; import org.apache.hive.service.rpc.thrift.TStatusCode; +import com.google.common.annotations.VisibleForTesting; + /** - * HiveSQLException. - * + * An exception that provides information on a Hive access error or other + * errors. */ public class HiveSQLException extends SQLException { - /** - * - */ - private static final long serialVersionUID = -6095254671958748094L; + private static final long serialVersionUID = -6095254671958748095L; + + @VisibleForTesting + public static final List<String> DEFAULT_INFO = + Collections.singletonList("Server-side error; please check HS2 logs."); /** - * + * Constructor. */ public HiveSQLException() { super(); } /** - * @param reason + * Constructs a SQLException object with a given reason. The SQLState is + * initialized to null and the vendor code is initialized to 0. The cause is + * not initialized, and may subsequently be initialized by a call to the + * Throwable.initCause(java.lang.Throwable) method. + * + * @param reason a description of the exception */ public HiveSQLException(String reason) { super(reason); } /** - * @param cause + * Constructs a SQLException object with a given cause. The SQLState is + * initialized to null and the vendor code is initialized to 0. The reason is + * initialized to null if cause==null or to cause.toString() if cause!=null. + * + * @param cause the underlying reason for this SQLException - may be null + * indicating the cause is non-existent or unknown */ public HiveSQLException(Throwable cause) { super(cause); @@ -102,18 +115,13 @@ public class HiveSQLException extends SQLException { } public HiveSQLException(TStatus status) { - // TODO: set correct vendorCode field super(status.getErrorMessage(), status.getSqlState(), status.getErrorCode()); - if (status.getInfoMessages() != null) { - initCause(toCause(status.getInfoMessages())); - } } - - -/** - * Converts current object to a {@link TStatus} object - * @return a {@link TStatus} object + /** + * Converts current object to a {@link TStatus} object. + * + * @return a {@link TStatus} object */ public TStatus toTStatus() { // TODO: convert sqlState, etc. @@ -121,130 +129,25 @@ public class HiveSQLException extends SQLException { tStatus.setSqlState(getSQLState()); tStatus.setErrorCode(getErrorCode()); tStatus.setErrorMessage(getMessage()); - tStatus.setInfoMessages(toString(this)); + tStatus.setInfoMessages(DEFAULT_INFO); return tStatus; } /** - * Converts the specified {@link Exception} object into a {@link TStatus} object - * @param e a {@link Exception} object - * @return a {@link TStatus} object + * Converts the specified {@link Exception} object into a {@link TStatus} + * object. + * + * @param e a {@link Exception} object + * @return a {@link TStatus} object */ public static TStatus toTStatus(Exception e) { if (e instanceof HiveSQLException) { - return ((HiveSQLException)e).toTStatus(); + return ((HiveSQLException) e).toTStatus(); } TStatus tStatus = new TStatus(TStatusCode.ERROR_STATUS); tStatus.setErrorMessage(e.getMessage()); - tStatus.setInfoMessages(toString(e)); + tStatus.setInfoMessages(DEFAULT_INFO); return tStatus; } - /** - * Converts a {@link Throwable} object into a flattened list of texts including its stack trace - * and the stack traces of the nested causes. - * @param ex a {@link Throwable} object - * @return a flattened list of texts including the {@link Throwable} object's stack trace - * and the stack traces of the nested causes. - */ - public static List<String> toString(Throwable ex) { - return toString(ex, null); - } - - private static List<String> toString(Throwable cause, StackTraceElement[] parent) { - StackTraceElement[] trace = cause.getStackTrace(); - int m = trace.length - 1; - if (parent != null) { - int n = parent.length - 1; - while (m >= 0 && n >= 0 && trace[m].equals(parent[n])) { - m--; n--; - } - } - List<String> detail = enroll(cause, trace, m); - cause = cause.getCause(); - if (cause != null) { - detail.addAll(toString(cause, trace)); - } - return detail; - } - - private static List<String> enroll(Throwable ex, StackTraceElement[] trace, int max) { - List<String> details = new ArrayList<String>(); - StringBuilder builder = new StringBuilder(); - builder.append('*').append(ex.getClass().getName()).append(':'); - builder.append(ex.getMessage()).append(':'); - builder.append(trace.length).append(':').append(max); - details.add(builder.toString()); - for (int i = 0; i <= max; i++) { - builder.setLength(0); - builder.append(trace[i].getClassName()).append(':'); - builder.append(trace[i].getMethodName()).append(':'); - String fileName = trace[i].getFileName(); - builder.append(fileName == null ? "" : fileName).append(':'); - builder.append(trace[i].getLineNumber()); - details.add(builder.toString()); - } - return details; - } - - /** - * Converts a flattened list of texts including the stack trace and the stack - * traces of the nested causes into a {@link Throwable} object. - * @param details a flattened list of texts including the stack trace and the stack - * traces of the nested causes - * @return a {@link Throwable} object - */ - public static Throwable toCause(List<String> details) { - return toStackTrace(details, null, 0); - } - - private static Throwable toStackTrace(List<String> details, StackTraceElement[] parent, int index) { - String detail = details.get(index++); - if (!detail.startsWith("*")) { - return null; // should not be happened. ignore remaining - } - int i1 = detail.indexOf(':'); - int i3 = detail.lastIndexOf(':'); - int i2 = detail.substring(0, i3).lastIndexOf(':'); - String exceptionClass = detail.substring(1, i1); - String exceptionMessage = detail.substring(i1 + 1, i2); - Throwable ex = newInstance(exceptionClass, exceptionMessage); - - int length = Integer.parseInt(detail.substring(i2 + 1, i3)); - int unique = Integer.parseInt(detail.substring(i3 + 1)); - - int i = 0; - StackTraceElement[] trace = new StackTraceElement[length]; - for (; i <= unique; i++) { - detail = details.get(index++); - int j1 = detail.indexOf(':'); - int j3 = detail.lastIndexOf(':'); - int j2 = detail.substring(0, j3).lastIndexOf(':'); - String className = detail.substring(0, j1); - String methodName = detail.substring(j1 + 1, j2); - String fileName = detail.substring(j2 + 1, j3); - if (fileName.isEmpty()) { - fileName = null; - } - int lineNumber = Integer.parseInt(detail.substring(j3 + 1)); - trace[i] = new StackTraceElement(className, methodName, fileName, lineNumber); - } - int common = trace.length - i; - if (common > 0) { - System.arraycopy(parent, parent.length - common, trace, trace.length - common, common); - } - if (details.size() > index) { - ex.initCause(toStackTrace(details, trace, index)); - } - ex.setStackTrace(trace); - return ex; - } - - private static Throwable newInstance(String className, String message) { - try { - return (Throwable)Class.forName(className).getConstructor(String.class).newInstance(message); - } catch (Exception e) { - return new RuntimeException(className + ":" + message); - } - } } diff --git a/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java b/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java index 5481b90..9dbf494 100644 --- a/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java +++ b/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java @@ -124,7 +124,7 @@ import org.slf4j.LoggerFactory; */ public abstract class ThriftCLIService extends AbstractService implements TCLIService.Iface, Runnable { - public static final Logger LOG = LoggerFactory.getLogger(ThriftCLIService.class.getName()); + protected static final Logger LOG = LoggerFactory.getLogger(ThriftCLIService.class); protected CLIService cliService; private static final TStatus OK_STATUS = new TStatus(TStatusCode.SUCCESS_STATUS); @@ -259,7 +259,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setDelegationToken(token); resp.setStatus(OK_STATUS); } catch (HiveSQLException e) { - LOG.error("Error obtaining delegation token", e); + LOG.error("Failed to get delegation token [request: {}]", req, e); TStatus tokenErrorStatus = HiveSQLException.toTStatus(e); tokenErrorStatus.setSqlState("42000"); resp.setStatus(tokenErrorStatus); @@ -281,7 +281,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe hiveAuthFactory, req.getDelegationToken()); resp.setStatus(OK_STATUS); } catch (HiveSQLException e) { - LOG.error("Error canceling delegation token", e); + LOG.error("Failed to cancel delegation token [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } } @@ -300,7 +300,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe hiveAuthFactory, req.getDelegationToken()); resp.setStatus(OK_STATUS); } catch (HiveSQLException e) { - LOG.error("Error obtaining renewing token", e); + LOG.error("Failed to renew delegation token [request: {}]", e); resp.setStatus(HiveSQLException.toTStatus(e)); } } @@ -318,7 +318,9 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe public TOpenSessionResp OpenSession(TOpenSessionReq req) throws TException { LOG.info("Client protocol version: " + req.getClient_protocol()); TOpenSessionResp resp = new TOpenSessionResp(); + String userName = null; try { + userName = getUserName(req); SessionHandle sessionHandle = getSessionHandle(req, resp); resp.setSessionHandle(sessionHandle.toTSessionHandle()); Map<String, String> configurationMap = new HashMap<String, String>(); @@ -337,7 +339,8 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe context.setSessionHandle(sessionHandle); } } catch (Exception e) { - LOG.warn("Error opening session: ", e); + // Do not log request as it contains password information + LOG.error("Login attempt failed for user : {}", userName, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -363,7 +366,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe try { cliService.setApplicationName(sh, e.getValue()); } catch (Exception ex) { - LOG.warn("Error setting application name", ex); + LOG.error("Failed setting application name", ex); resp = new TSetClientInfoResp(HiveSQLException.toTStatus(ex)); } } @@ -392,7 +395,6 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe clientIpAddress = TSetIpAddressProcessor.getUserIpAddress(); } } - LOG.debug("Client's IP Address: " + clientIpAddress); return clientIpAddress; } @@ -428,7 +430,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe userName = getShortName(userName); String effectiveClientUser = getProxyUser(userName, req.getConfiguration(), getIpAddress()); - LOG.debug("Client's username: " + effectiveClientUser); + LOG.debug("Client's username: {}", effectiveClientUser); return effectiveClientUser; } @@ -463,6 +465,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe throws HiveSQLException, LoginException, IOException { String userName = getUserName(req); String ipAddress = getIpAddress(); + LOG.info("Creating Hive session handle for user [{}] from IP {}", userName, ipAddress); TProtocolVersion protocol = getMinVersion(CLIService.SERVER_VERSION, req.getClient_protocol()); SessionHandle sessionHandle; @@ -480,7 +483,8 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe } private double getProgressedPercentage(OperationHandle opHandle) throws HiveSQLException { - checkArgument(OperationType.EXECUTE_STATEMENT.equals(opHandle.getOperationType())); + checkArgument(OperationType.EXECUTE_STATEMENT == opHandle.getOperationType()); + Operation operation = cliService.getSessionManager().getOperationManager().getOperation(opHandle); SessionState state = operation.getParentSession().getSessionState(); ProgressMonitor monitor = state.getProgressMonitor(); @@ -527,7 +531,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe context.setSessionHandle(null); } } catch (Exception e) { - LOG.warn("Error closing session: ", e); + LOG.error("Failed to close the session", e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -543,7 +547,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setInfoValue(getInfoValue.toTGetInfoValue()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting info: ", e); + LOG.error("Failed to get info", e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -568,7 +572,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe // Note: it's rather important that this (and other methods) catch Exception, not Throwable; // in combination with HiveSessionProxy.invoke code, perhaps unintentionally, it used // to also catch all errors; and now it allows OOMs only to propagate. - LOG.warn("Error executing statement: ", e); + LOG.error("Failed to execute statement [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -582,7 +586,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(operationHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting type info: ", e); + LOG.error("Failed to get type info [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -596,7 +600,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting catalogs: ", e); + LOG.error("Failed getting catalogs [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -611,7 +615,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting schemas: ", e); + LOG.error("Failed to get schemas [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -627,7 +631,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting tables: ", e); + LOG.error("Failed to get tables [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -641,7 +645,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting table types: ", e); + LOG.error("Failed to get table types [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -660,7 +664,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting columns: ", e); + LOG.error("Failed to get column types [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -676,7 +680,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting functions: ", e); + LOG.error("Failed to get function: [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -726,7 +730,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe } resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting operation status: ", e); + LOG.error("Failed to get operation status [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -739,7 +743,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe cliService.cancelOperation(new OperationHandle(req.getOperationHandle())); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error cancelling operation: ", e); + LOG.error("Failed to get cancel operation [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -752,7 +756,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe cliService.closeOperation(new OperationHandle(req.getOperationHandle())); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error closing operation: ", e); + LOG.error("Failed to close operation [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -767,7 +771,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setSchema(schema.toTTableSchema()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting result set metadata: ", e); + LOG.error("Failed to get result set metadata [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -792,7 +796,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setHasMoreRows(false); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error fetching results: ", e); + LOG.error("Failed fetch results [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -809,7 +813,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting functions: ", e); + LOG.error("Failed to get primary keys [request: {}]", req, e); resp.setStatus(HiveSQLException.toTStatus(e)); } return resp; @@ -827,9 +831,9 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe resp.setOperationHandle(opHandle.toTOperationHandle()); resp.setStatus(OK_STATUS); } catch (Exception e) { - LOG.warn("Error getting functions: ", e); - resp.setStatus(HiveSQLException.toTStatus(e)); - } + LOG.error("Failed to get cross reference [request: {}]", req, e); + resp.setStatus(HiveSQLException.toTStatus(e)); + } return resp; } @@ -838,6 +842,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe try { return new TGetQueryIdResp(cliService.getQueryId(req.getOperationHandle())); } catch (HiveSQLException e) { + LOG.error("Failed to get query ID [request: {}]", req, e); throw new TException(e); } } @@ -861,12 +866,12 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe if (cliService.getHiveConf().getVar( ConfVars.HIVE_SERVER2_TRANSPORT_MODE).equalsIgnoreCase("http")) { proxyUser = SessionManager.getProxyUserName(); - LOG.debug("Proxy user from query string: " + proxyUser); + LOG.debug("Proxy user from query string: {}", proxyUser); } if (proxyUser == null && sessionConf != null && sessionConf.containsKey(HiveAuthConstants.HS2_PROXY_USER)) { String proxyUserFromThriftBody = sessionConf.get(HiveAuthConstants.HS2_PROXY_USER); - LOG.debug("Proxy user from thrift body: " + proxyUserFromThriftBody); + LOG.debug("Proxy user from thrift body: {}", proxyUserFromThriftBody); proxyUser = proxyUserFromThriftBody; } @@ -887,7 +892,7 @@ public abstract class ThriftCLIService extends AbstractService implements TCLISe // Verify proxy user privilege of the realUser for the proxyUser HiveAuthFactory.verifyProxyAccess(realUser, proxyUser, ipAddress, hiveConf); - LOG.debug("Verified proxy user: " + proxyUser); + LOG.debug("Verified proxy user: {}", proxyUser); return proxyUser; } } diff --git a/service/src/test/org/apache/hive/service/cli/TestHiveSQLException.java b/service/src/test/org/apache/hive/service/cli/TestHiveSQLException.java index ac7fec0..7e774ad 100644 --- a/service/src/test/org/apache/hive/service/cli/TestHiveSQLException.java +++ b/service/src/test/org/apache/hive/service/cli/TestHiveSQLException.java @@ -20,11 +20,10 @@ package org.apache.hive.service.cli; import java.util.List; -import junit.framework.Assert; - import org.apache.commons.lang.StringUtils; import org.apache.hive.service.rpc.thrift.TStatus; import org.apache.hive.service.rpc.thrift.TStatusCode; +import org.junit.Assert; import org.junit.Test; public class TestHiveSQLException { @@ -40,7 +39,7 @@ public class TestHiveSQLException { Assert.assertEquals(TStatusCode.ERROR_STATUS, status.getStatusCode()); Assert.assertEquals(ex1.getMessage(), status.getErrorMessage()); - Assert.assertEquals(HiveSQLException.toString(ex1), status.getInfoMessages()); + Assert.assertEquals(HiveSQLException.DEFAULT_INFO, status.getInfoMessages()); } /** @@ -58,109 +57,7 @@ public class TestHiveSQLException { Assert.assertEquals(TStatusCode.ERROR_STATUS, status.getStatusCode()); Assert.assertEquals(expectedSqlState, status.getSqlState()); Assert.assertEquals(expectedMessage, status.getErrorMessage()); - Assert.assertEquals(HiveSQLException.toString(ex1), status.getInfoMessages()); - } - - /** - * Tests the conversion between the exception text with the simple cause and the - * Throwable object - */ - @Test - public void testExceptionMarshalling() throws Exception { - Exception ex1 = createException(); - ex1.initCause(createSimpleCause()); - Throwable ex = HiveSQLException.toCause(HiveSQLException.toString(ex1)); - - Assert.assertSame(RuntimeException.class, ex.getClass()); - Assert.assertEquals("exception1", ex.getMessage()); - Assert.assertSame(UnsupportedOperationException.class, ex.getCause().getClass()); - Assert.assertEquals("exception2", ex.getCause().getMessage()); - } - - /** - * Tests the conversion between the exception text with nested cause and - * the Throwable object - */ - @Test - public void testNestedException() { - Exception ex1 = createException(); - ex1.initCause(createNestedCause()); - Throwable ex = HiveSQLException.toCause(HiveSQLException.toString(ex1)); - - Assert.assertSame(RuntimeException.class, ex.getClass()); - Assert.assertEquals("exception1", ex.getMessage()); - - Assert.assertSame(UnsupportedOperationException.class, ex.getCause().getClass()); - Assert.assertEquals("exception2", ex.getCause().getMessage()); - - Assert.assertSame(Exception.class, ex.getCause().getCause().getClass()); - Assert.assertEquals("exception3", ex.getCause().getCause().getMessage()); - } - - /** - * Tests the conversion of the exception with unknown source - */ - @Test - public void testExceptionWithUnknownSource() { - Exception ex1 = createException(); - ex1.initCause(createSimpleCause()); - List<String> details = HiveSQLException.toString(ex1); - - // Simulate the unknown source - String[] tokens = details.get(1).split(":"); - tokens[2] = null; - tokens[3] = "-1"; - details.set(1, StringUtils.join(tokens, ":")); - - Throwable ex = HiveSQLException.toCause(details); - - Assert.assertSame(RuntimeException.class, ex.getClass()); - Assert.assertEquals("exception1", ex.getMessage()); - Assert.assertSame(UnsupportedOperationException.class, ex.getCause().getClass()); - Assert.assertEquals("exception2", ex.getCause().getMessage()); - } - - /** - * Tests the conversion of the exception that the class type of one of the causes - * doesn't exist. The stack trace text is generated on the server and passed to JDBC - * client. It's possible that some cause types don't exist on the client and HiveSQLException - * can't convert them and use RunTimeException instead. - */ - @Test - public void testExceptionWithMissingTypeOnClient() { - Exception ex1 = new UnsupportedOperationException(); - ex1.initCause(createSimpleCause()); - List<String> details = HiveSQLException.toString(ex1); - - // Simulate an unknown type - String[] tokens = details.get(0).split(":"); - tokens[0] = "*DummyException"; - details.set(0, StringUtils.join(tokens, ":")); - - Throwable ex = HiveSQLException.toCause(details); - Assert.assertEquals(RuntimeException.class, ex.getClass()); - } - - /** - * Tests the conversion of the exception from anonymous class - */ - @Test - public void testExceptionFromAnonymousClass() { - Dummy d = new Dummy() { - - public void testExceptionConversion() { - Exception ex1 = createException(); - ex1.initCause(createSimpleCause()); - Throwable ex = HiveSQLException.toCause(HiveSQLException.toString(ex1)); - - Assert.assertSame(RuntimeException.class, ex.getClass()); - Assert.assertEquals("exception1", ex.getMessage()); - Assert.assertSame(UnsupportedOperationException.class, ex.getCause().getClass()); - Assert.assertEquals("exception2", ex.getCause().getMessage()); - } - }; - - d.testExceptionConversion(); + Assert.assertEquals(HiveSQLException.DEFAULT_INFO, status.getInfoMessages()); } interface Dummy { @@ -175,7 +72,4 @@ public class TestHiveSQLException { return new UnsupportedOperationException("exception2"); } - private static Exception createNestedCause() { - return new UnsupportedOperationException("exception2", new Exception("exception3")); - } }