[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [Patch] fix bug #39175 Header value length limited with 2
From: |
Miquel Llobet |
Subject: |
Re: [Bug-wget] [Patch] fix bug #39175 Header value length limited with 256 |
Date: |
Sun, 15 Mar 2015 00:09:23 +0100 |
>
> Could this be dinamically allocated?
Indeed it can! Thanks Giuseppe and Darshit for pointing out. Ideally we
should change all the resp_header_copy calls to resp_header_strdup (dynamic
resp header copy). There is a bit to code to change, as the function
returns are different etc. expect a big patch.
I will also make a test for long header sizes as Tim suggested.
Cheers,
Miquel Llobet
On Sat, Mar 14, 2015 at 8:34 PM, Darshit Shah <address@hidden> wrote:
> Hi Miquel,
>
> Thanks a lot for making this contribution.
>
> I not a supporter of making such large (8K is gigantic!) memory
> assignments on the stack. Wget runs not only on the latest x86_64
> processors, but often also on ancient and obscure architectures. And
> assignment of a variable using 8K of memory on the stack may not be
> possible on some architectures.
> Instead, I think this should be done via dynamic memory allocation.
>
>
> On 03/12, Miquel Llobet wrote:
>
>> Increased the header buffer to 8Kb, as there are no limits to the size of
>> field name, values or headers themselves. While the current value is big
>> enough, other projects such as Apache [1] or nginx have limits of 4-8Kb.
>>
>> If we want to allow for arbitrary size headers we should use
>> resp_header_strdup instead of resp_header_copy, but this new value should
>> be enough.
>>
>> --- src/http.c.orig 2015-03-12 21:50:03.000000000 +0100
>> +++ src/http.c 2015-03-12 21:04:08.000000000 +0100
>> @@ -1695,7 +1695,7 @@
>>
>> char *head;
>> struct response *resp;
>> - char hdrval[512];
>> + char hdrval[8190];
>> char *message;
>>
>> /* Declare WARC variables. */
>>
>> [1]: https://httpd.apache.org/docs/2.2/mod/core.html#
>> limitrequestfieldsize
>>
>> Miquel Llobet
>>
> --- end quoted text ---
>
> --
> Thanking You,
> Darshit Shah
>