zuul-core: always cache full bodies, regardless of SSL

In the scope of Retrying POST bodies in response to 503's Zuul keeps a copy of the body to send again in certain circumstances.   Normally, if the body has not been sent to the origin, and Zuul has not sent anything to the client, Zuul will compromise the HTTP spec and consider POSTs retryable.   (Too many things depend on this wrong behavior; it would risky to change.)  In the current code, Netty's SSL handler will mutate the body when sending it, so the ProxyEndpoint will keep a pristine copy of the body assuming the full body is present.  In the case SSL was not requested, Zuul depends on Netty not clearing the buffers as it sends, and thus doesn't keep the copy around.

Internally at Netflix, we want to automatically upgrade to SSL if possible, but that is only after the ProxyEndpoint code has decided to cache or not.   An Origin may not explicit ask for SSL, but eventually use it, so it is too late to set the NIWS `IsSecure` bit and cache the body.   (Requesting SSL is per Origin, using SSL is per Server).

To ensure that 503's are retryable, and that we don't depend on Netty's HTTP subtleties, this change unconditionally caches the body, and pessimistically assumes that the body could be mutated.  This allows for auto SSL upgrades later down the call chain as well.

Sorry, no tests.   This code definitely warrants a test, but it is ridiculously difficult to add one.   I'm taking out a loan on our tech debt, which I hope to one day pay back.

zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java

@@ -145,11 +145,11 @@ public class ProxyEndpoint extends SyncZuulFilterAdapter<HttpRequestMessage, Htt
     protected RequestAttempt currentRequestAttempt;
     protected List<RequestStat> requestStats = new ArrayList<>();
     protected RequestStat currentRequestStat;
-    private final byte[] sslRetryBodyCache;
+    private final byte[] retryBodyCache;
 
     public static final Set<String> IDEMPOTENT_HTTP_METHODS = Sets.newHashSet("GET", "HEAD", "OPTIONS");
     private static final DynamicIntegerSetProperty RETRIABLE_STATUSES_FOR_IDEMPOTENT_METHODS = new DynamicIntegerSetProperty("zuul.retry.allowed.statuses.idempotent", "500");
-    private static final DynamicBooleanProperty ENABLE_CACHING_SSL_BODIES = new DynamicBooleanProperty("zuul.cache.ssl.bodies", true);
+    private static final DynamicBooleanProperty ENABLE_CACHING_BODIES = new DynamicBooleanProperty("zuul.cache.bodies", true);
 
     private static final CachedDynamicIntProperty MAX_OUTBOUND_READ_TIMEOUT = new CachedDynamicIntProperty("zuul.origin.readtimeout.max", 90 * 1000);
 
@@ -159,7 +159,7 @@ public class ProxyEndpoint extends SyncZuulFilterAdapter<HttpRequestMessage, Htt
     private static final Logger LOG = LoggerFactory.getLogger(ProxyEndpoint.class);
     private static final Counter NO_RETRY_INCOMPLETE_BODY = SpectatorUtils.newCounter("zuul.no.retry","incomplete_body");
     private static final Counter NO_RETRY_RESP_STARTED = SpectatorUtils.newCounter("zuul.no.retry","resp_started");
-    private final Counter populatedSslRetryBody;
+    private final Counter populatedRetryBody;
 
 
     public ProxyEndpoint(final HttpRequestMessage inMesg, final ChannelHandlerContext ctx,
@@ -180,9 +180,9 @@ public ProxyEndpoint(final HttpRequestMessage inMesg, final ChannelHandlerContex
         chosenServer = new AtomicReference<>();
         chosenHostAddr = new AtomicReference<>();
 
-        this.sslRetryBodyCache = preCacheBodyForRetryingSslRequests();
-        this.populatedSslRetryBody = SpectatorUtils.newCounter(
-                "zuul.populated.ssl.retry.body", origin == null ? "null" : origin.getName().getTarget());
+        this.retryBodyCache = preCacheBodyForRetryingRequests();
+        this.populatedRetryBody = SpectatorUtils.newCounter(
+                "zuul.populated.retry.body", origin == null ? "null" : origin.getName().getTarget());
 
         this.methodBinding = methodBinding;
         this.requestAttemptFactory = requestAttemptFactory;
@@ -565,22 +565,24 @@ private void onOriginConnectFailed(Throwable cause) {
         }
     }
 
-    private byte[] preCacheBodyForRetryingSslRequests() {
+    private byte[] preCacheBodyForRetryingRequests() {
         // Netty SSL handler clears body ByteBufs, so we need to cache the body if we want to retry POSTs
-        if (ENABLE_CACHING_SSL_BODIES.get() && origin != null &&
-                // only cache requests if already buffered
-                origin.getClientConfig().get(IClientConfigKey.Keys.IsSecure, false) && zuulRequest.hasCompleteBody()) {
+        // Followup: We expect most origin connections to be secure, so it's okay to unconditionally cache here.
+        // Additionally, it's risky to assume the plaintext handlers won't clear the body (they do), so just pay the
+        // cost caching regardless.
+        if (ENABLE_CACHING_BODIES.get() && origin != null && zuulRequest.hasCompleteBody()) {
+            // only cache requests if already buffered
             return zuulRequest.getBody();
         }
         return null;
     }
 
     private void repopulateRetryBody() {
-        // if SSL origin request body is cached and has been cleared by Netty SslHandler, set it from cache
         // note: it's not null but is empty because the content chunks exist but the actual readable bytes are 0
-        if (sslRetryBodyCache != null && attemptNum > 1 && zuulRequest.getBody() != null && zuulRequest.getBody().length == 0) {
-            zuulRequest.setBody(sslRetryBodyCache);
-            populatedSslRetryBody.increment();
+        if (retryBodyCache != null && attemptNum > 1
+                && zuulRequest.getBodyLength() == 0 && zuulRequest.getBody() != null) {
+            zuulRequest.setBody(retryBodyCache);
+            populatedRetryBody.increment();
         }
     }