python-keystoneclient

  1. Bug #1175367
  2. Comment #16

Comment 16 for bug 1175367

Revision history for this message
Brant Knudson (blk-u) wrote : Re: Memcache encryption middleware improperly implemented

Comments on fix_memcache_crypt3.patch:

Mostly suggestions, but do have a couple questions.

file keystoneclient/middleware/auth_token.py:
line 848: suggest use "not ..." rather than "is None"
line 860: Not sure what "This" is referring to, is it the memcache_crypt.unprotect_data function or the _cache_get function?
line 864: no need to include exception string because LOG.exception will print it out.
line 870: suggest use "not serialized rather than "is None"
line 875: comment doesn't make sense since would have returned at 871 if serialized is None.
line 895: suggest use "not ..." rather than "is None"

file keystoneclient/middleware/memcache_crypt.py:
line 146: Should be "except Exception:", see HACKING.rst

tests/test_memcache_crypt.py:
line 17: would like to see a test for 2 empty strings and 1 empty one not.
line 23: looks like the error message is the length of the MAC? Was the intent to check all 3 are equal?
line 63: should use assertIsNone