On Wed, Mar 15, 2017 at 10:56:10AM +0100, Willy Tarreau wrote:
> > I did some digging and found that the ability to handle dynamic buffers was
> > added in 00f0084752eab236af80e61291d672e835790cff
> > <http://git.haproxy.org/?p=haproxy-1.5.git;a=commit;h=00f0084752eab236af80e61291d672e835790cff>
> > but it seems to have been removed in
> > d7bdcb874bcbd82737e51f080b9b0092863399f9
> > <http://git.haproxy.org/?p=haproxy-1.6.git;a=commit;h=d7bdcb874bcbd82737e51f080b9b0092863399f9>,
> > but I can't tell if the removal was done on purpose or if it was an
> > accident. Either way, d7bdcb874bcb was not backported to 1.5, making their
> > behaviour wildly different. Also, due to d7bdcb874bcb, 1.6 does not work as
> > stated in the docs
> > <https://cbonte.github.io/haproxy-dconv/1.6/configuration.html#7.3.5-req.payload>,
> > which say that "As a special case, if the <length> argument is zero, the
> > the whole buffer from <offset> to the end is extracted."
>
> That's a very accute observation.
>
> > Was that on purpose or is it an actual bug?
>
> Both. It was made on purpose to address a specific case without thinking about
> this one in my opinion. I think we should return "don't know yet" when the
> current buffer size is zero (not allocated yet) and use the buffer's size
> when the argument is zero. I'm going to have a look at it.
Looking closer, I was wrong, it's only a bug. In fact the same condition
was used to address both payload_lv() and payload() while only the latter
makes a special case of length zero. So I think this should be changed
like this :
- if (!buf_size || buf_size > global.tune.bufsize || buf_offset +
buf_size > global.tune.bufsize) {
+ if (buf_size > global.tune.bufsize || buf_offset + buf_size >
global.tune.bufsize) {
Looking at the rest of the function it should work. Would you care to test
and to send us this patch with a suitable commit message ? Please look at
CONTRIBUTING to get all the info and/or just "git log src/payload.c".
Thanks,
Willy
