Skip to content

Commit

Permalink
finagle-http: Remove Misleading MaxRequest/MaxResponse Size Methods
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ryanoneill authored and jenkins committed May 15, 2019
1 parent d48c02f commit 5eb3ae2
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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``
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand Down
12 changes: 0 additions & 12 deletions finagle-http/src/main/scala/com/twitter/finagle/Http.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 5eb3ae2

Please sign in to comment.