Merge remote-tracking branch 'upstream/master'

Author: dskrvk

README.org

@@ -45,6 +45,15 @@
  - [[#testimonials][Testimonials]]
  - [[#license][License]]
 
+* HELP WANTED
+Hi Everyone, I'm in need of help with the 3.0.0 version (the master branch) of
+clj-http. In moving to the non-deprecated Apache HTTP client APIs I have broken
+a lot of the existing features for clj-http. Since 3.0.0 is basically a rewrite
+of =core.clj=, there are many things that still need implementing before 3.0 can
+be released. To see what needs fixing run =lein test :all=.
+
+If you have interest, please do send PRs for fixing.
+
 * Introduction
 
 ** Overview
@@ -54,7 +63,7 @@ library has taken over from mmcgrana's clj-http.
 
 [[https://secure.travis-ci.org/dakrone/clj-http.png]]
 
-** Philosophy 
+** Philosophy
 
 The design of =clj-http= is inspired by the [[http://github.com/mmcgrana/ring][Ring]] protocol for Clojure HTTP
  server applications.
@@ -160,11 +169,11 @@ Example requests:
   {:client-params {"http.protocol.allow-circular-redirects" false
                    "http.protocol.version" HttpVersion/HTTP_1_0
                    "http.useragent" "clj-http"}})
-                   
+
 ;; Set your own cookie policy
 (client/post "http://example.com"
   {:client-params {:cookie-policy (fn [cookie origin] (your-validation cookie origin))}})
-  
+
 ;; Completely ignore cookies:
 (client/post "http://example.com"
   {:client-params {:cookie-policy (constantly nil)}})
@@ -218,7 +227,7 @@ content encodings.
 (client/put "http://example.com/api" {:body "my PUT body"})
 
 #+END_SRC
-   
+
 ** POST
 
 #+BEGIN_SRC clojure
@@ -470,9 +479,9 @@ APIs:
     :last {:href "https://api.github.com/gists?page=22884"}}
 #+END_SRC
 
-** Redirects 
+** Redirects
 
-clj-http conforms its behaviour regarding automatic redirects to the [[https://tools.ietf.org/html/rfc2616#section-10.3][RFC]]. 
+clj-http conforms its behaviour regarding automatic redirects to the [[https://tools.ietf.org/html/rfc2616#section-10.3][RFC]].
 
 It means that redirects on status =301=, =302= and =307= are not redirected on
 methods other than =GET= and =HEAD=. If you want a behaviour closer to what most
@@ -853,7 +862,7 @@ and clj-json can now live together without causing problems.
 Persistent connections kept alive by the connection manager become stale: the
 target server shuts down the connection on its end without HttpClient being able
 to react to that event, while the connection is being idle, thus rendering the
-connection half-closed or 'stale'. 
+connection half-closed or 'stale'.
 
 This can be solved by using (with-connection-pool) as described in the
 'Using Persistent Connection' section above.

src/clj_http/conn_mgr.clj

@@ -198,19 +198,29 @@
 (defn ^javax.net.ssl.HostnameVerifier hostname-verifier []
   (BrowserCompatHostnameVerifier.))
 
-(defn ^org.apache.http.config.Registry registry-builder []
+(defn ^org.apache.http.config.Registry insecure-registry-builder []
   (-> (RegistryBuilder/create)
       (.register "http" PlainConnectionSocketFactory/INSTANCE)
-      (.register "https" (SSLConnectionSocketFactory.
-                          (ssl-context)
-                          (hostname-verifier)))
+      (.register "https" insecure-socket-factory)
+      (.build)))
+
+(defn ^org.apache.http.config.Registry registry-builder [{:keys [insecure?] :as opts}]
+  (-> (RegistryBuilder/create)
+      (.register "http" PlainConnectionSocketFactory/INSTANCE)
+      (.register "https"
+                 (cond
+                   insecure? insecure-socket-factory
+                   :default
+                   (SSLConnectionSocketFactory.
+                    (ssl-context)
+                    (hostname-verifier))))
       (.build)))
 
 (defn pooling-conn-mgr []
-  (PoolingHttpClientConnectionManager. (registry-builder)))
+  (PoolingHttpClientConnectionManager. (registry-builder {})))
 
 (defn basic-conn-mgr []
-  (BasicHttpClientConnectionManager. (registry-builder)))
+  (BasicHttpClientConnectionManager. (registry-builder {})))
 
 (defn reusable? [^HttpClientConnectionManager conn-mgr]
   (instance? PoolingHttpClientConnectionManager conn-mgr))

src/clj_http/core.clj

@@ -127,7 +127,7 @@
   "Coerce the http-entity from an HttpResponse to a stream that closes itself
   and the connection manager when closed."
   [^HttpEntity http-entity ^HttpClientConnectionManager conn-mgr ^CloseableHttpResponse response]
-  (when http-entity
+  (if http-entity
     (proxy [FilterInputStream]
         [^InputStream (.getContent http-entity)]
       (close []
@@ -138,7 +138,9 @@
           (finally
             (.close response)
             (when-not (conn/reusable? conn-mgr)
-              (.shutdown conn-mgr))))))))
+              (.shutdown conn-mgr))))))
+    (when-not (conn/reusable? conn-mgr)
+      (.shutdown conn-mgr))))
 
 (defn- print-debug!
   "Print out debugging information to *out* for a given request."
@@ -217,15 +219,20 @@
           (.addHeader http-req header-n header-vth))
         (.addHeader http-req header-n (str header-v))))
     (when (opt req :debug) (print-debug! req http-req))
-    (let [^CloseableHttpResponse response (.execute client http-req context)
-          ^HttpEntity entity (.getEntity response)
-          status (.getStatusLine response)]
-      {:body (coerce-body-entity entity conn-mgr response)
-       :headers (parse-headers
-                 (.headerIterator response)
-                 (opt req :use-header-maps-in-response))
-       :length (if (nil? entity) 0 (.getContentLength entity))
-       :chunked? (if (nil? entity) false (.isChunked entity))
-       :repeatable? (if (nil? entity) false (.isRepeatable entity))
-       :streaming? (if (nil? entity) false (.isStreaming entity))
-       :status (.getStatusCode status)})))
+    (try
+      (let [^CloseableHttpResponse response (.execute client http-req context)
+            ^HttpEntity entity (.getEntity response)
+            status (.getStatusLine response)]
+        {:body (coerce-body-entity entity conn-mgr response)
+         :headers (parse-headers
+                   (.headerIterator response)
+                   (opt req :use-header-maps-in-response))
+         :length (if (nil? entity) 0 (.getContentLength entity))
+         :chunked? (if (nil? entity) false (.isChunked entity))
+         :repeatable? (if (nil? entity) false (.isRepeatable entity))
+         :streaming? (if (nil? entity) false (.isStreaming entity))
+         :status (.getStatusCode status)})
+      (catch Throwable t
+        (when-not (conn/reusable? conn-mgr)
+          (.shutdown conn-mgr))
+        (throw t)))))

test/clj_http/test/conn_mgr_test.clj

@@ -78,7 +78,7 @@
                      :as :stream})
       (is false "request should have thrown an exception")
       (catch Exception e))
-    (is @shutdown? "Connection manager has been shut down")))
+    (is @shutdown? "Connection manager has been shutdown")))
 
 (deftest ^:integration t-closed-conn-mgr-for-empty-body
   (run-server)
@@ -87,9 +87,9 @@
              (shutdown []
                (reset! shutdown? true)))
         response (core/request {:request-method :get :uri "/unmodified-resource"
-                             :server-port 18080 :scheme :http
-                             :server-name "localhost"
-                             :connection-manager cm })]
+                                :server-port 18080 :scheme :http
+                                :server-name "localhost"
+                                :connection-manager cm})]
     (is (nil? (:body response)) "response shouldn't have body")
     (is (= 304 (:status response)))
-    (is @shutdown? "connection manager should be shut downed")))
+    (is @shutdown? "connection manager should be shutdown")))