Skip to content

Fixes an incorrect libmemcached version check for OPT_PREFIX_KEY #6

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

jhansche
Copy link

The PREFIX_KEY functionality had a comment explaining that a bug
had to be worked around, in all version prior to 0.50. However,
that assumption was invalid, as the memcached_set_prefix_key()
behavior was only rewritten in version 0.49. Since the error was
fixed in libmemcached-0.50, that means it only exists in one version:
0.49.

I tested this fix against every release between 0.44 - 0.50, and
confirmed that the bug only has to be worked around in exactly
version 0.49.

The simple test:

[terminalA] $ nc -l localhost 5555

[terminalB] $ php -a
php > $mc = new Memcached();
php > $mc->addServer('localhost', 5555, 1);
php > $mc->setOption(Memcached::OPT_PREFIX_KEY, 'foo');
php > $mc->set('bar', 1, 15);

I saw "foo0bar" coming through in version 0.44 - 0.48. The problem
was fixed in 0.49 and 0.50.

The PREFIX_KEY functionality had a comment explaining that a bug
had to be worked around, in all version prior to 0.50.  However,
that assumption was invalid, as the memcached_set_prefix_key()
behavior was only rewritten in version 0.49.  Since the error was
fixed in libmemcached-0.50, that means it only exists in one version:
0.49.

I tested this fix against every release between 0.44 - 0.50, and
confirmed that the bug only has to be worked around in *exactly*
version 0.49.

The simple test:

  $ nc -l localhost 5555

  $ php -a
  php > $mc = new Memcached();
  php > $mc->addServer('localhost', 5555, 1);
  php > $mc->setOption(Memcached::OPT_PREFIX_KEY, 'foo');
  php > $mc->set('bar', 1, 15);

I saw "foo0bar" coming through in version 0.44 - 0.48. The problem
was fixed in 0.49 and 0.50.
@iliaal
Copy link
Member

iliaal commented Nov 10, 2012

Patch was applied.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants