From 9efcba87302dc76fe31b33d685bca1fea34cf7f2 Mon Sep 17 00:00:00 2001
From: Tomas Celaya <tjcelaya@joyent.com>
Date: Mon, 31 Jul 2017 18:10:03 -0700
Subject: [PATCH] [HTTPCLIENT-1865] DefaultServiceUnavailableRetryStrategy does
 not respect HttpEntity#isRepeatable.

---
 .../sync/ServiceUnavailableRetryExec.java     |  5 +++
 .../sync/TestServiceUnavailableRetryExec.java | 32 +++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/sync/ServiceUnavailableRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/sync/ServiceUnavailableRetryExec.java
index 42fcff345..721f98db1 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/sync/ServiceUnavailableRetryExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/sync/ServiceUnavailableRetryExec.java
@@ -39,6 +39,7 @@ import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.util.Args;
 import org.apache.logging.log4j.LogManager;
@@ -82,6 +83,10 @@ final class ServiceUnavailableRetryExec implements ExecChainHandler {
         for (int c = 1;; c++) {
             final ClassicHttpResponse response = chain.proceed(currentRequest, scope);
             try {
+                final HttpEntity entity = request.getEntity();
+                if (entity != null && !entity.isRepeatable()) {
+                    return response;
+                }
                 if (this.retryStrategy.retryRequest(response, c, context)) {
                     response.close();
                     final long nextInterval = this.retryStrategy.getRetryInterval(response, context);
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/sync/TestServiceUnavailableRetryExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/sync/TestServiceUnavailableRetryExec.java
index 60b8fd3d3..1ce49208a 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/sync/TestServiceUnavailableRetryExec.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/sync/TestServiceUnavailableRetryExec.java
@@ -26,17 +26,22 @@
  */
 package org.apache.hc.client5.http.impl.sync;
 
+import java.io.ByteArrayInputStream;
+
 import org.apache.hc.client5.http.HttpRoute;
+import org.apache.hc.client5.http.entity.EntityBuilder;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.client5.http.sync.ExecChain;
 import org.apache.hc.client5.http.sync.ExecRuntime;
 import org.apache.hc.client5.http.sync.ServiceUnavailableRetryStrategy;
 import org.apache.hc.client5.http.sync.methods.HttpGet;
+import org.apache.hc.client5.http.sync.methods.HttpPost;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.protocol.HttpContext;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
@@ -113,4 +118,31 @@ public class TestServiceUnavailableRetryExec {
             throw ex;
         }
     }
+
+    @Test
+    public void testNonRepeatableEntityResponseReturnedImmediately() throws Exception {
+        final HttpRoute route = new HttpRoute(target);
+
+        final HttpPost request = new HttpPost("/test");
+        request.setEntity(EntityBuilder.create()
+                .setStream(new ByteArrayInputStream(new byte[]{}))
+                .build());
+        final HttpClientContext context = HttpClientContext.create();
+
+        final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
+        Mockito.when(chain.proceed(
+                Mockito.<ClassicHttpRequest>any(),
+                Mockito.<ExecChain.Scope>any())).thenReturn(response);
+        Mockito.when(retryStrategy.retryRequest(
+                Mockito.<HttpResponse>any(),
+                Mockito.anyInt(),
+                Mockito.<HttpContext>any())).thenReturn(Boolean.TRUE, Boolean.FALSE);
+
+        final ExecChain.Scope scope = new ExecChain.Scope(route, request, endpoint, context);
+        final ClassicHttpResponse finalResponse = retryExec.execute(request, scope, chain);
+
+        Assert.assertSame(response, finalResponse);
+        Mockito.verify(response, Mockito.times(0)).close();
+    }
+
 }