ok2c commented on code in PR #423:
URL:
https://github.com/apache/httpcomponents-client/pull/423#discussion_r1133252971
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -44,11 +46,63 @@
*/
public class ContentResponseHandler extends
AbstractHttpClientResponseHandler<Content> {
+ /**
+ * Handles the response entity and transforms it into a {@link Content}
object. If the response entity is null or
+ * has zero length, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param entity the response entity.
+ * @return a {@link Content} object that encapsulates the response body,
or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws IOException if an I/O error occurs.
+ */
@Override
public Content handleEntity(final HttpEntity entity) throws IOException {
- return entity != null ?
- new Content(EntityUtils.toByteArray(entity),
ContentType.parse(entity.getContentType())) :
- Content.NO_CONTENT;
+ if (entity != null && entity.getContentLength() > 0) {
+ return new Content(EntityUtils.toByteArray(entity),
ContentType.parse(entity.getContentType()));
+ } else {
+ return Content.NO_CONTENT;
+ }
+ }
+
+ /**
+ * Handles a successful response (2xx status code) and returns the
response entity as a {@link Content} object.
+ * If no response entity exists, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param response the HTTP response.
+ * @return a {@link Content} object that encapsulates the response body,
or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws HttpResponseException if the response was unsuccessful (a >=
300 status code).
+ * @throws IOException if an I/O error occurs.
+ */
+ @Override
+ public Content handleResponse(final ClassicHttpResponse response) throws
IOException {
+ final int statusCode = response.getCode();
+ final HttpEntity entity = response.getEntity();
+ final Content content = handleEntity(entity);
+ if (statusCode >= 300) {
+ final StringBuilder messageBuilder = new
StringBuilder(response.getReasonPhrase());
Review Comment:
@arturobernalg I would not abuse the reason phase and would add an extra new
attribute to the `HttpResponseException` class.
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -44,11 +46,63 @@
*/
public class ContentResponseHandler extends
AbstractHttpClientResponseHandler<Content> {
+ /**
+ * Handles the response entity and transforms it into a {@link Content}
object. If the response entity is null or
+ * has zero length, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param entity the response entity.
+ * @return a {@link Content} object that encapsulates the response body,
or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws IOException if an I/O error occurs.
+ */
@Override
public Content handleEntity(final HttpEntity entity) throws IOException {
- return entity != null ?
- new Content(EntityUtils.toByteArray(entity),
ContentType.parse(entity.getContentType())) :
- Content.NO_CONTENT;
+ if (entity != null && entity.getContentLength() > 0) {
+ return new Content(EntityUtils.toByteArray(entity),
ContentType.parse(entity.getContentType()));
+ } else {
+ return Content.NO_CONTENT;
+ }
+ }
+
+ /**
+ * Handles a successful response (2xx status code) and returns the
response entity as a {@link Content} object.
+ * If no response entity exists, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param response the HTTP response.
+ * @return a {@link Content} object that encapsulates the response body,
or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws HttpResponseException if the response was unsuccessful (a >=
300 status code).
+ * @throws IOException if an I/O error occurs.
+ */
+ @Override
+ public Content handleResponse(final ClassicHttpResponse response) throws
IOException {
+ final int statusCode = response.getCode();
+ final HttpEntity entity = response.getEntity();
+ final Content content = handleEntity(entity);
+ if (statusCode >= 300) {
+ final StringBuilder messageBuilder = new
StringBuilder(response.getReasonPhrase());
+ if (content != null && content.getType() != null) {
+ messageBuilder.append(", body was ");
+ truncateString(content.asString(), messageBuilder);
Review Comment:
@arturobernalg The truncation logic looks flawed. What is the point of
truncating the message if it has already been fully loaded in memory? Use
`EntityUtils#toByteArray` with the content max limit instead.
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -44,11 +46,63 @@
*/
public class ContentResponseHandler extends
AbstractHttpClientResponseHandler<Content> {
+ /**
+ * Handles the response entity and transforms it into a {@link Content}
object. If the response entity is null or
+ * has zero length, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param entity the response entity.
+ * @return a {@link Content} object that encapsulates the response body,
or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws IOException if an I/O error occurs.
+ */
@Override
public Content handleEntity(final HttpEntity entity) throws IOException {
- return entity != null ?
- new Content(EntityUtils.toByteArray(entity),
ContentType.parse(entity.getContentType())) :
- Content.NO_CONTENT;
+ if (entity != null && entity.getContentLength() > 0) {
Review Comment:
@arturobernalg Is this reasonable? It is legal for an entity to have its
content length as -1 if it is chunk coded.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
