BUG/MEDIUM: chunk: do not rely on small trash by default for expressions

There's a corner case with get_trash_chunk_sz() combined with the use
of small bufs: if some incoming data is going to be inflated by a
converter in a non-predictable way (say url_enc etc) then there are
two possibilities:
  - either we try to allocate a size that corresponds to the data, but
    we risk to allocate a small buf to convert a 900B chunk, that will
    now fail if it contains too many non-printable chars;
  - or we try to allocate 3x the size to be conservative, but without
    large bufs we'd fail to transcode any chunk larger than 5.3kB, even
    if it contains only printable chars.

The approach should definitely be refined and it is not 100% reliable
for now. Better temporarily ignore the small buffers for these particular
cases where the savings are not relevant, and see how to pass the knowledge
of the expected size ranges deeper down the API in 3.5. We may possibly rely
on the current trash size (instead of contents) or other mechanisms that
are yet to be specified. alloc_small_trash_chunk() gets the same change
BTW for the same reasons.

The comment for get_trash_chunk_sz() was updated to restate the importance
of being conservative when requesting a size.

No backport is needed.

diff --git a/include/haproxy/chunk.h b/include/haproxy/chunk.h
index 341b33940685ab93b319ff145e88a0058e2b5f57..7c9350f7b58048d224e91d28188a59c054c3df5f 100644 (file)
--- a/include/haproxy/chunk.h
+++ b/include/haproxy/chunk.h
@@ -165,9 +165,7 @@ static forceinline struct buffer *alloc_small_trash_chunk(void)
  */
 static forceinline struct buffer *alloc_trash_chunk_sz(size_t size)
 {
-       if (pool_head_small_trash && size <= pool_head_small_trash->size)
-               return alloc_small_trash_chunk();
-       else if (size <= pool_head_trash->size)
+       if (size <= pool_head_trash->size)
                return alloc_trash_chunk();
        else if (pool_head_large_trash && size <= pool_head_large_trash->size)
                return alloc_large_trash_chunk();

diff --git a/src/chunk.c b/src/chunk.c
index fd95b9687f19fc9986bf7351895fd54f47482530..ab4f002b1b98adfcbe28e09b44d96c0b50690557 100644 (file)
--- a/src/chunk.c
+++ b/src/chunk.c
@@ -145,14 +145,15 @@ struct buffer *get_small_trash_chunk(void)
 
 /* Returns a trash chunk accordingly to the requested size. This function may
  * fail if the requested size is too big or if the large chunks are not
- * configured.
+ * configured. Note that requesting a size larger than the largest available
+ * buffer will result in NULL being returned, so better be conservative when
+ * requesting the size and plan to use get_larger_trash_chunk() later if not
+ * sufficient.
  */
 struct buffer *get_trash_chunk_sz(size_t size)
 {
-       if (likely(size > small_trash_size && size <= trash_size))
+       if (likely(size <= trash_size))
                return get_trash_chunk();
-       else if (small_trash_size && size <= small_trash_size)
-               return get_small_trash_chunk();
        else if (large_trash_size && size <= large_trash_size)
                return get_large_trash_chunk();
        else