Repository: hive Updated Branches: refs/heads/master e1e497574 -> f2172cdbc
HIVE-19228: Remove commons-httpclient 3.x usage (Janaki Lahorani reviewed by Aihua Xu) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/f2172cdb Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/f2172cdb Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/f2172cdb Branch: refs/heads/master Commit: f2172cdbc3af92e18db68c3daac44a73e92eaf48 Parents: e1e4975 Author: Aihua Xu <aihu...@apache.org> Authored: Wed May 9 10:56:32 2018 -0700 Committer: Aihua Xu <aihu...@apache.org> Committed: Wed May 9 11:27:45 2018 -0700 ---------------------------------------------------------------------- .../apache/hive/jdbc/TestActivePassiveHA.java | 99 +++++++++++++------- pom.xml | 6 -- ql/pom.xml | 15 --- .../hive/ql/parse/LoadSemanticAnalyzer.java | 16 +++- .../apache/hive/service/server/HiveServer2.java | 52 ++++++---- 5 files changed, 111 insertions(+), 77 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/f2172cdb/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestActivePassiveHA.java ---------------------------------------------------------------------- diff --git a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestActivePassiveHA.java b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestActivePassiveHA.java index c55271f..4055f13 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestActivePassiveHA.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestActivePassiveHA.java @@ -36,11 +36,6 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.TimeUnit; -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.HttpMethodBase; -import org.apache.commons.httpclient.methods.DeleteMethod; -import org.apache.commons.httpclient.methods.GetMethod; -import org.apache.commons.httpclient.methods.OptionsMethod; import org.apache.curator.test.TestingServer; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -54,9 +49,22 @@ import org.apache.hive.service.server.HS2ActivePassiveHARegistryClient; import org.apache.hive.service.server.HiveServer2Instance; import org.apache.hive.service.server.TestHS2HttpServerPam; import org.apache.hive.service.servlet.HS2Peers; +import org.apache.http.Header; import org.apache.http.HttpException; import org.apache.http.HttpHeaders; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpOptions; +import org.apache.http.client.methods.HttpRequestBase; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.StatusLine; +import org.apache.http.util.EntityUtils; import org.codehaus.jackson.map.ObjectMapper; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.util.B64Code; +import org.eclipse.jetty.util.StringUtil; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -406,7 +414,7 @@ public class TestActivePassiveHA { assertEquals("true", sendGet(url1, true)); // trigger failover on miniHS2_1 without authorization header - assertEquals("Unauthorized", sendDelete(url1, false)); + assertTrue(sendDelete(url1, false).contains("Unauthorized")); assertTrue(sendDelete(url1, true).contains("Failover successful!")); assertEquals(true, miniHS2_1.getNotLeaderTestFuture().get()); assertEquals(false, miniHS2_1.isLeader()); @@ -541,56 +549,79 @@ public class TestActivePassiveHA { } private String sendGet(String url, boolean enableAuth) throws Exception { - return sendAuthMethod(new GetMethod(url), enableAuth, false); + return sendAuthMethod(new HttpGet(url), enableAuth, false); } private String sendGet(String url, boolean enableAuth, boolean enableCORS) throws Exception { - return sendAuthMethod(new GetMethod(url), enableAuth, enableCORS); + return sendAuthMethod(new HttpGet(url), enableAuth, enableCORS); } private String sendDelete(String url, boolean enableAuth) throws Exception { - return sendAuthMethod(new DeleteMethod(url), enableAuth, false); + return sendAuthMethod(new HttpDelete(url), enableAuth, false); } private String sendDelete(String url, boolean enableAuth, boolean enableCORS) throws Exception { - return sendAuthMethod(new DeleteMethod(url), enableAuth, enableCORS); + return sendAuthMethod(new HttpDelete(url), enableAuth, enableCORS); } - private String sendAuthMethod(HttpMethodBase method, boolean enableAuth, boolean enableCORS) throws Exception { - HttpClient client = new HttpClient(); - try { - if (enableAuth) { - setupAuthHeaders(method); - } + private String sendAuthMethod(HttpRequestBase method, boolean enableAuth, boolean enableCORS) throws Exception { + CloseableHttpResponse httpResponse = null; + + try ( + CloseableHttpClient client = HttpClients.createDefault(); + ) { + // CORS check if (enableCORS) { String origin = "http://example.com"; - OptionsMethod optionsMethod = new OptionsMethod(method.getURI().toString()); - optionsMethod.addRequestHeader("Origin", origin); - setupAuthHeaders(optionsMethod); - int statusCode = client.executeMethod(optionsMethod); - if (statusCode == 200) { - assertNotNull(optionsMethod.getResponseHeader("Access-Control-Allow-Origin")); - assertEquals(origin, optionsMethod.getResponseHeader("Access-Control-Allow-Origin").getValue()); + + HttpOptions optionsMethod = new HttpOptions(method.getURI().toString()); + + optionsMethod.addHeader("Origin", origin); + + if (enableAuth) { + setupAuthHeaders(optionsMethod); + } + + httpResponse = client.execute(optionsMethod); + + if (httpResponse != null) { + StatusLine statusLine = httpResponse.getStatusLine(); + if (statusLine != null) { + String response = httpResponse.getStatusLine().getReasonPhrase(); + int statusCode = httpResponse.getStatusLine().getStatusCode(); + + if (statusCode == 200) { + Header originHeader = httpResponse.getFirstHeader("Access-Control-Allow-Origin"); + assertNotNull(originHeader); + assertEquals(origin, originHeader.getValue()); + } else { + fail("CORS returned: " + statusCode + " Error: " + response); + } + } else { + fail("Status line is null"); + } } else { - fail("CORS returned: " + statusCode + " Error: " + optionsMethod.getStatusLine().getReasonPhrase()); + fail("No http Response"); } } - int statusCode = client.executeMethod(method); - if (statusCode == 200) { - return method.getResponseBodyAsString(); - } else { - return method.getStatusLine().getReasonPhrase(); + + if (enableAuth) { + setupAuthHeaders(method); } + + httpResponse = client.execute(method); + + return EntityUtils.toString(httpResponse.getEntity()); } finally { - method.releaseConnection(); + httpResponse.close(); } } - private void setupAuthHeaders(final HttpMethodBase method) { - String userPass = ADMIN_USER + ":" + ADMIN_PASSWORD; - method.addRequestHeader(HttpHeaders.AUTHORIZATION, - "Basic " + new String(Base64.getEncoder().encode(userPass.getBytes()))); + private void setupAuthHeaders(final HttpRequestBase method) { + String authB64Code = + B64Code.encode(ADMIN_USER + ":" + ADMIN_PASSWORD, StringUtil.__ISO_8859_1); + method.setHeader(HttpHeader.AUTHORIZATION.asString(), "Basic " + authB64Code); } private Map<String, String> getConfOverlay(final String instanceId) { http://git-wip-us.apache.org/repos/asf/hive/blob/f2172cdb/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index e113399..ce3da37 100644 --- a/pom.xml +++ b/pom.xml @@ -133,7 +133,6 @@ <commons-collections.version>3.2.2</commons-collections.version> <commons-compress.version>1.9</commons-compress.version> <commons-exec.version>1.1</commons-exec.version> - <commons-httpclient.version>3.0.1</commons-httpclient.version> <commons-io.version>2.4</commons-io.version> <commons-lang.version>2.6</commons-lang.version> <commons-lang3.version>3.2</commons-lang3.version> @@ -358,11 +357,6 @@ <artifactId>commons-collections</artifactId> <version>${commons-collections.version}</version> </dependency> - <dependency> - <groupId>commons-httpclient</groupId> - <artifactId>commons-httpclient</artifactId> - <version>${commons-httpclient.version}</version> - </dependency> <dependency> <groupId>commons-io</groupId> <artifactId>commons-io</artifactId> http://git-wip-us.apache.org/repos/asf/hive/blob/f2172cdb/ql/pom.xml ---------------------------------------------------------------------- diff --git a/ql/pom.xml b/ql/pom.xml index 867a38a..fedb5f1 100644 --- a/ql/pom.xml +++ b/ql/pom.xml @@ -109,21 +109,6 @@ <version>${commons-codec.version}</version> </dependency> <dependency> - <groupId>commons-httpclient</groupId> - <artifactId>commons-httpclient</artifactId> - <version>${commons-httpclient.version}</version> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-log4j12</artifactId> - </exclusion> - <exclusion> - <groupId>commmons-logging</groupId> - <artifactId>commons-logging</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> <groupId>commons-io</groupId> <artifactId>commons-io</artifactId> <version>${commons-io.version}</version> http://git-wip-us.apache.org/repos/asf/hive/blob/f2172cdb/ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java index 866f43d..189975e 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java @@ -18,6 +18,8 @@ package org.apache.hadoop.hive.ql.parse; +import org.apache.commons.codec.DecoderException; +import org.apache.commons.codec.net.URLCodec; import org.apache.hadoop.hive.conf.HiveConf.StrictChecks; import java.io.IOException; import java.io.Serializable; @@ -31,7 +33,6 @@ import java.util.ArrayList; import java.util.HashSet; import org.antlr.runtime.tree.Tree; -import org.apache.commons.httpclient.util.URIUtil; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -109,8 +110,8 @@ public class LoadSemanticAnalyzer extends SemanticAnalyzer { return (srcs); } - private URI initializeFromURI(String fromPath, boolean isLocal) throws IOException, - URISyntaxException { + private URI initializeFromURI(String fromPath, boolean isLocal) + throws IOException, URISyntaxException, SemanticException { URI fromURI = new Path(fromPath).toUri(); String fromScheme = fromURI.getScheme(); @@ -121,8 +122,13 @@ public class LoadSemanticAnalyzer extends SemanticAnalyzer { // directory if (!path.startsWith("/")) { if (isLocal) { - path = URIUtil.decode( - new Path(System.getProperty("user.dir"), fromPath).toUri().toString()); + try { + path = new String(URLCodec.decodeUrl( + new Path(System.getProperty("user.dir"), fromPath).toUri().toString() + .getBytes("US-ASCII")), "US-ASCII"); + } catch (DecoderException de) { + throw new SemanticException("URL Decode failed", de); + } } else { path = new Path(new Path("/user/" + System.getProperty("user.name")), path).toString(); http://git-wip-us.apache.org/repos/asf/hive/blob/f2172cdb/service/src/java/org/apache/hive/service/server/HiveServer2.java ---------------------------------------------------------------------- diff --git a/service/src/java/org/apache/hive/service/server/HiveServer2.java b/service/src/java/org/apache/hive/service/server/HiveServer2.java index cf3edbf..661beb5 100644 --- a/service/src/java/org/apache/hive/service/server/HiveServer2.java +++ b/service/src/java/org/apache/hive/service/server/HiveServer2.java @@ -45,9 +45,6 @@ import org.apache.commons.cli.Option; import org.apache.commons.cli.OptionBuilder; import org.apache.commons.cli.Options; import org.apache.commons.cli.ParseException; -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.HttpMethodBase; -import org.apache.commons.httpclient.methods.DeleteMethod; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.concurrent.BasicThreadFactory; import org.apache.curator.framework.CuratorFramework; @@ -109,7 +106,12 @@ import org.apache.hive.service.cli.thrift.ThriftHttpCLIService; import org.apache.hive.service.servlet.HS2LeadershipStatus; import org.apache.hive.service.servlet.HS2Peers; import org.apache.hive.service.servlet.QueryProfileServlet; -import org.apache.http.HttpHeaders; +import org.apache.http.StatusLine; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.util.EntityUtils; import org.apache.logging.log4j.util.Strings; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; @@ -1377,22 +1379,38 @@ public class HiveServer2 extends CompositeService { // invoke DELETE /leader endpoint for failover String webEndpoint = "http://" + targetInstance.getHost() + ":" + webPort + "/leader"; - HttpClient client = new HttpClient(); - HttpMethodBase method = new DeleteMethod(webEndpoint); - try { - int statusCode = client.executeMethod(method); - if (statusCode == 200) { - System.out.println(method.getResponseBodyAsString()); - } else { - String response = method.getStatusLine().getReasonPhrase(); - LOG.error("Unable to failover HiveServer2 instance: " + workerIdentity + ". status code: " + - statusCode + "error: " + response); - System.err.println("Unable to failover HiveServer2 instance: " + workerIdentity + ". status code: " + - statusCode + " error: " + response); + HttpDelete httpDelete = new HttpDelete(webEndpoint); + CloseableHttpResponse httpResponse = null; + try ( + CloseableHttpClient client = HttpClients.createDefault(); + ) { + int statusCode = -1; + String response = "Response unavailable"; + httpResponse = client.execute(httpDelete); + if (httpResponse != null) { + StatusLine statusLine = httpResponse.getStatusLine(); + if (statusLine != null) { + response = httpResponse.getStatusLine().getReasonPhrase(); + statusCode = httpResponse.getStatusLine().getStatusCode(); + + if (statusCode == 200) { + System.out.println(EntityUtils.toString(httpResponse.getEntity())); + } + } + } + + if (statusCode != 200) { + // Failover didn't succeed - log error and exit + LOG.error("Unable to failover HiveServer2 instance: " + workerIdentity + + ". status code: " + statusCode + "error: " + response); + System.err.println("Unable to failover HiveServer2 instance: " + workerIdentity + + ". status code: " + statusCode + " error: " + response); System.exit(-1); } } finally { - method.releaseConnection(); + if (httpResponse != null) { + httpResponse.close(); + } } } catch (IOException e) { LOG.error("Error listing HiveServer2 HA instances from ZooKeeper", e);