[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [VOTE] Merging 1.9-cache-improvements to /trunk

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Fri, 15 May 2015 14:09:21 +0100

Philip Martin <philip.martin_at_wandisco.com> writes:

> Prompted by the warnings I think there are some issues to fix. For
> APR_HASH_KEY_STRING keys there is no protection against abnormally long
> keys. combined_long_key() will allocate strlen() memory even if it is
> many GB. The item will not get cached if key+data length is more than
> 4GB but the memory for the key, which could be more than 4GB, will be
> permanently allocated in the cache. There is also a problem with
> overflow in membuffer_cache_set_internal() when calculating key+data
> length, although in practice a key large enough to trigger this will
> probably fail memory allocation first.

Another issue:

static svn_boolean_t
entry_keys_match(const entry_key_t *lhs,
                 const entry_key_t *rhs)
{
  return (lhs->fingerprint[0] == rhs->fingerprint[0])
      && (lhs->fingerprint[1] == rhs->fingerprint[1])
      && (lhs->key_len == rhs->key_len);
}

I think the key_len comparison is wrong and should be removed. If two
keys have fingerprints that collide it does not mean the key lengths are
the same. When attempting to use two keys with colliding fingerprints
the behaviour when the key lengths vary should be same as when the key
lengths are the same and the key data varies.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Received on 2015-05-15 15:10:18 CEST

This is an archived mail posted to the Subversion Dev mailing list.