diff --git a/src/clj/puppetlabs/http/client/async.clj b/src/clj/puppetlabs/http/client/async.clj index 3201ad3..1d8e704 100644 --- a/src/clj/puppetlabs/http/client/async.clj +++ b/src/clj/puppetlabs/http/client/async.clj @@ -24,7 +24,8 @@ (org.apache.http.entity InputStreamEntity ContentType) (java.io InputStream) (com.puppetlabs.http.client.impl Compression) - (org.apache.http.impl.client LaxRedirectStrategy)) + (org.apache.http.client RedirectStrategy) + (org.apache.http.impl.client LaxRedirectStrategy DefaultRedirectStrategy)) (:require [puppetlabs.certificate-authority.core :as ssl] [clojure.string :as str] [puppetlabs.kitchensink.core :as ks] @@ -237,7 +238,8 @@ (schema/defn extract-client-opts :- common/ClientOptions [opts :- common/UserRequestOptions] - (select-keys opts [:ssl-context :ssl-ca-cert :ssl-cert :ssl-key :force-redirects])) + (select-keys opts [:ssl-context :ssl-ca-cert :ssl-cert :ssl-key + :force-redirects :follow-redirects])) (schema/defn extract-ssl-opts :- common/SslOptions [opts :- common/ClientOptions] @@ -247,17 +249,31 @@ [opts :- common/UserRequestOptions] (select-keys opts [:url :method :headers :body :decompress-body :as :persistent :query-params])) -(schema/defn create-default-client :- common/Client +(schema/defn ^:always-validate redirect-strategy :- RedirectStrategy [opts :- common/ClientOptions] - (let [force-redirects (:force-redirects opts) - configured-opts (configure-ssl (extract-ssl-opts opts)) + (let [follow-redirects (:follow-redirects opts) + force-redirects (:force-redirects opts)] + (cond + (and (not (nil? follow-redirects)) (not follow-redirects)) + (proxy [RedirectStrategy] [] + (isRedirected [req resp context] + false) + (getRedirect [req resp context] + nil)) + force-redirects + (LaxRedirectStrategy.) + :else + (DefaultRedirectStrategy.)))) + +(schema/defn ^:always-validate create-default-client :- common/Client + [opts :- common/ClientOptions] + (let [configured-opts (configure-ssl (extract-ssl-opts opts)) client-builder (HttpAsyncClients/custom) client (do (if (:ssl-context configured-opts) (.setSSLContext client-builder (:ssl-context configured-opts))) - (if force-redirects - (.setRedirectStrategy client-builder - (LaxRedirectStrategy.))) + (.setRedirectStrategy client-builder + (redirect-strategy opts)) (.build client-builder))] (.start client) client)) @@ -316,7 +332,10 @@ respectively. Defaults to `:stream`. * :query-params - used to set the query parameters of an http request * :force-redirects - used to set whether or not the client should follow - redirects. Defaults to false. + redirects on POST or PUT requests. Defaults to false. + * :follow-redirects - used to set whether or not the client should follow + redirects in general. Defaults to true. If set to false, will override + the :force-redirects setting. SSL options: @@ -335,15 +354,13 @@ (schema/defn create-client :- common/HTTPClient [opts :- common/ClientOptions] - (let [force-redirects (:force-redirects opts) - opts (configure-ssl (extract-ssl-opts opts)) + (let [configured-opts (configure-ssl (extract-ssl-opts opts)) client-builder (HttpAsyncClients/custom) - client (do (if (:ssl-context opts) + client (do (if (:ssl-context configured-opts) (.setSSLContext client-builder - (:ssl-context opts))) - (if force-redirects - (.setRedirectStrategy client-builder - (LaxRedirectStrategy.))) + (:ssl-context configured-opts))) + (.setRedirectStrategy client-builder + (redirect-strategy opts)) (.build client-builder))] (.start client) (reify common/HTTPClient diff --git a/src/clj/puppetlabs/http/client/common.clj b/src/clj/puppetlabs/http/client/common.clj index 14061c0..322a13d 100644 --- a/src/clj/puppetlabs/http/client/common.clj +++ b/src/clj/puppetlabs/http/client/common.clj @@ -57,7 +57,8 @@ (ok :ssl-cert) UrlOrString (ok :ssl-key) UrlOrString (ok :ssl-ca-cert) UrlOrString - (ok :force-redirects) schema/Bool}) + (ok :force-redirects) schema/Bool + (ok :follow-redirects) schema/Bool}) (def RawUserRequestOptions "The list of request options passed by a user into the @@ -98,27 +99,28 @@ (def SslOptions (schema/either {} SslContextOptions SslCertOptions SslCaCertOptions)) -(def RedirectOption - {(schema/optional-key :force-redirects) schema/Bool}) +(def RedirectOptions + {(schema/optional-key :force-redirects) schema/Bool + (schema/optional-key :follow-redirects) schema/Bool}) (def UserRequestOptions "A cleaned-up version of RawUserRequestClientOptions, which is formed after validating the RawUserRequestClientOptions and merging it with the defaults." (schema/either - (merge RequestOptions RedirectOption) - (merge RequestOptions SslContextOptions RedirectOption) - (merge RequestOptions SslCaCertOptions RedirectOption) - (merge RequestOptions SslCertOptions RedirectOption))) + (merge RequestOptions RedirectOptions) + (merge RequestOptions SslContextOptions RedirectOptions) + (merge RequestOptions SslCaCertOptions RedirectOptions) + (merge RequestOptions SslCertOptions RedirectOptions))) (def ClientOptions "The options from UserRequestOptions that are related to the instantiation/management of a client. This is everything from UserRequestOptions not included in RequestOptions." (schema/either - RedirectOption - (merge SslContextOptions RedirectOption) - (merge SslCertOptions RedirectOption) - (merge SslCaCertOptions RedirectOption))) + RedirectOptions + (merge SslContextOptions RedirectOptions) + (merge SslCertOptions RedirectOptions) + (merge SslCaCertOptions RedirectOptions))) (def ResponseCallbackFn (schema/maybe (schema/pred ifn?))) diff --git a/src/clj/puppetlabs/http/client/sync.clj b/src/clj/puppetlabs/http/client/sync.clj index d9b8911..f76b1e0 100644 --- a/src/clj/puppetlabs/http/client/sync.clj +++ b/src/clj/puppetlabs/http/client/sync.clj @@ -32,15 +32,13 @@ (schema/defn create-client :- common/HTTPClient [opts :- common/ClientOptions] - (let [force-redirects (:force-redirects opts) - opts (async/configure-ssl (async/extract-ssl-opts opts)) + (let [configured-opts (async/configure-ssl (async/extract-ssl-opts opts)) client-builder (HttpAsyncClients/custom) - client (do (if (:ssl-context opts) + client (do (if (:ssl-context configured-opts) (.setSSLContext client-builder - (:ssl-context opts))) - (if force-redirects - (.setRedirectStrategy client-builder - (LaxRedirectStrategy.))) + (:ssl-context configured-opts))) + (.setRedirectStrategy client-builder + (async/redirect-strategy opts)) (.build client-builder))] (.start client) (reify common/HTTPClient diff --git a/test/puppetlabs/http/client/async_plaintext_test.clj b/test/puppetlabs/http/client/async_plaintext_test.clj index 734f2ec..9a6d6d1 100644 --- a/test/puppetlabs/http/client/async_plaintext_test.clj +++ b/test/puppetlabs/http/client/async_plaintext_test.clj @@ -203,10 +203,32 @@ response (async/post "http://localhost:8080/hello" opts)] (is (= 200 (:status @response))) (is (= "Hello, World!" (:body @response))))) + (testing (str "redirects not followed by clojure client when :follow-redirects " + "is set to false") + (let [response (async/get "http://localhost:8080/hello" {:as :text + :follow-redirects false})] + (is (= 302 (:status @response))))) + (testing ":follow-redirects overrides :force-redirects with clojure client" + (let [response (async/get "http://localhost:8080/hello" {:as :text + :follow-redirects false + :force-redirects true})] + (is (= 302 (:status @response))))) (testing (str "redirects on POST followed by persistent clojure client " "when option is set") (let [client (async/create-client {:force-redirects true}) response (common/post client "http://localhost:8080/hello" {:as :text})] (is (= 200 (:status @response))) (is (= "Hello, World!" (:body @response))) + (common/close client))) + (testing (str "persistent clojure client does not follow redirects when " + ":follow-redirects is set to false") + (let [client (async/create-client {:follow-redirects false}) + response (common/get client "http://localhost:8080/hello" {:as :text})] + (is (= 302 (:status @response))) + (common/close client))) + (testing ":follow-redirects overrides :force-redirects with persistent clj client" + (let [client (async/create-client {:follow-redirects false + :force-redirects true}) + response (common/get client "http://localhost:8080/hello" {:as :text})] + (is (= 302 (:status @response))) (common/close client)))))) \ No newline at end of file diff --git a/test/puppetlabs/http/client/sync_plaintext_test.clj b/test/puppetlabs/http/client/sync_plaintext_test.clj index cc6d943..90cc764 100644 --- a/test/puppetlabs/http/client/sync_plaintext_test.clj +++ b/test/puppetlabs/http/client/sync_plaintext_test.clj @@ -354,10 +354,32 @@ response (sync/post "http://localhost:8080/hello" opts)] (is (= 200 (:status response))) (is (= "Hello, World!" (:body response))))) + (testing (str "redirects not followed by clojure client when :follow-redirects " + "is set to false") + (let [response (sync/get "http://localhost:8080/hello" {:as :text + :follow-redirects false})] + (is (= 302 (:status response))))) + (testing ":follow-redirects overrides :force-redirects with clojure client" + (let [response (sync/get "http://localhost:8080/hello" {:as :text + :follow-redirects false + :force-redirects true})] + (is (= 302 (:status response))))) (testing (str "redirects on POST followed by persistent clojure client " "when option is set") (let [client (sync/create-client {:force-redirects true}) response (common/post client "http://localhost:8080/hello" {:as :text})] (is (= 200 (:status response))) (is (= "Hello, World!" (:body response))) + (common/close client))) + (testing (str "persistent clojure client does not follow redirects when " + ":follow-redirects is set to false") + (let [client (sync/create-client {:follow-redirects false}) + response (common/get client "http://localhost:8080/hello" {:as :text})] + (is (= 302 (:status response))) + (common/close client))) + (testing ":follow-redirects overrides :force-redirects with persistent clj client" + (let [client (sync/create-client {:follow-redirects false + :force-redirects true}) + response (common/get client "http://localhost:8080/hello" {:as :text})] + (is (= 302 (:status response))) (common/close client))))))