diff --git a/.github/workflows/snyk_merge.yaml b/.github/workflows/snyk_merge.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..ae200ca5711c82639d82583248965ef1fb78b101
--- /dev/null
+++ b/.github/workflows/snyk_merge.yaml
@@ -0,0 +1,29 @@
+---
+name: Snyk Clojure Merge
+
+on: push
+
+jobs:
+ snyk_clojure:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Connect to Twingate
+ uses: twingate/github-action@v1
+ with:
+ service-key: ${{ secrets.TWINGATE_PUBLIC_REPO_KEY }}
+ - name: checkout the current PR
+ uses: actions/checkout@v2
+ with:
+ fetch-depth: 1
+ persist-credentials: false
+ - name: Run Clojure Snyk Scan
+ id: scan
+ uses: puppetlabs/security-snyk-clojure-action@v2
+ with:
+ snykToken: ${{ secrets.SNYK_PE_TOKEN }}
+ snykOrg: 'puppet-enterprise'
+ snykProject: 'clj-http-client'
+ # snykPolicy: '.snyk'
+ - name: Check output
+ if: steps.scan.outputs.vulns != ''
+ run: echo "Vulnerabilities detected; ${{ steps.scan.outputs.vulns }}" && exit 1
diff --git a/.travis.yml b/.travis.yml
index 8bf36bbc8484667c2534f0be68ddba4af10b341a..93df768b07879844c33638191a158c7728ba9d7b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,16 +1,30 @@
+dist: bionic
language: clojure
-lein: 2.9.1
+lein: 2.9.10
jobs:
include:
- - stage: jdk8
- script: lein with-profile dev test
- jdk: openjdk8
+ # The OpenJDK versions used by Travis are pretty old, which causes
+ # problems with some of the tests. This pulls in a semi-recent version
+ # from the Ubuntu repos.
+ - name: jdk8
+ before_install:
+ - sudo rm -rf /usr/local/lib/jvm/
+ - sudo rm -rf /usr/lib/jvm/openjdk-8
+ - sudo apt-get install -y openjdk-8-jdk-headless
+ - export JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64/
+ script:
+ - lein with-profile dev test
#- # same as previous stage
# script: lein with-profile fips test
# jdk: openjdk8
- - stage: jdk11
- script: lein with-profile dev test
- jdk: openjdk11
+ - name: jdk11
+ before_install:
+ - sudo rm -rf /usr/local/lib/jvm/
+ - sudo rm -rf /usr/lib/jvm/openjdk-11
+ - sudo apt-get install -y openjdk-11-jdk-headless
+ - export JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64/
+ script:
+ - lein with-profile dev test
#- # same as previous stage
# script: lein with-profile fips test
# jdk: openjdk11
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d3140e520878529054a21cb6e4b871e31502d254..bb8b29cbc9517a74ad4eda6d973af32ed9af7f1f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,39 @@
+## Unreleased
+
+# 2.1.0
+* update to clj-parent 5.2.6 to move from bouncy-castle `15on` libraries to the `18on` version
+* Use `OPTIONS` request method when calling the synchronous client's `options` function.
+
+## 2.0.0
+* [PE-33177](https://tickets.puppetlabs.com/browse/PE-33177) Add TLSv1.3 to ClientOptions default SSL protocols and remove TLSv1 and TLSv1.1.
+
+## 1.2.4
+* Public release of 1.2.3, no other changes.
+
+## 1.2.3
+* [SERVER-3068](https://tickets.puppetlabs.com/browse/SERVER-3068) Conditionally allow auth headers when being redirected.
+
+## 1.2.2
+* Internal release of 1.2.1, no other changes.
+
+## 1.2.1
+* [SERVER-3068](https://tickets.puppetlabs.com/browse/SERVER-3068) Do not copy auth headers when being redirected.
+
+## 1.2.0
+* [SERVER-2780](https://tickets.puppetlabs.com/browse/SERVER-2780) Add reason phrase to HTTP response
+
+## 1.1.3
+* [PE-27189]((https://tickets.puppetlabs.com/browse/PE-27189) Disambiguate ambiguous type specifications
+
+## 1.1.2
+* [PE-26658](https://tickets.puppetlabs.com/browse/PE-26658) Apply JVM security policy to FIPS
+
+## 1.1.1
+* Released by automation w/o content
+
+## 1.1.0
+* [PDB-4357](https://tickets.puppetlabs.com/browse/PDB-4357) FIPS support via bc-fips jar
+
## 1.0.0
* Adds `::max-connections-total` and `::max-connections-per-route` to allow the Apache HTTP client values to be overridden. They default to 20 and 2 (to correlate to the Apache client behavior) respectively.
* Update to clj-parent 1.7.12. This moves the library to require Java 8.
diff --git a/CODEOWNERS b/CODEOWNERS
index f9553e5d03d937043cc801c60e5091b5699273d0..97c7df0f21240ae6e57a2125b19d196c4927122d 100644
--- a/CODEOWNERS
+++ b/CODEOWNERS
@@ -1,4 +1 @@
-# This will cause the puppetserver-maintainers group to be assigned
-# review of any opened PRs against the branches containing this file.
-
-* @puppetlabs/puppetserver-maintainers
+* @puppetlabs/dumpling
diff --git a/README.md b/README.md
index 58b5c6f3705c683869107f8cd7f5e498fb3dc88c..5684b7302a944467bd4bacefdd642cdbb50f3472 100644
--- a/README.md
+++ b/README.md
@@ -34,6 +34,14 @@ The repo contains these files needed for testing, though if needed you may
want to read `dev-resources/gen-pki.sh` for the commands to generate additional
sets of files.
+## Upgrading dependencies
+
+The APIs provided by httpclient change significantly in 5.0 and some of the
+internal classes that we've extended change within minor releases of the 4.5
+series. Whenever upgrading the apache/httpcomponents dependencies an audit of
+the Java classes should be undertaken, especially the classes pertaining to
+SafeRedirectedRequest and RedirectStrategy.
+
## Support
We use the [Trapperkeeper project on JIRA](https://tickets.puppetlabs.com/browse/TK)
diff --git a/project.clj b/project.clj
index 576c490fb0fbc56087d9944ac471cf49c223e891..f78cb04beb3f08f29060151c8de9741e539090de 100644
--- a/project.clj
+++ b/project.clj
@@ -1,11 +1,11 @@
-(defproject puppetlabs/http-client "1.2.0"
+(defproject puppetlabs/http-client "2.1.0"
:description "HTTP client wrapper"
:license {:name "Apache License, Version 2.0"
:url "http://www.apache.org/licenses/LICENSE-2.0.html"}
:min-lein-version "2.9.1"
- :parent-project {:coords [puppetlabs/clj-parent "4.2.10"]
+ :parent-project {:coords [puppetlabs/clj-parent "5.2.6"]
:inherit [:managed-dependencies]}
;; Abort when version ranges or version conflicts are detected in
@@ -34,7 +34,8 @@
;; depend on this source jar using a :classifier in their :dependencies.
:classifiers [["sources" :sources-jar]]
- :profiles {:defaults {:dependencies [[cheshire]
+ :profiles {:provided {:dependencies [[org.bouncycastle/bcpkix-jdk18on]]}
+ :defaults {:dependencies [[cheshire]
[puppetlabs/kitchensink :classifier "test"]
[puppetlabs/trapperkeeper]
[puppetlabs/trapperkeeper :classifier "test"]
@@ -44,7 +45,7 @@
:resource-paths ["dev-resources"]
:jvm-opts ["-Djava.util.logging.config.file=dev-resources/logging.properties"]}
:dev [:defaults
- {:dependencies [[org.bouncycastle/bcpkix-jdk15on]]}]
+ {:dependencies [[org.bouncycastle/bcpkix-jdk18on]]}]
:fips [:defaults
{:dependencies [[org.bouncycastle/bcpkix-fips]
[org.bouncycastle/bc-fips]
@@ -78,5 +79,5 @@
:plugins [[lein-parent "0.3.7"]
[puppetlabs/i18n "0.8.0"]]
- :repositories [["puppet-releases" "https://artifactory.delivery.puppetlabs.net/artifactory/list/clojure-releases__local/"]
- ["puppet-snapshots" "https://artifactory.delivery.puppetlabs.net/artifactory/list/clojure-snapshots__local/"]])
+ :repositories [["puppet-releases" "https://artifactory.delivery.puppetlabs.net/artifactory/clojure-releases__local/"]
+ ["puppet-snapshots" "https://artifactory.delivery.puppetlabs.net/artifactory/clojure-snapshots__local/"]])
diff --git a/src/clj/puppetlabs/http/client/sync.clj b/src/clj/puppetlabs/http/client/sync.clj
index e9d345428668c6fdc4a30a9532d85ba8b845d7b7..621c8f094bd35fde1f5092c749556fe96682e051 100644
--- a/src/clj/puppetlabs/http/client/sync.clj
+++ b/src/clj/puppetlabs/http/client/sync.clj
@@ -66,7 +66,7 @@
(trace [this url] (common/trace this url {}))
(trace [this url opts] (common/make-request this url :trace opts))
(options [this url] (common/options this url {}))
- (options [this url opts] (common/make-request this url :post opts))
+ (options [this url opts] (common/make-request this url :options opts))
(patch [this url] (common/patch this url {}))
(patch [this url opts] (common/make-request this url :patch opts))
(make-request [this url method] (common/make-request this url method {}))
diff --git a/src/java/com/puppetlabs/http/client/ClientOptions.java b/src/java/com/puppetlabs/http/client/ClientOptions.java
index 1a68b439b35c6adcfc7c2add15670dbe96e7b889..033b9f3b02cdc81f23e9525df00d5de3e5ea28cc 100644
--- a/src/java/com/puppetlabs/http/client/ClientOptions.java
+++ b/src/java/com/puppetlabs/http/client/ClientOptions.java
@@ -18,7 +18,7 @@ import java.security.Security;
*/
public class ClientOptions {
public static final String[] DEFAULT_SSL_PROTOCOLS =
- new String[] {"TLSv1", "TLSv1.1", "TLSv1.2"};
+ new String[] {"TLSv1.3", "TLSv1.2"};
private SSLContext sslContext;
private String sslCert;
diff --git a/src/java/com/puppetlabs/http/client/impl/CreateRedirectUtil.java b/src/java/com/puppetlabs/http/client/impl/CreateRedirectUtil.java
new file mode 100644
index 0000000000000000000000000000000000000000..fe1482b19baca946ee71915f9b2209e3c273cdf1
--- /dev/null
+++ b/src/java/com/puppetlabs/http/client/impl/CreateRedirectUtil.java
@@ -0,0 +1,79 @@
+package com.puppetlabs.http.client.impl;
+
+import org.apache.http.*;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.client.methods.RequestBuilder;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.impl.client.DefaultRedirectStrategy;
+import org.apache.http.protocol.HttpContext;
+
+import java.net.URI;
+import java.util.Arrays;
+import java.util.List;
+
+// This class overrides the getRedirect() method of DefaultRedirectStrategy
+// (or LaxRedirectStrategy as it inherits this method from DefaultRedirectStrategy)
+// so that we do not copy the auth headers for non get or head requests, and the newly
+// created request is wrapped in a SafeRedirectedRequest. SafeRedirectedRequest
+// will proxy method invocations to the wrapped request, but intercepts setHeaders(),
+// which is used by callers to copy over headers from the original request.
+//
+// See https://stackoverflow.com/questions/17970633/header-values-overwritten-on-redirect-in-httpclient
+// for the inspiration for this work.
+//
+// Note: This implementation is VERY version specific and will need to be
+// revised whenever updating httpclient.
+public class CreateRedirectUtil {
+ public static final int SC_PERMANENT_REDIRECT = 308;
+ public static final List<String> SECURITY_RELATED_HEADERS = Arrays.asList(
+ "X-Authorization", "Authorization",
+ "Cookie", "Set-Cookie", "WWW-Authenticate",
+ "Proxy-Authorization", "Proxy-Authenticate"
+ );
+
+ public static HttpUriRequest getRedirect(
+ final DefaultRedirectStrategy redirectStrategy,
+ final HttpRequest request,
+ final HttpResponse response,
+ final HttpContext context) throws ProtocolException {
+
+ final URI uri = redirectStrategy.getLocationURI(request, response, context);
+ final String method = request.getRequestLine().getMethod();
+
+ // This is new to allow Redirects to contain Auth headers IF they are to the same host/port/scheme
+ final HttpClientContext clientContext = HttpClientContext.adapt(context);
+ final HttpHost target = clientContext.getTargetHost();
+ final boolean localRedirect = target.getHostName().equals(uri.getHost()) &&
+ target.getPort() == uri.getPort() &&
+ target.getSchemeName().equals(uri.getScheme());
+
+
+ if (method.equalsIgnoreCase(HttpHead.METHOD_NAME)) {
+ return localRedirect ? new HttpHead(uri) : SafeRedirectedRequest.wrap(new HttpHead(uri));
+ } else if (method.equalsIgnoreCase(HttpGet.METHOD_NAME)) {
+ return localRedirect ? new HttpGet(uri) : SafeRedirectedRequest.wrap(new HttpGet(uri));
+ } else {
+ final int status = response.getStatusLine().getStatusCode();
+ if (status == HttpStatus.SC_TEMPORARY_REDIRECT || status == SC_PERMANENT_REDIRECT) {
+
+ if (! localRedirect) {
+ // RequestBuilder.copy will copy any existing headers, which we don't want
+ RequestBuilder builder = RequestBuilder.copy(request).setUri(uri);
+ for (String header : SECURITY_RELATED_HEADERS) {
+ // .removeHeaders does an equalsIgnoreCase() on the passed String.
+ builder.removeHeaders(header);
+ }
+
+ return SafeRedirectedRequest.wrap(builder.build());
+ } else {
+ return RequestBuilder.copy(request).setUri(uri).build();
+ }
+ } else {
+ return localRedirect ? new HttpGet(uri) : SafeRedirectedRequest.wrap(new HttpGet(uri));
+ }
+ }
+ }
+
+}
diff --git a/src/java/com/puppetlabs/http/client/impl/JavaClient.java b/src/java/com/puppetlabs/http/client/impl/JavaClient.java
index ef0d8f2d07a85d25287e852e88538137d2e95f65..78406d96da7f6f92e91a0dd407d23a41d4456843 100644
--- a/src/java/com/puppetlabs/http/client/impl/JavaClient.java
+++ b/src/java/com/puppetlabs/http/client/impl/JavaClient.java
@@ -8,6 +8,8 @@ import com.puppetlabs.http.client.HttpMethod;
import com.puppetlabs.http.client.RequestOptions;
import com.puppetlabs.http.client.ResponseBodyType;
import com.puppetlabs.http.client.impl.metrics.TimerUtils;
+import com.puppetlabs.http.client.impl.SafeDefaultRedirectStrategy;
+import com.puppetlabs.http.client.impl.SafeLaxRedirectStrategy;
import com.puppetlabs.ssl_utils.SSLUtils;
import org.apache.commons.io.IOUtils;
@@ -39,8 +41,6 @@ import org.apache.http.concurrent.FutureCallback;
import org.apache.http.conn.ssl.DefaultHostnameVerifier;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.InputStreamEntity;
-import org.apache.http.impl.client.DefaultRedirectStrategy;
-import org.apache.http.impl.client.LaxRedirectStrategy;
import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
import org.apache.http.impl.nio.client.HttpAsyncClients;
@@ -558,10 +558,10 @@ public class JavaClient {
};
}
else if (coercedOptions.getForceRedirects()) {
- redirectStrategy = new LaxRedirectStrategy();
+ redirectStrategy = new SafeLaxRedirectStrategy();
}
else {
- redirectStrategy = new DefaultRedirectStrategy();
+ redirectStrategy = new SafeDefaultRedirectStrategy();
}
clientBuilder.setRedirectStrategy(redirectStrategy);
diff --git a/src/java/com/puppetlabs/http/client/impl/SafeDefaultRedirectStrategy.java b/src/java/com/puppetlabs/http/client/impl/SafeDefaultRedirectStrategy.java
new file mode 100644
index 0000000000000000000000000000000000000000..8fa81593cb003a291e4d345119917ae301a9a7d7
--- /dev/null
+++ b/src/java/com/puppetlabs/http/client/impl/SafeDefaultRedirectStrategy.java
@@ -0,0 +1,27 @@
+package com.puppetlabs.http.client.impl;
+
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.ProtocolException;
+import org.apache.http.annotation.Contract;
+import org.apache.http.annotation.ThreadingBehavior;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.impl.client.DefaultRedirectStrategy;
+import org.apache.http.protocol.HttpContext;
+
+// Shares behavior of CreateRedirectUtil.getRedirected() with SafeLaxRedirectStrategy
+@Contract(threading = ThreadingBehavior.IMMUTABLE)
+public class SafeDefaultRedirectStrategy extends DefaultRedirectStrategy {
+
+ // Used by the Apache HttpClientBuilder internally
+ public static final SafeDefaultRedirectStrategy INSTANCE = new SafeDefaultRedirectStrategy();
+
+ @Override
+ public HttpUriRequest getRedirect(
+ final HttpRequest request,
+ final HttpResponse response,
+ final HttpContext context) throws ProtocolException {
+
+ return CreateRedirectUtil.getRedirect(this, request, response, context);
+ }
+}
diff --git a/src/java/com/puppetlabs/http/client/impl/SafeLaxRedirectStrategy.java b/src/java/com/puppetlabs/http/client/impl/SafeLaxRedirectStrategy.java
new file mode 100644
index 0000000000000000000000000000000000000000..ac01c900000f4d988a08cfa045e5aaaef17acc85
--- /dev/null
+++ b/src/java/com/puppetlabs/http/client/impl/SafeLaxRedirectStrategy.java
@@ -0,0 +1,28 @@
+package com.puppetlabs.http.client.impl;
+
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.ProtocolException;
+import org.apache.http.annotation.Contract;
+import org.apache.http.annotation.ThreadingBehavior;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.impl.client.LaxRedirectStrategy;
+import org.apache.http.protocol.HttpContext;
+
+// Shares behavior of CreateRedirectUtil.getRedirected() with SafeDefaultRedirectStrategy
+@Contract(threading = ThreadingBehavior.IMMUTABLE)
+public class SafeLaxRedirectStrategy extends LaxRedirectStrategy {
+
+ // Used by the Apache HttpClientBuilder internally
+ public static final SafeLaxRedirectStrategy INSTANCE = new SafeLaxRedirectStrategy();
+
+ @Override
+ public HttpUriRequest getRedirect(
+ final HttpRequest request,
+ final HttpResponse response,
+ final HttpContext context) throws ProtocolException {
+
+ return CreateRedirectUtil.getRedirect(this, request, response, context);
+
+ }
+}
diff --git a/src/java/com/puppetlabs/http/client/impl/SafeRedirectedRequest.java b/src/java/com/puppetlabs/http/client/impl/SafeRedirectedRequest.java
new file mode 100644
index 0000000000000000000000000000000000000000..0e2fa238e220fc4008a394d4d8098ef79c6484d5
--- /dev/null
+++ b/src/java/com/puppetlabs/http/client/impl/SafeRedirectedRequest.java
@@ -0,0 +1,66 @@
+package com.puppetlabs.http.client.impl;
+
+import java.lang.reflect.Proxy;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+
+import org.apache.http.Header;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpUriRequest;
+
+// To use this class call SafeRedirectedRequest.wrap(HttpUriRequest). Will
+// wrap the given request and proxy all methods to it, except for setHeaders,
+// which we implement and filter out potentially unsafe headers from.
+//
+// See https://stackoverflow.com/questions/30344715/automatically-delegating-all-methods-of-a-java-class
+// for inspiration for this work.
+public class SafeRedirectedRequest
+ // We extend HttpGet to satisfy the requirement to have implementations
+ // of the HttpUriRequest, but proxy all invocations to the wrapped delegate.
+ extends HttpGet
+ implements HttpUriRequest, InvocationHandler {
+
+ private final HttpUriRequest delegate;
+
+ public SafeRedirectedRequest(HttpUriRequest delegate) {
+ this.delegate = delegate;
+ }
+
+ public static HttpUriRequest wrap(HttpUriRequest wrapped) {
+ return (HttpUriRequest) Proxy.newProxyInstance(HttpUriRequest.class.getClassLoader(),
+ new Class[]{HttpUriRequest.class},
+ new SafeRedirectedRequest(wrapped));
+ }
+
+ // There are other ways to set headers (setHeader(String, String),
+ // setHeader(Header)), however this is the method currently used to
+ // copy exiting headers when being redirected.
+ @Override
+ public void setHeaders(final Header[] headers) {
+ final Header[] cleanedHeaders = Arrays.stream(headers).filter(header ->
+ CreateRedirectUtil.SECURITY_RELATED_HEADERS.stream().noneMatch(mask ->
+ mask.equalsIgnoreCase(header.getName())))
+ .toArray(Header[]::new);
+ delegate.setHeaders(cleanedHeaders);
+ }
+
+ // Begin Proxy/InvocationHandler implementation
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+ Method m = findMethod(this.getClass(), method);
+ if (m != null) {
+ return m.invoke(this, args);
+ } else {
+ return method.invoke(this.delegate, args);
+ }
+ }
+
+ private Method findMethod(Class<?> clazz, Method method) throws Throwable {
+ try {
+ return clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());
+ } catch (NoSuchMethodException e) {
+ return null;
+ }
+ }
+}
diff --git a/test/puppetlabs/http/client/async_plaintext_test.clj b/test/puppetlabs/http/client/async_plaintext_test.clj
index b90b86f304be2f73c72115805f94e670ec2461bb..403089d9b39c931474be246ba7fcb86a9dfb3e8a 100644
--- a/test/puppetlabs/http/client/async_plaintext_test.clj
+++ b/test/puppetlabs/http/client/async_plaintext_test.clj
@@ -1,10 +1,11 @@
(ns puppetlabs.http.client.async-plaintext-test
- (:import (com.puppetlabs.http.client Async RequestOptions ClientOptions)
+ (:import (com.puppetlabs.http.client Async RequestOptions ClientOptions ResponseBodyType)
(org.apache.http.impl.nio.client HttpAsyncClients)
(java.net URI SocketTimeoutException ServerSocket URL)
(java.util Locale)
(java.util.concurrent CountDownLatch TimeUnit))
(:require [clojure.test :refer :all]
+ [clojure.set :as set]
[puppetlabs.http.client.test-common :refer :all]
[puppetlabs.i18n.core :as i18n]
[puppetlabs.trapperkeeper.core :as tk]
@@ -110,9 +111,7 @@
(is (= 200 (.getStatus (.deref response))))
(is (= "Hello, World!" (slurp (.getBody (.deref response)))))))
(testing "client closes properly"
- (.close client)
- (is (thrown? IllegalStateException
- (.get client request-options))))))
+ (is (= nil (.close client))))))
(testing "clojure async client"
(let [client (async/create-client {})]
(testing "HEAD request to missing endpoint"
@@ -162,10 +161,7 @@
"http://localhost:10000/hello/"
:bad))))
(testing "client closes properly"
- (common/close client)
- (is (thrown? IllegalStateException
- (common/get client
- "http://localhost:10000/hello/")))))))))
+ (is (= nil (common/close client)))))))))
(deftest java-api-cookie-test
(testlogging/with-test-logging
@@ -270,6 +266,180 @@
(is (= 200 (:status @response)))
(is (= queryparams (read-string (:body @response))))))))))
+
+(defn create-redirect-web-service
+ [state]
+ (tk/defservice redirect-web-service-with-state
+ [[:WebserverService add-ring-handler]]
+ (init [this context]
+ (add-ring-handler
+ (fn
+ [{:keys [uri headers] :as req}]
+ (try
+ (swap! state #(conj % {:uri uri :headers headers}))
+ (condp = uri
+ "/hello" {:status 302
+ :headers {"Location" "http://localhost:8082/world"}
+ :body ""}
+ "/hello/foo" {:status 302
+ :headers {"Location" "/hello/bar"}
+ :body ""}
+ "/hello/bar" {:status 200 :body "It's me, I'm still here ...and I'm still me."}
+ {:status 404 :body "D'oh"})
+ (catch Throwable e
+ (prn e))))
+ "/hello"
+ {:server-id :hello})
+ (add-ring-handler
+ (fn
+ [{:keys [uri headers] :as req}]
+ (try
+ (swap! state #(conj % {:uri uri :headers headers}))
+ (condp = uri
+ "/world" {:status 200 :body "Hello, World!"}
+ {:status 404 :body "D'oh"})
+ (catch Throwable e
+ (prn e))))
+ "/world"
+ {:server-id :world})
+ context)))
+
+(deftest redirect-headers-test-async
+ ;(testlogging/with-test-logging
+ (let [state (atom [])
+ headers {"X-Authorization" "foo", "Authorization" "bar",
+ "Cookie" "123", "Set-Cookie" "true",
+ "WWW-Authenticate" "Basic token",
+ "Proxy-Authorization" "Basic token", "Proxy-Authenticate" "Basic"}
+ header-names #{"x-authorization" "authorization" "cookie" "set-cookie"
+ "www-authenticate" "proxy-authorization" "proxy-authenticate"
+ "X-Authorization" "Authorizaion"
+ "Cookie" "Set-Cookie"
+ "WWW-Authenticate", "Proxy-Authorization", "Proxy-Authenticate"}]
+ (create-redirect-web-service state)
+ (testutils/with-app-with-config app
+ [jetty9/jetty9-service redirect-web-service-with-state]
+ {:webserver {:hello {:port 8081} :world {:port 8082}}}
+ (testing "default redirect policy does not include authorization headers"
+ (let [client (Async/createClient (ClientOptions.))
+ request-options (RequestOptions.
+ (URI. "http://localhost:8081/hello")
+ headers
+ nil
+ true
+ ResponseBodyType/TEXT)]
+ (try
+ (testing "GET requests"
+ (let [response (.deref (.get client request-options))
+ [first-req second-req] @state]
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello" (:uri first-req)))
+ (is (= "/world" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (not-any? header-names (keys (:headers second-req))))))
+ (reset! state [])
+ (testing "HEAD requests"
+ (let [response (.deref (.head client request-options))
+ [first-req second-req] @state]
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello" (:uri first-req)))
+ (is (= "/world" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (not-any? header-names (keys (:headers second-req))))))
+ (reset! state [])
+ (testing "POST requests fail"
+ (let [response (.deref (.post client request-options))
+ [first-req second-req] @state]
+ (is (= 302 (.getStatus response)))
+ (is (= "/hello" (:uri first-req)))
+ (is (nil? second-req))))
+ (reset! state [])
+ (testing "Pass auth headers when redirected to URI with same scheme, host, and port"
+ (let [request-options (RequestOptions.
+ (URI. "http://localhost:8081/hello/foo")
+ headers
+ nil
+ true
+ ResponseBodyType/TEXT)
+ response (.deref (.get client request-options))
+ [first-req second-req] @state]
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello/foo" (:uri first-req)))
+ (is (= "/hello/bar" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers second-req)))))))))
+ (finally
+ (.close client)))))
+
+ ;; NOTE: apache http client does not currently support redirects for PUT under any circumstance
+ (testing "lax redirect policy does not include authorization headers"
+ (let [client (Async/createClient (.. (ClientOptions.)
+ (setForceRedirects true)))
+ request-options (RequestOptions.
+ (URI. "http://localhost:8081/hello")
+ headers
+ nil
+ true
+ ResponseBodyType/TEXT)]
+ (try
+ (reset! state [])
+ (testing "GET requests"
+ (let [response (.deref (.get client request-options))
+ [first-req second-req] @state]
+ (is (= 2 (count @state)))
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello" (:uri first-req)))
+ (is (= "/world" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (not-any? header-names (keys (:headers second-req))))))
+ (reset! state [])
+ (testing "HEAD requests"
+ (let [response (.deref (.head client request-options))
+ [first-req second-req] @state]
+ (is (= 2 (count @state)))
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello" (:uri first-req)))
+ (is (= "/world" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (not-any? header-names (keys (:headers second-req))))))
+ (reset! state [])
+ (testing "POST requests"
+ (let [response (.deref (.post client request-options))
+ [first-req second-req] @state]
+ (is (= 2 (count @state)))
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello" (:uri first-req)))
+ (is (= "/world" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (not-any? header-names (keys (:headers second-req))))))
+ (reset! state [])
+ (testing "DELETE requests"
+ (let [response (.deref (.delete client request-options))
+ [first-req second-req] @state]
+ (is (= 2 (count @state)))
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello" (:uri first-req)))
+ (is (= "/world" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (not-any? header-names (keys (:headers second-req))))))
+ (reset! state [])
+ (testing "Pass auth headers when redirected to URI with same scheme, host, and port"
+ (let [ request-options (RequestOptions.
+ (URI. "http://localhost:8081/hello/foo")
+ headers
+ nil
+ true
+ ResponseBodyType/TEXT)
+ response (.deref (.get client request-options))
+ [first-req second-req] @state]
+ (is (= 200 (.getStatus response)))
+ (is (= "/hello/foo" (:uri first-req)))
+ (is (= "/hello/bar" (:uri second-req)))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers first-req)))))))
+ (is (= 7 (count (set/intersection header-names (set (keys (:headers second-req)))))))))
+ (finally
+ (.close client))))))))
+
(deftest redirect-test-async
(testlogging/with-test-logging
(testutils/with-app-with-config app
diff --git a/test/puppetlabs/http/client/sync_plaintext_test.clj b/test/puppetlabs/http/client/sync_plaintext_test.clj
index 3ce029bee181bef21c698b4f0ebc58fd25d08c76..86eb753495ebbd7d92ccdd0d05bced003d946f0a 100644
--- a/test/puppetlabs/http/client/sync_plaintext_test.clj
+++ b/test/puppetlabs/http/client/sync_plaintext_test.clj
@@ -154,7 +154,7 @@
(is (= "Hello, World!" (slurp (.getBody response))))))
(testing "client closes properly"
(.close client)
- (is (thrown? IllegalStateException
+ (is (thrown? HttpClientException
(.get client request-options))))))
(testing "persistent clojure client"
(let [client (sync/create-client {})]
diff --git a/test/puppetlabs/http/client/sync_ssl_test.clj b/test/puppetlabs/http/client/sync_ssl_test.clj
index 3df7d1d32cdb9811d9ae452b579e5ce67db1cefd..c8dd77b115f08d6b7df20d0b6560fa441d210e58 100644
--- a/test/puppetlabs/http/client/sync_ssl_test.clj
+++ b/test/puppetlabs/http/client/sync_ssl_test.clj
@@ -140,7 +140,8 @@
(and (instance? SSLException cause#)
(or (re-find #"handshake_failure" message#)
(re-find #"internal_error" message#)))
- (instance? ConnectionClosedException cause#))))))
+ (instance? ConnectionClosedException cause#))))
+ (catch ConnectionClosedException cce# true)))
(defn java-https-get-with-protocols
[client-protocols client-cipher-suites]
@@ -183,5 +184,5 @@
(is (java-unsupported-protocol-exception?
(java-https-get-with-protocols ["TLSv1.2"] nil))))
(testing "clojure sync client"
- (is (thrown? SSLException (clj-https-get-with-protocols ["TLSv1.2"] nil)))))))
+ (is (java-unsupported-protocol-exception? (clj-https-get-with-protocols ["TLSv1.2"] nil)))))))