diff --git a/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/pom.xml b/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/pom.xml index 6e37b3c8d4..a82cdc1ae8 100644 --- a/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/pom.xml +++ b/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/pom.xml @@ -31,7 +31,7 @@ XWiki Rendering - Transformation - Link Checker Verifies if external URLs are valid and decorate with parameters if not - 0.87 + 0.90 @@ -45,12 +45,26 @@ ${commons.version} - commons-httpclient - commons-httpclient + org.apache.httpcomponents + httpclient org.apache.commons commons-lang3 + + + com.github.tomakehurst + wiremock + 1.46 + test + + + + log4j + log4j + + + diff --git a/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/src/main/java/org/xwiki/rendering/internal/transformation/linkchecker/DefaultHTTPChecker.java b/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/src/main/java/org/xwiki/rendering/internal/transformation/linkchecker/DefaultHTTPChecker.java index d143dc981c..2726badedf 100644 --- a/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/src/main/java/org/xwiki/rendering/internal/transformation/linkchecker/DefaultHTTPChecker.java +++ b/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/src/main/java/org/xwiki/rendering/internal/transformation/linkchecker/DefaultHTTPChecker.java @@ -22,11 +22,13 @@ import javax.inject.Inject; import javax.inject.Singleton; -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; -import org.apache.commons.httpclient.cookie.CookiePolicy; -import org.apache.commons.httpclient.methods.GetMethod; -import org.apache.commons.httpclient.params.HttpMethodParams; +import org.apache.http.client.config.CookieSpecs; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.slf4j.Logger; import org.xwiki.component.annotation.Component; import org.xwiki.component.phase.Initializable; @@ -54,17 +56,30 @@ public class DefaultHTTPChecker implements HTTPChecker, Initializable * Note: If one day we wish to configure timeouts, here's some good documentation about it: * http://brian.olore.net/wp/2009/08/apache-httpclient-timeout/ */ - private HttpClient httpClient; + private CloseableHttpClient httpClient; @Override public void initialize() throws InitializationException { - this.httpClient = new HttpClient(new MultiThreadedHttpConnectionManager()); + HttpClientBuilder httpClientBuilder = HttpClientBuilder.create(); + + // Make the Http Client reusable by several threads + PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(); + httpClientBuilder.setConnectionManager(connectionManager); + + // Pre-configure with everything configured at JVM level (e.g. proxy setup). + httpClientBuilder.useSystemProperties(); // Set our user agent to be a good citizen. - this.httpClient.getParams().setParameter(HttpMethodParams.USER_AGENT, "XWikiLinkChecker"); + httpClientBuilder.setUserAgent("XWikiLinkChecker"); + // Ignore cookies since this can cause errors in logs and we don't need cookies when checking sites. - this.httpClient.getParams().setCookiePolicy(CookiePolicy.IGNORE_COOKIES); + RequestConfig config = RequestConfig.custom() + .setCookieSpec(CookieSpecs.IGNORE_COOKIES) + .build(); + httpClientBuilder.setDefaultRequestConfig(config); + + this.httpClient = httpClientBuilder.build(); } @Override @@ -72,13 +87,11 @@ public int check(String url) { int responseCode; - GetMethod method = null; + CloseableHttpResponse httpResponse = null; try { - method = new GetMethod(url); - - // Execute the method. - responseCode = this.httpClient.executeMethod(method); - + HttpGet httpGet = new HttpGet(url); + httpResponse = this.httpClient.execute(httpGet); + responseCode = httpResponse.getStatusLine().getStatusCode(); this.logger.debug("Result of pinging [{}]: code = [{}]", url, responseCode); } catch (Exception e) { // Some error in the transport or in the passed URL, use a special response code (0) which isn't in the @@ -86,8 +99,13 @@ public int check(String url) responseCode = 0; this.logger.debug("Error while checking [{}]", url, e); } finally { - if (method != null) { - method.releaseConnection(); + if (httpResponse != null) { + try { + httpResponse.close(); + } catch (Exception ee) { + // Failed to close, ignore but log the error. + this.logger.error("Failed to close HTTP connection for [{}]", url, ee); + } } } diff --git a/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/src/test/java/org/xwiki/rendering/internal/transformation/linkchecker/DefaultHTTPCheckerTest.java b/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/src/test/java/org/xwiki/rendering/internal/transformation/linkchecker/DefaultHTTPCheckerTest.java new file mode 100644 index 0000000000..9838f40935 --- /dev/null +++ b/xwiki-rendering-transformations/xwiki-rendering-transformation-linkchecker/src/test/java/org/xwiki/rendering/internal/transformation/linkchecker/DefaultHTTPCheckerTest.java @@ -0,0 +1,62 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.rendering.internal.transformation.linkchecker; + +import org.junit.Rule; +import org.junit.Test; +import org.xwiki.test.mockito.MockitoComponentMockingRule; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; + +import static com.github.tomakehurst.wiremock.client.RequestPatternBuilder.allRequests; +import static com.github.tomakehurst.wiremock.client.WireMock.findAll; +import static org.junit.Assert.*; + +/** + * Integration tests for {@link org.xwiki.rendering.internal.transformation.linkchecker.DefaultHTTPChecker}. + * + * @version $Id$ + * @since 6.1M2 + */ +public class DefaultHTTPCheckerTest +{ + @Rule + public WireMockRule proxyWireMockRule = new WireMockRule(8888); + + @Rule + public MockitoComponentMockingRule mocker = + new MockitoComponentMockingRule<>(DefaultHTTPChecker.class); + + @Test + public void testProxy() throws Exception + { + // First call the link checker but since we haven't set up any proxy our Mock HTTP Server is not going to be + // called (since http://host will lead to nowhere... + assertEquals(0, this.mocker.getComponentUnderTest().check("http://host")); + assertTrue("The HTTP server was not called by the link checker", findAll(allRequests()).isEmpty()); + + // Second, setup a proxy by using System Properties, then call again the checker and this time it should + // succeed since http://host will go to the proxy which is pointing to our Mock HTTP Server! + System.setProperty("http.proxyHost", "localhost"); + System.setProperty("http.proxyPort", "8888"); + assertEquals(404, this.mocker.getComponentUnderTest().check("http://host")); + assertFalse("The HTTP server was called by the link checker", findAll(allRequests()).isEmpty()); + } +}