From cb5de4555d461a2c7c806e95c96f57b05e256dc2 Mon Sep 17 00:00:00 2001 From: Jeremy Barlow Date: Wed, 8 Feb 2017 12:29:42 -0800 Subject: [PATCH] (PDB-2640) Close gzip-wrapped body input stream on request completion In the previous commit, if gzip compression was applied to a request input stream, the stream was not automatically closed when the request was complete. The Apache HTTP async library, however, closes an input stream in cases where gzip compression is not being added. In this commit, the original request input stream is closed immediately after attempts to copy the data from it to the downstream gzipped input stream have been completed, for compatibility with the pre-existing Apache HTTP async library behavior. --- .../http/client/impl/JavaClient.java | 8 ++- .../http/client/gzip_request_test.clj | 64 ++++++++++++------- 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/java/com/puppetlabs/http/client/impl/JavaClient.java b/src/java/com/puppetlabs/http/client/impl/JavaClient.java index 1f8ff13..ebec76c 100644 --- a/src/java/com/puppetlabs/http/client/impl/JavaClient.java +++ b/src/java/com/puppetlabs/http/client/impl/JavaClient.java @@ -386,8 +386,12 @@ public class JavaClient { if (requestBody instanceof InputStream) { InputStream requestInputStream = (InputStream) requestBody; byte[] byteBuffer = new byte[GZIP_BUFFER_SIZE]; - IOUtils.copyLarge(requestInputStream, - gzipOutputStream, byteBuffer); + try { + IOUtils.copyLarge(requestInputStream, + gzipOutputStream, byteBuffer); + } finally { + requestInputStream.close(); + } } else { throwUnsupportedBodyException(requestBody); } diff --git a/test/puppetlabs/http/client/gzip_request_test.clj b/test/puppetlabs/http/client/gzip_request_test.clj index e18bf41..4b7e7e5 100644 --- a/test/puppetlabs/http/client/gzip_request_test.clj +++ b/test/puppetlabs/http/client/gzip_request_test.clj @@ -3,7 +3,7 @@ SimpleRequestOptions ResponseBodyType CompressType) - (java.io ByteArrayInputStream) + (java.io ByteArrayInputStream FilterInputStream) (java.net URI) (java.util.zip GZIPInputStream)) (:require [clojure.test :refer :all] @@ -31,10 +31,14 @@ (apply str (repeat 4000 "and�i�said�hey�yeah�yeah�whats�going�on"))) (defn string->byte-array-input-stream - [source] - (-> source - (.getBytes) - (ByteArrayInputStream.))) + [source is-closed-atom] + (let [bis (-> source + (.getBytes) + (ByteArrayInputStream.))] + (proxy [FilterInputStream] [bis] + (close [] + (reset! is-closed-atom true) + (proxy-super close))))) (defn post-gzip-clj-request [port body] @@ -71,17 +75,23 @@ (is (= "gzip" (:request-content-encoding response))) (is (= big-request-body (:request-body-decompressed response))))) (testing "short inputstream body is gzipped in request" - (let [response (post-gzip-clj-request + (let [is-closed (atom false) + response (post-gzip-clj-request port - (string->byte-array-input-stream short-request-body))] + (string->byte-array-input-stream short-request-body + is-closed))] (is (= "gzip" (:request-content-encoding response))) - (is (= short-request-body (:request-body-decompressed response))))) + (is (= short-request-body (:request-body-decompressed response))) + (is @is-closed "input stream was not closed after request"))) (testing "big inputstream body is gzipped in request" - (let [response (post-gzip-clj-request + (let [is-closed (atom false) + response (post-gzip-clj-request port - (string->byte-array-input-stream big-request-body))] + (string->byte-array-input-stream big-request-body + is-closed))] (is (= "gzip" (:request-content-encoding response))) - (is (= big-request-body (:request-body-decompressed response)))))))) + (is (= big-request-body (:request-body-decompressed response))) + (is @is-closed "input stream was not closed after request")))))) (deftest java-sync-client-gzip-requests (testing "for java sync client" @@ -97,22 +107,32 @@ (is (= "gzip" (:request-content-encoding response))) (is (= big-request-body (:request-body-decompressed response))))) (testing "short inputstream body is gzipped in request" - (let [response (post-gzip-java-request + (let [is-closed (atom false) + response (post-gzip-java-request port - (string->byte-array-input-stream short-request-body))] + (string->byte-array-input-stream short-request-body + is-closed))] (is (= "gzip" (:request-content-encoding response))) - (is (= short-request-body (:request-body-decompressed response))))) + (is (= short-request-body (:request-body-decompressed response))) + (is @is-closed "input stream was not closed after request"))) (testing "big inputstream body is gzipped in request" - (let [response (post-gzip-java-request + (let [is-closed (atom false) + response (post-gzip-java-request port - (string->byte-array-input-stream big-request-body))] + (string->byte-array-input-stream big-request-body + is-closed))] (is (= "gzip" (:request-content-encoding response))) - (is (= big-request-body (:request-body-decompressed response)))))))) + (is (= big-request-body (:request-body-decompressed response))) + (is @is-closed "input stream was not closed after request")))))) (deftest connect-exception-during-gzip-request-returns-failure (testing "connection exception during gzip request returns failure" - (is (connect-exception-thrown? - (http-client/post "http://localhost:65535" - {:body short-request-body - :compress-request-body :gzip - :as :text}))))) + (let [is-closed (atom false)] + (is (connect-exception-thrown? + (http-client/post "http://localhost:65535" + {:body (string->byte-array-input-stream + short-request-body + is-closed) + :compress-request-body :gzip + :as :text}))) + (is @is-closed "input stream was not closed after request"))))