[Git][clojure-team/puppetlabs-http-client-clojure][debian/main] 6 commits: New upstream version 1.2.0

Jérôme Charaoui (@lavamind) gitlab at salsa.debian.org
Fri Oct 21 05:06:02 BST 2022



Jérôme Charaoui pushed to branch debian/main at Debian Clojure Maintainers / puppetlabs-http-client-clojure


Commits:
cd760a38 by Louis-Philippe Véronneau at 2020-12-27T18:59:30-05:00
New upstream version 1.2.0
- - - - -
8299f363 by Jérôme Charaoui at 2022-10-18T21:51:08-04:00
New upstream version 2.1.0
- - - - -
9a93d423 by Jérôme Charaoui at 2022-10-18T21:51:10-04:00
Update upstream source from tag 'upstream/2.1.0'

Update to upstream version '2.1.0'
with Debian dir 77a4433b80a0d4ec744d1335a30ac077f572dc57
- - - - -
786ffc31 by Jérôme Charaoui at 2022-10-20T23:53:29-04:00
d/patches: rebase for new upstream release

- - - - -
0aa37a02 by Jérôme Charaoui at 2022-10-20T23:53:31-04:00
d/patches: test fixed upstream, reenable

- - - - -
8c6167f2 by Jérôme Charaoui at 2022-10-20T23:54:35-04:00
Update changelog for 2.1.0-1 release

- - - - -


21 changed files:

- + .github/workflows/snyk_merge.yaml
- .travis.yml
- CHANGELOG.md
- CODEOWNERS
- README.md
- debian/changelog
- debian/patches/0001_Lein_Local.patch
- debian/patches/0003-Avoid-exit-code-1-when-running-gen-pki.sh-during-tes.patch → debian/patches/0002-Avoid-exit-code-1-when-running-gen-pki.sh-during-tes.patch
- − debian/patches/0002_Fix_Testsuite_Failure.patch
- debian/patches/series
- project.clj
- src/clj/puppetlabs/http/client/sync.clj
- src/java/com/puppetlabs/http/client/ClientOptions.java
- + src/java/com/puppetlabs/http/client/impl/CreateRedirectUtil.java
- src/java/com/puppetlabs/http/client/impl/JavaClient.java
- + src/java/com/puppetlabs/http/client/impl/SafeDefaultRedirectStrategy.java
- + src/java/com/puppetlabs/http/client/impl/SafeLaxRedirectStrategy.java
- + src/java/com/puppetlabs/http/client/impl/SafeRedirectedRequest.java
- test/puppetlabs/http/client/async_plaintext_test.clj
- test/puppetlabs/http/client/sync_plaintext_test.clj
- test/puppetlabs/http/client/sync_ssl_test.clj


Changes:

=====================================
.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 at v1
+      with:
+        service-key: ${{ secrets.TWINGATE_PUBLIC_REPO_KEY }}
+    - name: checkout the current PR
+      uses: actions/checkout at v2
+      with:
+        fetch-depth: 1
+        persist-credentials: false
+    - name: Run Clojure Snyk Scan
+      id: scan
+      uses: puppetlabs/security-snyk-clojure-action at 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


=====================================
.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


=====================================
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..


=====================================
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


=====================================
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)


=====================================
debian/changelog
=====================================
@@ -1,3 +1,14 @@
+puppetlabs-http-client-clojure (2.1.0-1) unstable; urgency=medium
+
+  * Team upload.
+  * New upstream version 2.1.0
+  * d/patches:
+    + rebase patches for new upstream version
+    + test fixed upstream, reenable
+  * disable i386 build in salsa-ci
+
+ -- Jérôme Charaoui <jerome at riseup.net>  Tue, 18 Oct 2022 21:51:35 -0400
+
 puppetlabs-http-client-clojure (1.2.0-7) unstable; urgency=medium
 
   * Team upload.


=====================================
debian/patches/0001_Lein_Local.patch
=====================================
@@ -4,18 +4,18 @@ Subject: Patch project.clj to build locally using lein.
 
 Forwarded: not-needed
 ---
- project.clj | 58 +++++++++++++++++++++++++++++++---------------------------
- 1 file changed, 31 insertions(+), 27 deletions(-)
+ project.clj | 59 +++++++++++++++++++++++++++++++----------------------------
+ 1 file changed, 31 insertions(+), 28 deletions(-)
 
 diff --git a/project.clj b/project.clj
-index 576c490..8db6c3c 100644
+index f78cb04..7644a51 100644
 --- a/project.clj
 +++ b/project.clj
 @@ -5,25 +5,22 @@
  
    :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]}
 +  :dependencies [[org.clojure/clojure "1.10.x"]
  
@@ -49,11 +49,12 @@ index 576c490..8db6c3c 100644
  
    :source-paths ["src/clj"]
    :java-source-paths ["src/java"]
-@@ -34,17 +31,23 @@
+@@ -34,18 +31,23 @@
    ;; 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"]
@@ -76,12 +77,12 @@ index 576c490..8db6c3c 100644
                          :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]]}]
 +                   {:dependencies [[org.bouncycastle/bcpkix "debian"]]}]
               :fips [:defaults
                      {:dependencies [[org.bouncycastle/bcpkix-fips]
                                      [org.bouncycastle/bc-fips]
-@@ -75,8 +78,9 @@
+@@ -76,8 +78,9 @@
    :lein-release {:scm :git
                   :deploy-via :lein-deploy}
  
@@ -92,6 +93,6 @@ index 576c490..8db6c3c 100644
 +                                                   [com.fasterxml.jackson.dataformat/jackson-dataformat-smile]
 +                                                   [com.fasterxml.jackson.dataformat/jackson-dataformat-cbor]]]]
  
--  :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/"]])
 +  :local-repo "debian/maven-repo")


=====================================
debian/patches/0003-Avoid-exit-code-1-when-running-gen-pki.sh-during-tes.patch → debian/patches/0002-Avoid-exit-code-1-when-running-gen-pki.sh-during-tes.patch
=====================================


=====================================
debian/patches/0002_Fix_Testsuite_Failure.patch deleted
=====================================
@@ -1,18 +0,0 @@
-This part of the testsuite fails, but the failure seems very minor and it can
-be reproduced using upstream's dev environment (so probably not a result of
-something broken in Debian).
-See https://github.com/puppetlabs/clj-http-client/issues/85 for more details.
-Index: puppetlabs-http-client-clojure/test/puppetlabs/http/client/sync_ssl_test.clj
-===================================================================
---- puppetlabs-http-client-clojure.orig/test/puppetlabs/http/client/sync_ssl_test.clj
-+++ puppetlabs-http-client-clojure/test/puppetlabs/http/client/sync_ssl_test.clj
-@@ -182,6 +182,6 @@
-       (testing "java sync client"
-         (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)))))))
--
-+      ;; (testing "clojure sync client"
-+      ;;   (is (thrown? SSLException (clj-https-get-with-protocols ["TLSv1.2"] nil)))))))
-+        )))


=====================================
debian/patches/series
=====================================
@@ -1,3 +1,2 @@
 0001_Lein_Local.patch
-0002_Fix_Testsuite_Failure.patch
-0003-Avoid-exit-code-1-when-running-gen-pki.sh-during-tes.patch
+0002-Avoid-exit-code-1-when-running-gen-pki.sh-during-tes.patch


=====================================
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/"]])


=====================================
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 {}))


=====================================
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;


=====================================
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));
+            }
+        }
+    }
+
+}


=====================================
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);
 


=====================================
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
+ at 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);
+    }
+}


=====================================
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
+ at 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);
+
+    }
+}


=====================================
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;
+        }
+    }
+}


=====================================
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


=====================================
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 {})]


=====================================
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)))))))
 



View it on GitLab: https://salsa.debian.org/clojure-team/puppetlabs-http-client-clojure/-/compare/f3be3a635cd50702b7105f4ea7cef715e4dbc08e...8c6167f2ee55f82b9c62d095eea176b5fd6cf260

-- 
View it on GitLab: https://salsa.debian.org/clojure-team/puppetlabs-http-client-clojure/-/compare/f3be3a635cd50702b7105f4ea7cef715e4dbc08e...8c6167f2ee55f82b9c62d095eea176b5fd6cf260
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-java-commits/attachments/20221021/e49af30f/attachment.htm>


More information about the pkg-java-commits mailing list