From 5eb3ae242289b074771deef2bd574636959e4d56 Mon Sep 17 00:00:00 2001
From: Ryan O'Neill <ryano@twitter.com>
Date: Wed, 15 May 2019 18:38:50 +0000
Subject: [PATCH] finagle-http: Remove Misleading MaxRequest/MaxResponse Size
 Methods

Problem

The `MaxRequestSize` param within Finagle HTTP is applicable to Finagle
HTTP servers only. The `MaxResponseSize` param is applicable to Finagle
HTTP clients only. It is incorrect and misleading to have a
`client.withMaxRequestSize(size)` method and a
`server.withMaxResponseSize(size)` method, since while they change the
underlying `Stack` params for that client or server, changing that `Stack`
param for that particular side has no effect.

Solution

Remove the `client.withMaxRequestSize(size)` and
`server.withMaxResponseSize(size)` methods, and more clearly document the
underlying params to indicate that `MaxRequestSize` is for HTTP servers only,
and `MaxResponseSize` is for HTTP clients only.

JIRA Issues: CSL-8121

Differential Revision: https://phabricator.twitter.biz/D314019
---
 CHANGELOG.rst                                      |  7 +++++++
 .../com/twitter/finagle/http/param/params.scala    | 14 ++++++++++++++
 .../src/main/scala/com/twitter/finagle/Http.scala  | 12 ------------
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 45149ade2c..f3fccdb340 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -28,6 +28,13 @@ Runtime Behavior Changes
 Breaking API Changes
 ~~~~~~~~~~~~~~~~~~~~
 
+* finagle-http: For Finagle HTTP clients, the `withMaxRequestSize(size)` API
+  method has been removed. For Finagle HTTP servers, the
+  `withMaxResponseSize(size)` method has been removed. The underlying `Stack`
+  params which are set by these methods are respectively HTTP server and HTTP
+  client side params only. Using these removed methods had no effect on the
+  setup of Finagle HTTP clients and servers. ``PHAB_ID=D314019``
+
 * finagle-mysql: HandshakeResponse has been removed from finagle-mysql's public
   API. It is expected that users of the library are relying entirely on
   finagle-mysql for handshaking. ``PHAB_ID=D304512``
diff --git a/finagle-base-http/src/main/scala/com/twitter/finagle/http/param/params.scala b/finagle-base-http/src/main/scala/com/twitter/finagle/http/param/params.scala
index 1faff02936..cce802a456 100644
--- a/finagle-base-http/src/main/scala/com/twitter/finagle/http/param/params.scala
+++ b/finagle-base-http/src/main/scala/com/twitter/finagle/http/param/params.scala
@@ -33,6 +33,13 @@ object MaxInitialLineSize {
     Stack.Param(MaxInitialLineSize(4.kilobytes))
 }
 
+/**
+ * The maximum size of an inbound HTTP request that this
+ * Finagle server can receive from a client.
+ *
+ * @note This param only applies to Finagle HTTP servers,
+ * and not to Finagle HTTP clients.
+ */
 case class MaxRequestSize(size: StorageUnit) {
   require(size < 2.gigabytes, s"MaxRequestSize should be less than 2 Gb, but was $size")
 }
@@ -41,6 +48,13 @@ object MaxRequestSize {
     Stack.Param(MaxRequestSize(5.megabytes))
 }
 
+/**
+ * The maximum size of an inbound HTTP response that this
+ * Finagle client can receive from a server.
+ *
+ * @note This param only applies to Finagle HTTP clients,
+ * and not to Finagle HTTP servers.
+ */
 case class MaxResponseSize(size: StorageUnit) {
   require(size < 2.gigabytes, s"MaxResponseSize should be less than 2 Gb, but was $size")
 }
diff --git a/finagle-http/src/main/scala/com/twitter/finagle/Http.scala b/finagle-http/src/main/scala/com/twitter/finagle/Http.scala
index b74011a5eb..2cfff0db39 100644
--- a/finagle-http/src/main/scala/com/twitter/finagle/Http.scala
+++ b/finagle-http/src/main/scala/com/twitter/finagle/Http.scala
@@ -293,12 +293,6 @@ object Http extends Client[Request, Response] with HttpRichClient with Server[Re
     def withMaxInitialLineSize(size: StorageUnit): Client =
       configured(http.param.MaxInitialLineSize(size))
 
-    /**
-     * Configures the maximum request size that the client can send.
-     */
-    def withMaxRequestSize(size: StorageUnit): Client =
-      configured(http.param.MaxRequestSize(size))
-
     /**
      * Configures the maximum response size that client can receive.
      */
@@ -528,12 +522,6 @@ object Http extends Client[Request, Response] with HttpRichClient with Server[Re
     def withMaxRequestSize(size: StorageUnit): Server =
       configured(http.param.MaxRequestSize(size))
 
-    /**
-     * Configures the maximum response size this server can send.
-     */
-    def withMaxResponseSize(size: StorageUnit): Server =
-      configured(http.param.MaxResponseSize(size))
-
     /**
      * Streaming allows applications to work with HTTP messages that have large
      * (or infinite) content bodies.