Fix five defects: assert guards, exception message, password leak, circular import #273

Part of the repository modernization effort.

Five defects. Each one is small. Each one is independent of the Python 2 cleanup.

Defect 1: assert guards the wire protocol

bmemcached/protocol.py:253:

assert magic == self.MAGIC['response']

This is the only check on a memcached response header.

Two problems:

1. python -O strips the assert. The client then processes a corrupt stream as valid data.
2. The assert raises a bare AssertionError. The surrounding except socket.error does not catch it. The header is read. The body is not read. The socket stays open in a desynchronized state. The next call on that thread reads the leftover body as a header.

Failure case: a proxy sends a malformed header. The client wedges. It does not disconnect and retry.

Fix: raise a MemcachedException instead. Call self.disconnect() before you raise.

Defect 2: assert guards input validation

bmemcached/protocol.py:670:

assert cas != 0, '0 is an invalid CAS value'

python -O strips this assert. cas(key, value, 0, ...) then reaches _set_add_replace('set', ..., cas=0). The wire protocol reads cas=0 as "no CAS check". The call becomes an unconditional overwrite. The caller asked for a compare-and-swap.

bmemcached/client/mixin.py:71:

assert servers, "No memcached servers supplied"

python -O strips this assert too. An empty server list passes. The failure appears later with an unclear message.

Fix: raise ValueError in both places.

Defect 3: MemcachedException has no usable message

bmemcached/exceptions.py:1-4:

class MemcachedException(Exception):
    def __init__(self, message, code):
        self.message = message
        self.code = code

__init__ never calls super().__init__(message). The class defines no __str__. BaseException.__new__ still sets self.args = (message, code).

Verified:

>>> str(MemcachedException('boom', 1))
"('boom', 1)"

Every caller and every log line that calls str(exc) gets a tuple repr, not the message. AuthenticationNotSupported and InvalidCredentials inherit the same defect.

Fix: add __str__. Return the message and the code.

Defect 4: __str__ prints the SASL password

bmemcached/protocol.py:129-130:

def __str__(self):
    return "{}_{}_{}".format(self.server, self._username, self._password)

The plaintext password reaches any log line, any repr() of a server list, and any traceback that prints a Protocol object.

Fix: remove the password from the string, or mask it.

Defect 5: fragile circular import

bmemcached/client/__init__.py:1-4:

from bmemcached.client.constants import SOCKET_TIMEOUT

from .replicating import ReplicatingClient
from .distributed import DistributedClient

bmemcached/client/distributed.py:4:

from bmemcached.client import SOCKET_TIMEOUT

distributed.py reads the name back off the partly initialized package module. The import works only because of the line order in __init__.py. A reorder raises ImportError at import time. No test catches this.

bmemcached/client/mixin.py:3 already uses the safe form.

Fix: change distributed.py:4 to from bmemcached.client.constants import SOCKET_TIMEOUT.

Acceptance criteria

• bmemcached/protocol.py:253 raises a MemcachedException on a bad magic byte. It calls self.disconnect() first.
• bmemcached/protocol.py:670 raises ValueError when cas is 0.
• bmemcached/client/mixin.py:71 raises ValueError on an empty server list.
• MemcachedException defines __str__. str(MemcachedException("boom", 1)) holds the text boom.
• Protocol.__str__ does not hold the password.
• bmemcached/client/distributed.py:4 imports from bmemcached.client.constants.
• New test: a short or malformed response header raises a clear exception. Protocol.connection is None after the call.
• New test: cas(key, value, 0, ...) raises ValueError.
• New test: str(MemcachedException("boom", 1)) holds boom.
• New test: str(protocol_instance) does not hold the configured password.
• The full test suite passes.

Files to change

bmemcached/protocol.py, bmemcached/exceptions.py, bmemcached/client/mixin.py, bmemcached/client/distributed.py, test/test_errors.py, test/test_auth.py.

Order

Independent of the Python 2 cleanup issues. Do this issue before the typing issue. It changes the shape of MemcachedException.