On 09.02.2012 19:08, Philip Martin wrote:
> Peter Samuelson<peter_at_p12n.org> writes:
>
>> [Philip Martin]
>>> --- ../src/subversion/libsvn_subr/svn_base64.c (revision 1242045)
>>> +++ ../src/subversion/libsvn_subr/svn_base64.c (working copy)
>>> @@ -410,7 +410,7 @@
>>> /* Resize the stringbuf to make room for the (approximate) size of
>>> output, to avoid repeated resizes later.
>>> The optimizations in decode_line rely on no resizes being necessary! */
>>> - svn_stringbuf_ensure(str, str->len + (len / 4) * 3 + 3);
>>> + svn_stringbuf_ensure(str, str->len + (len / 4) * 3 + 4);
Thanks for fixing that!
>>>
>>> while ( !*done&& p< end )
>>> {
>>>
>>> but I don't really understand the comment. There doesn't seem to be
>>> anything "approximate" about this calculation?
>> It is approximate because the encoded length is always a multiple of 4,
>> even if the input length was not a multiple of 3. (This is why so many
>> base64 strings end in = or ==; those are padding to a 4-byte output
>> group.) The decoder doesn't yet know if the final group of 4 bytes
>> will decode to 1, 2 or 3 bytes of plaintext.
> I believe approximate is still wrong. It's supposed to be an accurate
> calculation of the maximum output length. If some approximate value is
> used and the output is longer than that then the code will crash.
>
Sorry about my imprecise commentary in the code.
It should have read "close upper boundary" instead
of "approximate".
As already discussed on IRC, the real cause here
is that _ensure() should follow the first invariant listed
in svn_string.h by allocating *additional* space for
the terminating 0. In particular svn_stringbuf_ensure()
is inconsistent with svn_stringbuf_create_ensure()
in this respect.
Fixing that is somewhat hard as all callers need to
be inspected for potential side-effects. _ensure2()
would probably be the better solution.
-- Stefan^2.
Received on 2012-02-12 13:57:35 CET