Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(maint) add support for multiple Set-Cookie headers in a response #104

Merged
merged 2 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## Unreleased

# 2.1.2
* add support for multiple `Set-Cookie` headers in a response separated by newlines.
* update project.clj to remove composite profiles that include maps, it is deprecated in lein 2.11.0

# 2.1.1
* [PE-34843](https://tickets.puppetlabs.com/browse/PE-34843) Properly reuse connections when using a client certificate
* Update to clj-parent 5.2.11
Expand Down
44 changes: 22 additions & 22 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

:min-lein-version "2.9.1"

:parent-project {:coords [puppetlabs/clj-parent "5.3.8"]
:parent-project {:coords [puppetlabs/clj-parent "5.6.7"]
:inherit [:managed-dependencies]}

;; Abort when version ranges or version conflicts are detected in
Expand Down Expand Up @@ -44,27 +44,27 @@
[puppetlabs/ring-middleware]]
:resource-paths ["dev-resources"]
:jvm-opts ["-Djava.util.logging.config.file=dev-resources/logging.properties"]}
:dev [:defaults
{:dependencies [[org.bouncycastle/bcpkix-jdk18on]]}]
:fips [:defaults
{:dependencies [[org.bouncycastle/bcpkix-fips]
[org.bouncycastle/bc-fips]
[org.bouncycastle/bctls-fips]]
;; this only ensures that we run with the proper profiles
;; during testing. This JVM opt will be set in the puppet module
;; that sets up the JVM classpaths during installation.
:jvm-opts ~(let [version (System/getProperty "java.version")
[major minor _] (clojure.string/split version #"\.")
unsupported-ex (ex-info "Unsupported major Java version. Expects 8 or 11."
{:major major
:minor minor})]
(condp = (java.lang.Integer/parseInt major)
1 (if (= 8 (java.lang.Integer/parseInt minor))
["-Djava.security.properties==dev-resources/jdk8-fips-security"]
(throw unsupported-ex))
11 ["-Djava.security.properties==dev-resources/jdk11-fips-security"]
17 ["-Djava.security.properties==dev-resources/jdk17-fips-security"]
(throw unsupported-ex)))}]
:dev-deps {:dependencies [[org.bouncycastle/bcpkix-jdk18on]]}
:dev [:defaults :dev-deps]
:fips-deps {:dependencies [[org.bouncycastle/bcpkix-fips]
[org.bouncycastle/bc-fips]
[org.bouncycastle/bctls-fips]]
;; this only ensures that we run with the proper profiles
;; during testing. This JVM opt will be set in the puppet module
;; that sets up the JVM classpaths during installation.
:jvm-opts ~(let [version (System/getProperty "java.version")
[major minor _] (clojure.string/split version #"\.")
unsupported-ex (ex-info "Unsupported major Java version. Expects 8 or 11."
{:major major
:minor minor})]
(condp = (java.lang.Integer/parseInt major)
1 (if (= 8 (java.lang.Integer/parseInt minor))
["-Djava.security.properties==dev-resources/jdk8-fips-security"]
(throw unsupported-ex))
11 ["-Djava.security.properties==dev-resources/jdk11-fips-security"]
17 ["-Djava.security.properties==dev-resources/jdk17-fips-security"]
(throw unsupported-ex)))}
:fips [:defaults :fips-deps]
:sources-jar {:java-source-paths ^:replace []
:jar-exclusions ^:replace []
:source-paths ^:replace ["src/clj" "src/java"]}}
Expand Down
18 changes: 17 additions & 1 deletion src/java/com/puppetlabs/http/client/impl/JavaClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,23 @@ private static void completeResponse(ResponseDeliveryDelegate responseDeliveryDe
try {
Map<String, String> headers = new HashMap<>();
for (Header h : httpResponse.getAllHeaders()) {
headers.put(h.getName().toLowerCase(), h.getValue());
String headerName = h.getName().toLowerCase();
// the http specs allow for multiple headers with the same name,
// but unfortunately since headers are stored in a map, this isn't
// possible without breaking changes. This adds special casing for the
// Set-Cookie header, to add entries separated by newlines.
if (headerName.equals("set-cookie")) {
String headerValue = headers.get("set-cookie");
if (headerValue != null) {
headers.put(headerName, headerValue + "\n" + h.getValue());
}
else
{
headers.put(headerName, h.getValue());
}
} else {
headers.put(headerName.toLowerCase(), h.getValue());
}
}
String origContentEncoding = headers.get("content-encoding");
if (requestOptions.getDecompressBody()) {
Expand Down
4 changes: 3 additions & 1 deletion test/puppetlabs/http/client/sync_plaintext_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
[_]
{:status 200
:body "cookie has been set"
:cookies {"session_id" {:value "session-id-hash"}}})
:cookies {"session_id" {:value "session-id-hash"}
"someothercookie" {:value "somevalue" :path "/" :secure true}}})

(defn check-cookie-handler
[req]
Expand Down Expand Up @@ -272,6 +273,7 @@
(let [client (Sync/createClient (ClientOptions.))]
(testing "Set a cookie using Java API"
(let [response (.get client (RequestOptions. "http://localhost:10000/cookietest"))]
(is (= "session_id=session-id-hash\nsomeothercookie=somevalue;Path=/;Secure" (.get (.getHeaders response) "set-cookie")))
(is (= 200 (.getStatus response)))))
(testing "Check if cookie still exists"
(let [response (.get client (RequestOptions. "http://localhost:10000/cookiecheck"))]
Expand Down
Loading