From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 2 Jul 2019 13:48:03 +0000 (+0200)
Subject: BUG/MEDIUM: channel/htx: Use the total HTX size in channel_htx_recv_limit()
X-Git-Tag: v2.1-dev1~40
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=621da6bafaffc4a0182fb46cfa4b0153c83469e7;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: channel/htx: Use the total HTX size in channel_htx_recv_limit()

The receive limit of an HTX channel must be calculated against the total size of
the HTX message. Otherwise, the buffer may never be seen as full whereas the
receive limit is 0. Indeed, the function channel_htx_full() already takes care
to add a block size to the buffer's reserve (8 bytes). So if the function
channel_htx_recv_limit() also keep a block size free in addition to the buffer's
reserve, it means that at least 2 block size will be kept free but only one will
be taken into account, freezing the stream if the option http-buffer-request is
enabled.

This patch fixes the Github issue #136. It should be backported to 2.0 and
1.9. Thanks jaroslawr (Jarosław Rzeszótko) for his help.
---

diff --git a/include/common/htx.h b/include/common/htx.h
index 7d15365a37..ca2309b00a 100644
--- a/include/common/htx.h
+++ b/include/common/htx.h
@@ -616,15 +616,6 @@ static inline uint32_t htx_free_space(const struct htx *htx)
 	return (htx->size - htx_used_space(htx));
 }
 
-/* Returns the maximum space usable for data in <htx>. This is in fact the
- * maximum sice for a uniq block to fill the HTX message. */
-static inline uint32_t htx_max_data_space(const struct htx *htx)
-{
-	if (!htx->size)
-		return 0;
-	return (htx->size - sizeof(htx->blocks[0]));
-}
-
 /* Returns the maximum size available to store some data in <htx> if a new block
  * is reserved.
  */
diff --git a/include/proto/channel.h b/include/proto/channel.h
index 9dc2b991cc..5e5d8228c2 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -736,17 +736,17 @@ static inline int channel_htx_recv_limit(const struct channel *chn, const struct
 	 * cause an integer underflow in the comparison since both are unsigned
 	 * while maxrewrite is signed.
 	 * The code below has been verified for being a valid check for this :
-	 *   - if (o + to_forward) overflow => return max_data_space  [ large enough ]
-	 *   - if o + to_forward >= maxrw   => return max_data_space  [ large enough ]
-	 *   - otherwise return max_data_space - (maxrw - (o + to_forward))
+	 *   - if (o + to_forward) overflow => return htx->size  [ large enough ]
+	 *   - if o + to_forward >= maxrw   => return htx->size  [ large enough ]
+	 *   - otherwise return htx->size - (maxrw - (o + to_forward))
 	 */
 	transit = co_data(chn) + chn->to_forward;
 	reserve -= transit;
 	if (transit < chn->to_forward ||                 // addition overflow
 	    transit >= (unsigned)global.tune.maxrewrite) // enough transit data
-		return htx_max_data_space(htx);
+		return htx->size;
  end:
-	return (htx_max_data_space(htx) - reserve);
+	return (htx->size - reserve);
 }
 
 /* HTX version of channel_full(). Instead of checking if INPUT data exceeds