From dd2ca97506f7d9ff5260477b36509c164263962c Mon Sep 17 00:00:00 2001 From: nevermindr Date: Thu, 19 Nov 2015 20:57:13 +0100 Subject: [PATCH 1/2] Slack notification plugin error proxy rewritten with org.apache.http.impl.client.CloseableHttpClient added httpclient4.3.2 dependency fixed tests * HttpClientStub -> CloseableHttpClientStub * getContent on response object fixed --- .../plugins/slack/StandardSlackService.java | 78 +++++--- .../slack/CloseableHttpClientStub.java | 62 +++++++ .../slack/CloseableHttpResponseStub.java | 172 ++++++++++++++++++ .../slack/StandardSlackServiceStub.java | 6 +- .../slack/StandardSlackServiceTest.java | 16 +- 5 files changed, 294 insertions(+), 40 deletions(-) create mode 100644 src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java create mode 100644 src/test/java/jenkins/plugins/slack/CloseableHttpResponseStub.java diff --git a/src/main/java/jenkins/plugins/slack/StandardSlackService.java b/src/main/java/jenkins/plugins/slack/StandardSlackService.java index 4a1f45e4..c5242735 100755 --- a/src/main/java/jenkins/plugins/slack/StandardSlackService.java +++ b/src/main/java/jenkins/plugins/slack/StandardSlackService.java @@ -2,33 +2,46 @@ import hudson.security.ACL; -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.HttpStatus; -import org.apache.commons.httpclient.UsernamePasswordCredentials; -import org.apache.commons.httpclient.auth.AuthScope; -import org.apache.commons.httpclient.methods.PostMethod; - import org.jenkinsci.plugins.plaincredentials.StringCredentials; import com.cloudbees.plugins.credentials.CredentialsMatcher; import com.cloudbees.plugins.credentials.CredentialsMatchers; -import com.cloudbees.plugins.credentials.CredentialsProvider; import com.cloudbees.plugins.credentials.domains.DomainRequirement; import org.json.JSONArray; import org.json.JSONObject; import java.io.UnsupportedEncodingException; +import java.io.InputStream; import java.net.URLEncoder; import java.util.Collections; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.ArrayList; import jenkins.model.Jenkins; import hudson.ProxyConfiguration; import org.apache.commons.lang.StringUtils; +import org.apache.http.auth.AuthScope; +import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.http.client.CredentialsProvider; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.conn.routing.HttpRoutePlanner; +import org.apache.http.impl.client.BasicCredentialsProvider; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.DefaultProxyRoutePlanner; +import org.apache.http.message.BasicNameValuePair; +import org.apache.http.util.EntityUtils; +import org.apache.http.HttpEntity; +import org.apache.http.HttpHost; +import org.apache.http.HttpStatus; +import org.apache.http.NameValuePair; public class StandardSlackService implements SlackService { @@ -76,23 +89,23 @@ public boolean publish(String message, String color) { JSONArray attachments = new JSONArray(); attachments.put(attachment); - PostMethod post; + HttpPost post; String url; + List nvps = new ArrayList(); //prepare post methods for both requests types if (!botUser || !StringUtils.isEmpty(baseUrl)) { url = "https://" + teamDomain + "." + host + "/services/hooks/jenkins-ci?token=" + getTokenToUse(); if (!StringUtils.isEmpty(baseUrl)) { url = baseUrl + getTokenToUse(); } - post = new PostMethod(url); + post = new HttpPost(url); JSONObject json = new JSONObject(); json.put("channel", roomId); json.put("attachments", attachments); json.put("link_names", "1"); - post.addParameter("payload", json.toString()); - + nvps.add(new BasicNameValuePair("payload", json.toString())); } else { url = "https://slack.com/api/chat.postMessage?token=" + getTokenToUse() + "&channel=" + roomId + @@ -103,18 +116,22 @@ public boolean publish(String message, String color) { } catch (UnsupportedEncodingException e) { logger.log(Level.ALL, "Error while encoding attachments: " + e.getMessage()); } - post = new PostMethod(url); + post = new HttpPost(url); } logger.fine("Posting: to " + roomId + " on " + teamDomain + " using " + url + ": " + message + " " + color); - HttpClient client = getHttpClient(); - post.getParams().setContentCharset("UTF-8"); + CloseableHttpClient client = getHttpClient(); try { - int responseCode = client.executeMethod(post); - String response = post.getResponseBodyAsString(); - if (responseCode != HttpStatus.SC_OK) { - logger.log(Level.WARNING, "Slack post may have failed. Response: " + response); - result = false; + post.setEntity(new UrlEncodedFormEntity(nvps, "UTF-8")); + CloseableHttpResponse response = client.execute(post); + + int responseCode = response.getStatusLine().getStatusCode(); + if(responseCode != HttpStatus.SC_OK) { + HttpEntity entity = response.getEntity(); + String responseString = EntityUtils.toString(entity); + logger.log(Level.WARNING, "Slack post may have failed. Response: " + responseString); + logger.log(Level.WARNING, "Response Code: " + responseCode); + result = false; } else { logger.info("Posting succeeded"); } @@ -143,31 +160,34 @@ private String getTokenToUse() { } private StringCredentials lookupCredentials(String credentialId) { - List credentials = CredentialsProvider.lookupCredentials(StringCredentials.class, Jenkins.getInstance(), ACL.SYSTEM, Collections.emptyList()); + List credentials = com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials(StringCredentials.class, Jenkins.getInstance(), ACL.SYSTEM, Collections.emptyList()); CredentialsMatcher matcher = CredentialsMatchers.withId(credentialId); return CredentialsMatchers.firstOrNull(credentials, matcher); } - protected HttpClient getHttpClient() { - HttpClient client = new HttpClient(); + protected CloseableHttpClient getHttpClient() { + final HttpClientBuilder clientBuilder = HttpClients.custom(); + final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); + clientBuilder.setDefaultCredentialsProvider(credentialsProvider); + if (Jenkins.getInstance() != null) { ProxyConfiguration proxy = Jenkins.getInstance().proxy; if (proxy != null) { - client.getHostConfiguration().setProxy(proxy.name, proxy.port); + final HttpHost proxyHost = new HttpHost(proxy.name, proxy.port); + final HttpRoutePlanner routePlanner = new DefaultProxyRoutePlanner(proxyHost); + clientBuilder.setRoutePlanner(routePlanner); + String username = proxy.getUserName(); String password = proxy.getPassword(); // Consider it to be passed if username specified. Sufficient? if (username != null && !"".equals(username.trim())) { logger.info("Using proxy authentication (user=" + username + ")"); - // http://hc.apache.org/httpclient-3.x/authentication.html#Proxy_Authentication - // and - // http://svn.apache.org/viewvc/httpcomponents/oac.hc3x/trunk/src/examples/BasicAuthenticationExample.java?view=markup - client.getState().setProxyCredentials(AuthScope.ANY, - new UsernamePasswordCredentials(username, password)); + credentialsProvider.setCredentials(new AuthScope(proxyHost), + new UsernamePasswordCredentials(username, password)); } } } - return client; + return clientBuilder.build(); } void setHost(String host) { diff --git a/src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java b/src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java new file mode 100644 index 00000000..0d0e110d --- /dev/null +++ b/src/test/java/jenkins/plugins/slack/CloseableHttpClientStub.java @@ -0,0 +1,62 @@ +package jenkins.plugins.slack; + +import org.apache.commons.httpclient.HttpStatus; +import org.apache.http.HttpHost; +import org.apache.http.HttpRequest; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.conn.ClientConnectionManager; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.params.HttpParams; +import org.apache.http.protocol.HttpContext; + +import java.io.IOException; + +public class CloseableHttpClientStub extends CloseableHttpClient { + + private int numberOfCallsToExecuteMethod; + private int httpStatus; + private boolean failAlternateResponses = false; + + public CloseableHttpResponse execute(HttpUriRequest post) { + numberOfCallsToExecuteMethod++; + if (failAlternateResponses && (numberOfCallsToExecuteMethod % 2 == 0)) { + return new CloseableHttpResponseStub(HttpStatus.SC_NOT_FOUND); + } else { + return new CloseableHttpResponseStub(httpStatus); + } + } + + @Override + public ClientConnectionManager getConnectionManager() { + return null; + } + + @Override + public void close() throws IOException { + + } + + @Override + public HttpParams getParams() { + return null; + } + + @Override + protected CloseableHttpResponse doExecute(HttpHost httpHost, HttpRequest httpRequest, HttpContext httpContext) throws IOException, ClientProtocolException { + return null; + } + + public int getNumberOfCallsToExecuteMethod() { + return numberOfCallsToExecuteMethod; + } + + public void setHttpStatus(int httpStatus) { + this.httpStatus = httpStatus; + } + + public void setFailAlternateResponses(boolean failAlternateResponses) { + this.failAlternateResponses = failAlternateResponses; + } +} diff --git a/src/test/java/jenkins/plugins/slack/CloseableHttpResponseStub.java b/src/test/java/jenkins/plugins/slack/CloseableHttpResponseStub.java new file mode 100644 index 00000000..c45adf65 --- /dev/null +++ b/src/test/java/jenkins/plugins/slack/CloseableHttpResponseStub.java @@ -0,0 +1,172 @@ +package jenkins.plugins.slack; + +import org.apache.http.*; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.params.HttpParams; + +import java.io.IOException; +import java.util.Locale; + +public class CloseableHttpResponseStub implements CloseableHttpResponse { + + private int responseCode; + + public CloseableHttpResponseStub(int response) { + responseCode = response; + } + + @Override + public void close() throws IOException { + + } + + @Override + public StatusLine getStatusLine() { + return new StatusLine() { + @Override + public ProtocolVersion getProtocolVersion() { + return null; + } + + @Override + public int getStatusCode() { + return responseCode; + } + + @Override + public String getReasonPhrase() { + return null; + } + }; + } + + @Override + public void setStatusLine(StatusLine statusLine) { + + } + + @Override + public void setStatusLine(ProtocolVersion protocolVersion, int i) { + + } + + @Override + public void setStatusLine(ProtocolVersion protocolVersion, int i, String s) { + + } + + @Override + public void setStatusCode(int i) throws IllegalStateException { + + } + + @Override + public void setReasonPhrase(String s) throws IllegalStateException { + + } + + @Override + public HttpEntity getEntity() { + return null; + } + + @Override + public void setEntity(HttpEntity httpEntity) { + + } + + @Override + public Locale getLocale() { + return null; + } + + @Override + public void setLocale(Locale locale) { + + } + + @Override + public ProtocolVersion getProtocolVersion() { + return null; + } + + @Override + public boolean containsHeader(String s) { + return false; + } + + @Override + public Header[] getHeaders(String s) { + return new Header[0]; + } + + @Override + public Header getFirstHeader(String s) { + return null; + } + + @Override + public Header getLastHeader(String s) { + return null; + } + + @Override + public Header[] getAllHeaders() { + return new Header[0]; + } + + @Override + public void addHeader(Header header) { + + } + + @Override + public void addHeader(String s, String s1) { + + } + + @Override + public void setHeader(Header header) { + + } + + @Override + public void setHeader(String s, String s1) { + + } + + @Override + public void setHeaders(Header[] headers) { + + } + + @Override + public void removeHeader(Header header) { + + } + + @Override + public void removeHeaders(String s) { + + } + + @Override + public HeaderIterator headerIterator() { + return null; + } + + @Override + public HeaderIterator headerIterator(String s) { + return null; + } + + @Override + public HttpParams getParams() { + return null; + } + + @Override + public void setParams(HttpParams httpParams) { + + } +} diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java index 54260984..bf2763f5 100644 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceStub.java @@ -2,18 +2,18 @@ public class StandardSlackServiceStub extends StandardSlackService { - private HttpClientStub httpClientStub; + private CloseableHttpClientStub httpClientStub; public StandardSlackServiceStub(String baseUrl, String teamDomain, String token, String tokenCredentialId, boolean botUser, String roomId) { super(baseUrl, teamDomain, token, tokenCredentialId, botUser, roomId); } @Override - public HttpClientStub getHttpClient() { + public CloseableHttpClientStub getHttpClient() { return httpClientStub; } - public void setHttpClient(HttpClientStub httpClientStub) { + public void setHttpClient(CloseableHttpClientStub httpClientStub) { this.httpClientStub = httpClientStub; } } diff --git a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java index 0f6b4065..871609eb 100755 --- a/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java +++ b/src/test/java/jenkins/plugins/slack/StandardSlackServiceTest.java @@ -39,7 +39,7 @@ public void invalidTokenShouldFail() { @Test public void publishToASingleRoomSendsASingleMessage() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); assertEquals(1, service.getHttpClient().getNumberOfCallsToExecuteMethod()); @@ -48,7 +48,7 @@ public void publishToASingleRoomSendsASingleMessage() { @Test public void publishToMultipleRoomsSendsAMessageToEveryRoom() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); service.setHttpClient(httpClientStub); service.publish("message"); assertEquals(3, service.getHttpClient().getNumberOfCallsToExecuteMethod()); @@ -57,7 +57,7 @@ public void publishToMultipleRoomsSendsAMessageToEveryRoom() { @Test public void successfulPublishToASingleRoomReturnsTrue() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); assertTrue(service.publish("message")); @@ -66,7 +66,7 @@ public void successfulPublishToASingleRoomReturnsTrue() { @Test public void successfulPublishToMultipleRoomsReturnsTrue() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); assertTrue(service.publish("message")); @@ -75,7 +75,7 @@ public void successfulPublishToMultipleRoomsReturnsTrue() { @Test public void failedPublishToASingleRoomReturnsFalse() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1"); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_NOT_FOUND); service.setHttpClient(httpClientStub); assertFalse(service.publish("message")); @@ -84,7 +84,7 @@ public void failedPublishToASingleRoomReturnsFalse() { @Test public void singleFailedPublishToMultipleRoomsReturnsFalse() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, "#room1,#room2,#room3"); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setFailAlternateResponses(true); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); @@ -94,7 +94,7 @@ public void singleFailedPublishToMultipleRoomsReturnsFalse() { @Test public void publishToEmptyRoomReturnsTrue() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, false, ""); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); assertTrue(service.publish("message")); @@ -104,7 +104,7 @@ public void publishToEmptyRoomReturnsTrue() { @Test public void sendAsBotUserReturnsTrue() { StandardSlackServiceStub service = new StandardSlackServiceStub("", "domain", "token", null, true, "#room1"); - HttpClientStub httpClientStub = new HttpClientStub(); + CloseableHttpClientStub httpClientStub = new CloseableHttpClientStub(); httpClientStub.setHttpStatus(HttpStatus.SC_OK); service.setHttpClient(httpClientStub); assertTrue(service.publish("message")); From 3794779291bade20d4026da66ba76f353989e94f Mon Sep 17 00:00:00 2001 From: Jiameng Yu Date: Mon, 6 Mar 2017 11:22:43 -0500 Subject: [PATCH 2/2] Fix proxy issue #102 * Use CloseableHttpClient to replace HttpClient implementation * Updated test cases with CloseableHttpClientStub * Optimize CloseableHttpClient Response manipulation. * Rebased #156 on latest master --- pom.xml | 6 ++-- .../jenkins/plugins/slack/HttpClientStub.java | 34 ------------------- 2 files changed, 3 insertions(+), 37 deletions(-) delete mode 100644 src/test/java/jenkins/plugins/slack/HttpClientStub.java diff --git a/pom.xml b/pom.xml index ee69c41e..3e857f4c 100644 --- a/pom.xml +++ b/pom.xml @@ -70,9 +70,9 @@ 2.4.4 - commons-httpclient - commons-httpclient - 3.1 + org.apache.httpcomponents + httpclient + 4.3.2 org.json diff --git a/src/test/java/jenkins/plugins/slack/HttpClientStub.java b/src/test/java/jenkins/plugins/slack/HttpClientStub.java deleted file mode 100644 index f6de0935..00000000 --- a/src/test/java/jenkins/plugins/slack/HttpClientStub.java +++ /dev/null @@ -1,34 +0,0 @@ -package jenkins.plugins.slack; - -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.HttpMethod; -import org.apache.commons.httpclient.HttpStatus; - -public class HttpClientStub extends HttpClient { - - private int numberOfCallsToExecuteMethod; - private int httpStatus; - private boolean failAlternateResponses = false; - - @Override - public int executeMethod(HttpMethod httpMethod) { - numberOfCallsToExecuteMethod++; - if (failAlternateResponses && (numberOfCallsToExecuteMethod % 2 == 0)) { - return HttpStatus.SC_NOT_FOUND; - } else { - return httpStatus; - } - } - - public int getNumberOfCallsToExecuteMethod() { - return numberOfCallsToExecuteMethod; - } - - public void setHttpStatus(int httpStatus) { - this.httpStatus = httpStatus; - } - - public void setFailAlternateResponses(boolean failAlternateResponses) { - this.failAlternateResponses = failAlternateResponses; - } -}