update Apache HttpClient dependency to v. 4.5.1

[matthiasblaesing]

This request is based on this request, which asks for SNI support in redminenb:

https://github.com/redminenb/redminenb/issues/39

and the change log for http client (version 4.3.2):

http://www.apache.org/dist/httpcomponents/httpclient/RELEASE_NOTES-4.3.x.txt

SNI support for Oracle JRE 1.7+ is being among the most notable improvements.

I experimented with overriding the httpclient version to 4.4.1 (and creating my own HttpClient) in redminenb and got SNI support. Just based on a "compile test" exchanging the version is easy and compile almost cleanly - it looks as if "only" the constructor for StringEntity was changed to throw a different exception. A change that would allow usage with 4.2 + 4.4.1 could look like this:

# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- HEAD
+++ Modified In Working Tree
@@ -61,6 +61,7 @@
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
+import java.nio.charset.UnsupportedCharsetException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -588,7 +589,11 @@
    private static void setEntity(HttpEntityEnclosingRequest request, String body, String contentType) {
        StringEntity entity;
        try {
+                        try {
            entity = new StringEntity(body, CHARSET);
+                        } catch (UnsupportedCharsetException ex) {
+                            throw new UnsupportedEncodingException(ex.getCharsetName());
+                        }
        } catch (UnsupportedEncodingException e) {
            throw new RedmineInternalError("Required charset " + CHARSET
                    + " is not supported", e);

This maps the new UnsupportedCharsetException onto the old UnsupportedEncodingException. (Handling both exception in the catch directly does not work, as the compile complains that the UnsupportedEncodingException is never thrown ...)

I decided against a direct pull request, as changing the dependency needs to be evaluated (compatiblity with android comes to mind). And as many classes in httpclient seem to be deprecated restructering/modernizing RedmineManagerFactory looks like a good idea.

[alexeyOnGitHub]

why change the dependency version? what does the new version give us?

[matthiasblaesing]

The new http client supports SNI (Server name indication). This needed for Virtual Hosts, that use SSL/TLS. Without SNI the target host does not know the requestes hostname until the TLS-Handshake is complete and the http phase begins. The problem: In the TLS Handshake the server certificate is send, so you either need to have an SSL certificate, that covers all hostnames via SAN (subject alternative names) or the client needs to indicate the target host in the TLS-Handshake.

According to the linked change log (version 4.3.2):

http://www.apache.org/dist/httpcomponents/httpclient/RELEASE_NOTES-4.3.x.txt

The version is newer than your dependency.

[alexeyOnGitHub]

@maxkar - any objections against switching to the recent (latest?) Apache HttpClient?

[alexeyOnGitHub]

not sure how much value supporting multiple HttpClient versions would bring.

about Android support - there could be multiple issues there because Android has something similar to Java, but not quite. we do not test this library with android and so far nobody offered to help with that.

about refactoring - [separate] pull requests are always welcome!

[alexeyOnGitHub]

merged the code to "master"

[alexeyOnGitHub]

I see non-deterministic integration test failures with this newer HttpClient version (4.5.1). removed this change from "master" branch. either we are not using this version correctly or there are bugs in it. either way this should not be in "master" until it works 100 times out of 100.

[alexeyOnGitHub]

oops, I was able to reproduce this ND behavior when I switched tests to HTTPS. I investigated this and opened a new MAJOR issue #222.

[alexeyOnGitHub]

I applied a temporary fix in UserIntegrationTest class.

re-integrated the Apache HttpClient update to "master" branch.