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) ensure unbuffered-stream correctly handles 204 responses #105

Merged
merged 2 commits into from
Mar 11, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## Unreleased

# 2.1.3
* ensure unbuffered-stream correctly handles 204 responses with no body in the response.

# 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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.puppetlabs.http.client.impl;

import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.entity.BasicHttpEntity;
import org.apache.http.nio.IOControl;
import org.apache.http.nio.client.methods.AsyncByteConsumer;
import org.apache.http.protocol.HttpContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.PipedInputStream;
Expand All @@ -18,6 +21,8 @@ public class StreamingAsyncResponseConsumer extends AsyncByteConsumer<HttpRespon
private volatile Deliverable<HttpResponse> promise;
private volatile Promise<IOException> ioExceptionPromise = new Promise<>();

private static final Logger LOGGER = LoggerFactory.getLogger(StreamingAsyncResponseConsumer.class);

public void setFinalResult(IOException ioException) {
ioExceptionPromise.deliver(ioException);
}
Expand All @@ -28,10 +33,16 @@ public StreamingAsyncResponseConsumer(Deliverable<HttpResponse> promise) {

@Override
protected void onResponseReceived(final HttpResponse response) throws IOException {
PipedInputStream pis = new ExceptionInsertingPipedInputStream(ioExceptionPromise);
pos = new PipedOutputStream();
pos.connect(pis);
((BasicHttpEntity) response.getEntity()).setContent(pis);
HttpEntity entity = response.getEntity();
if (entity != null) {
PipedInputStream pis = new ExceptionInsertingPipedInputStream(ioExceptionPromise);
pos = new PipedOutputStream();
pos.connect(pis);
((BasicHttpEntity) entity).setContent(pis);
} else {
// this can happen if the server sends no response, like with a 204.
LOGGER.debug("Null entity when processing response");
}
this.response = response;
promise.deliver(response);
}
Expand Down
10 changes: 10 additions & 0 deletions test/puppetlabs/http/client/async_unbuffered_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,13 @@
(deftest java-existing-streaming-with-large-payload-with-decompression
(testing "java :stream with 1M payload and decompression"
(java-blocking-streaming (generate-data (* 1024 1024)) ResponseBodyType/STREAM true)))

(deftest java-204-streaming
(testing "client handles a webserver that returns a 204 response and no body correctly"
(testwebserver/with-test-webserver-and-config
(constantly {:status 204}) port {:shutdown-timeout-seconds 1}
(with-open [client (async/create-client {:connect-timeout-milliseconds 100})]
(let [response @(common/post client (str "http://localhost:" port "/something") {:method :post
:as :unbuffered-stream})]
(is (= 204 (:status response))))))))

Loading