The JIRA project is at https://issues.apache.org/jira/browse/PROTON

Details of JIRA projects etc for each component are listed on the
website, e.g. http://qpid.apache.org/proton/ in this case.

Robbie

On 28 May 2015 at 20:46, Michael Ivanov <iv...@isle.spb.ru> wrote:
> Sorry, how do I create a JIRA issue?
>
> 27.05.2015 16:48, Alan Conway пишет:
>> On Tue, 2015-05-26 at 23:25 +0300, Michael Ivanov wrote:
>>> Greetings,
>>>
>>> I have observed that pn_data_grow() function looses half of the available 
>>> data capacity.
>>> The following happens: when data overflows, pn_data_grow is invoked. It 
>>> increases
>>> data capacity 2 times and reallocates nodes array. Data capacity is 
>>> represented as
>>> uint16_t type and so when capacity reaches 32768 items, the result of 
>>> multiplication by 2
>>> becomes 0. This makes realloc return null and crashes the program.
>>>
>>> To alleviate the problem with large messages I changed the function as 
>>> follows:
>>>
>>> --- qpid-proton-0.9/proton-c/src/codec/codec.c       2015-03-31 
>>> 12:07:22.000000000 +0300
>>> +++ qpid-proton-0.9.fix/proton-c/src/codec/codec.c   2015-05-26 
>>> 21:18:55.801632941 +0300
>>> @@ -417,8 +417,21 @@ void pn_data_clear(pn_data_t *data)
>>>
>>>  int pn_data_grow(pn_data_t *data)
>>>  {
>>> -  data->capacity = 2*(data->capacity ? data->capacity : 2);
>>> -  data->nodes = (pni_node_t *) realloc(data->nodes, data->capacity * 
>>> sizeof(pni_node_t));
>>> +  size_t    s = data->capacity;
>>> +
>>> +  if (s < 0x7fff)
>>> +     s = 2 * (s? s : 2);
>>> +  else if (s < 0xffff - 1024)
>>> +     s += 1024;
>>> +  else if (s != 0xffff)
>>> +     s = 0xffff;
>>> +  else {
>>> +     pn_logf("Data node %p overflow", data);
>>> +     abort();
>>> +  }
>>> +
>>> +  data->nodes = (pni_node_t *) realloc(data->nodes, s * 
>>> sizeof(pni_node_t));
>>> +  data->capacity = s;
>>>    return 0;
>>>
>>> This allows to use capacities in 0x8000 ... 0xffff range and is supposed to 
>>> report
>>> data overflow.
>>
>>
>> Good catch, can you please create a JIRA with the patch attached so you
>> can get proper credit for it (for copyright reasons I can't take the
>> patch direct from email.) You can assign it to me, I'll apply it right
>> away.
>>
>> I would suggest a couple of changes:
>>
>> 1. Define an explicit PNI_NID_MAX in data.h right beside typedef
>> pni_nid_t rather than using literal 0xffff etc. in case someone changes
>> the definition of pni_nid_t at some point.
>>
>> 2. Use a simple double or max rather than adding 1k after 0x7fff. We
>> need to fix the algorithm but I don't think we need to add new
>> complexity unless there's a separate justification for that.
>>
>> 3. If we fail to increase (i.e. we're already at PNI_NID_MAX) return
>> PN_OVERFLOW rather than aborting. There is only one function
>> (pn_data_new) that calls pn_data_grow, make it check the return value
>> and return NULL on error. That won't make exiting code worse (it will
>> still crash on overrun) but will give well written code that checks
>> error values the choice.
>>
>> Thanks for this!
>> Alan.
>>
>
>
> --
>  \   / |                                   |
>  (OvO) |  Михаил Иванов                    |
>  (^^^) |      Тел.:    +7(911) 223-1300    |
>   \^/  |      E-mail:  iv...@isle.spb.ru   |
>   ^ ^  |                                   |

Reply via email to