Repository: dmwm/wmcore
Issue: [py2py3] check and test the use of hashlib.md5()

**Impact of the bug**

All of WMCore

**Describe the bug**

 We have not observed any bug at the moment, but this is very likely to create some problems.

When we use 

```python
    keyhash = hashlib.md5()
    keyhash.update(input)
```

`input` must be of type `bytes` in both py2 and py3. Often we simply use `.update(str(input))` to format the input for the hash function. I missed a couple of occurrences and I have to fix these, then we will need to check that all the occurrences properly work

- [ ] `src/python/WMCore/WebTools/Page.py`: change code, test
- [ ] `src/python/WMCore/Services/LogDB/LogDBBackend.py`: test
- [ ] `src/python/WMCore/MicroService/TaskManager.py`: test
- [ ] `src/python/WMCore/Database/CMSCouch.py`: change code, test
- [ ] `src/python/WMCore/Cache/WMConfigCache.py`: test
- [ ] `setup_test.py`: change code, test
- [ ] `test/python/WMCore_t/Database_t/CMSCouch_t.py`: change code, test

**Expected behavior**
We should make sure that every `input` is formatted to bytes in both py2 and py3



Changed files:
test/python/WMCore_t/WMSpec_t/StdSpecs_t/TaskChain_t.py, src/python/WMCore/ReqMgr/Utils/Validation.py, src/python/WMCore/WebTools/Page.py, src/python/WMCore/Services/LogDB/LogDBBackend.py, src/python/WMCore/WorkQueue/WMBSHelper.py, src/python/WMCore/ReqMgr/Web/utils.py, test/python/WMCore_t/WMSpec_t/StdSpecs_t/StepChain_t.py, src/python/WMCore/MicroService/TaskManager.py

diff --git a/src/python/WMCore/MicroService/TaskManager.py b/src/python/WMCore/MicroService/TaskManager.py
index 3732b8571..305596f68 100644
--- a/src/python/WMCore/MicroService/TaskManager.py
+++ b/src/python/WMCore/MicroService/TaskManager.py
@@ -7,10 +7,10 @@ Author: Valentin Kuznetsov <vkuznet [AT] gmail [DOT] com>
 from __future__ import division
 
 from builtins import range, object
-
 from future import standard_library
 standard_library.install_aliases()
 
+
 # system modules
 import time
 import json
@@ -19,8 +19,10 @@ import threading
 from queue import Queue
 
 # WMCore modules
+from Utils.PythonVersion import PY3
 from WMCore.MicroService.Tools.Common import getMSLogger
-from Utils.Utilities import encodeUnicodeToBytes
+from Utils.Utilities import encodeUnicodeToBytesConditional
+
 
 def genkey(query):
     """
@@ -31,12 +33,7 @@ def genkey(query):
         record = dict(query)
         query = json.JSONEncoder(sort_keys=True).encode(record)
     keyhash = hashlib.md5()
-    query = encodeUnicodeToBytes(query)
-    try:
-        keyhash.update(query)
-    except TypeError: # python3
-        # this may be avoided if we use encodeUnicodeToBytes(query) above
-        keyhash.update(query.encode('ascii'))
+    keyhash.update(encodeUnicodeToBytesConditional(query, condition=PY3))
     return keyhash.hexdigest()
 
 def set_thread_name(ident, name):
diff --git a/src/python/WMCore/ReqMgr/Utils/Validation.py b/src/python/WMCore/ReqMgr/Utils/Validation.py
index 7d3298cdc..3e1e5831c 100644
--- a/src/python/WMCore/ReqMgr/Utils/Validation.py
+++ b/src/python/WMCore/ReqMgr/Utils/Validation.py
@@ -6,6 +6,9 @@ from __future__ import print_function
 from future.utils import viewitems, viewvalues
 
 from hashlib import md5
+
+from Utils.PythonVersion import PY3
+from Utils.Utilities import encodeUnicodeToBytesConditional
 from WMCore.Lexicon import procdataset
 from WMCore.REST.Auth import authz_match
 from WMCore.ReqMgr.Auth import getWritePermission
@@ -275,7 +278,7 @@ def _validateDatatier(datatier, dbsUrl, expiration=3600):
     Provided a list of datatiers extracted from the outputDatasets, checks
     whether they all exist in DBS.
     """
-    cacheName = "dataTierList_" + md5(dbsUrl).hexdigest()
+    cacheName = "dataTierList_" + md5(encodeUnicodeToBytesConditional(dbsUrl, condition=PY3)).hexdigest()
     if not GenericDataCache.cacheExists(cacheName):
         mc = MemoryCacheStruct(expiration, getDataTiers, kwargs={'dbsUrl': dbsUrl})
         GenericDataCache.registerCache(cacheName, mc)
diff --git a/src/python/WMCore/ReqMgr/Web/utils.py b/src/python/WMCore/ReqMgr/Web/utils.py
index eb0f4e1e7..6357045c0 100644
--- a/src/python/WMCore/ReqMgr/Web/utils.py
+++ b/src/python/WMCore/ReqMgr/Web/utils.py
@@ -11,6 +11,9 @@ from builtins import str as newstr, bytes, int
 from future.utils import viewitems
 
 from future import standard_library
+
+from Utils.PythonVersion import PY3
+
 standard_library.install_aliases()
 
 # system modules
@@ -22,7 +25,7 @@ import cherrypy
 from urllib.error import URLError
 
 # WMCore Modules
-from Utils.Utilities import encodeUnicodeToBytes
+from Utils.Utilities import encodeUnicodeToBytes, encodeUnicodeToBytesConditional
 
 
 def tstamp():
@@ -32,7 +35,7 @@ def tstamp():
 def gen_color(val):
     "Generate unique color code for given string value"
     keyhash = hashlib.md5()
-    keyhash.update(val)
+    keyhash.update(encodeUnicodeToBytesConditional(val, condition=PY3))
     col = '#%s' % keyhash.hexdigest()[:6]
     return col
 
@@ -162,8 +165,7 @@ def genid(kwds):
     else:
         data = str(kwds)  # it is fine both in py2 and in py3
     keyhash = hashlib.md5()
-    data = encodeUnicodeToBytes(data)  # if data is not unicode, then it is not changed
-    keyhash.update(data)
+    keyhash.update(encodeUnicodeToBytesConditional(data, condition=PY3))
     return keyhash.hexdigest()
 
 def checkarg(kwds, arg):
diff --git a/src/python/WMCore/Services/LogDB/LogDBBackend.py b/src/python/WMCore/Services/LogDB/LogDBBackend.py
index 51f0c3e1f..7af0f253f 100644
--- a/src/python/WMCore/Services/LogDB/LogDBBackend.py
+++ b/src/python/WMCore/Services/LogDB/LogDBBackend.py
@@ -6,7 +6,9 @@ Interface to LogDB persistent storage
 """
 
 from builtins import object, bytes
-from Utils.Utilities import encodeUnicodeToBytes
+
+from Utils.PythonVersion import PY3
+from Utils.Utilities import encodeUnicodeToBytes, encodeUnicodeToBytesConditional
 
 # system modules
 import datetime
@@ -26,7 +28,7 @@ def gen_hash(key):
     if not isinstance(key, bytes):
         raise NotImplementedError
     keyhash = hashlib.md5()
-    keyhash.update(key)
+    keyhash.update(encodeUnicodeToBytesConditional(key, condition=PY3))
     return keyhash.hexdigest()
 
 def tstamp():
diff --git a/src/python/WMCore/WebTools/Page.py b/src/python/WMCore/WebTools/Page.py
index 63386c046..d62cca0e6 100644
--- a/src/python/WMCore/WebTools/Page.py
+++ b/src/python/WMCore/WebTools/Page.py
@@ -3,7 +3,7 @@
 Some generic base classes for building web pages with.
 """
 
-from builtins import str
+from builtins import str, bytes
 
 import hashlib
 import json
@@ -20,6 +20,8 @@ from Cheetah.Template import Template
 from cherrypy import log as cplog
 from cherrypy import request
 
+from Utils.PythonVersion import PY3
+from Utils.Utilities import encodeUnicodeToBytesConditional
 from WMCore.DataStructs.WMObject import WMObject
 from WMCore.WMFactory import WMFactory
 from WMCore.Wrappers.JsonWrapper.JSONThunker import JSONThunker
@@ -378,7 +380,10 @@ def runDas(self, func, data, expires):
 
     keyhash = hashlib.md5()
 
-    keyhash.update(str(results))
+    if not isinstance(results, (str, bytes)):
+        keyhash.update(encodeUnicodeToBytesConditional(str(results), condition=PY3))
+    else:
+        keyhash.update(encodeUnicodeToBytesConditional(results, condition=PY3))
     res_checksum = keyhash.hexdigest()
     dasdata = {'application': '%s.%s' % (self.config.application, func.__name__),
                'request_timestamp': start_time,
diff --git a/src/python/WMCore/WorkQueue/WMBSHelper.py b/src/python/WMCore/WorkQueue/WMBSHelper.py
index 4c126ae3e..0b0cb0990 100644
--- a/src/python/WMCore/WorkQueue/WMBSHelper.py
+++ b/src/python/WMCore/WorkQueue/WMBSHelper.py
@@ -12,6 +12,8 @@ import logging
 import threading
 from collections import defaultdict
 
+from Utils.PythonVersion import PY3
+from Utils.Utilities import encodeUnicodeToBytesConditional
 from WMComponent.DBS3Buffer.DBSBufferDataset import DBSBufferDataset
 from WMComponent.DBS3Buffer.DBSBufferFile import DBSBufferFile
 from WMCore.BossAir.BossAirAPI import BossAirAPI, BossAirException
@@ -225,6 +227,7 @@ class WMBSHelper(WMConnectionBase):
             if self.mask:
                 from hashlib import md5
                 mask_string = ",".join(["%s=%s" % (x, self.mask[x]) for x in sorted(self.mask)])
+                mask_string = encodeUnicodeToBytesConditional(mask_string, condition=PY3)
                 filesetName += "-%s" % md5(mask_string).hexdigest()
         else:
             filesetName = topLevelFilesetName
diff --git a/test/python/WMCore_t/WMSpec_t/StdSpecs_t/StepChain_t.py b/test/python/WMCore_t/WMSpec_t/StdSpecs_t/StepChain_t.py
index a81078bb7..231bc4bb1 100644
--- a/test/python/WMCore_t/WMSpec_t/StdSpecs_t/StepChain_t.py
+++ b/test/python/WMCore_t/WMSpec_t/StdSpecs_t/StepChain_t.py
@@ -18,6 +18,7 @@ from copy import deepcopy
 from hashlib import md5
 
 from Utils.PythonVersion import PY3
+from Utils.Utilities import encodeUnicodeToBytesConditional
 
 from WMCore.DAOFactory import DAOFactory
 from WMCore.DataStructs.Mask import Mask
@@ -1733,6 +1734,7 @@ class StepChainTests(EmulatedUnitTestCase):
 
         # same function as in WMBSHelper, otherwise we cannot know which fileset name is
         maskString = ",".join(["%s=%s" % (x, myMask[x]) for x in sorted(myMask)])
+        maskString = encodeUnicodeToBytesConditional(maskString, condition=PY3)
         topFilesetName = 'TestWorkload-GENSIM-%s' % md5(maskString).hexdigest()
         expFsets[0] = topFilesetName
         # returns a tuple of id, name, open and last_update
@@ -1755,6 +1757,7 @@ class StepChainTests(EmulatedUnitTestCase):
 
         # same function as in WMBSHelper, otherwise we cannot know which fileset name is
         maskString = ",".join(["%s=%s" % (x, myMask[x]) for x in sorted(myMask)])
+        maskString = encodeUnicodeToBytesConditional(maskString, condition=PY3)
         topFilesetName = 'TestWorkload-GENSIM-%s' % md5(maskString).hexdigest()
         expFsets.append(topFilesetName)
         # returns a tuple of id, name, open and last_update
@@ -2064,6 +2067,7 @@ class StepChainTests(EmulatedUnitTestCase):
 
         # same function as in WMBSHelper, otherwise we cannot know which fileset name is
         maskString = ",".join(["%s=%s" % (x, myMask[x]) for x in sorted(myMask)])
+        maskString = encodeUnicodeToBytesConditional(maskString, condition=PY3)
         topFilesetName = 'TestWorkload-GENSIM-%s' % md5(maskString).hexdigest()
         expFsets[0] = topFilesetName
         # returns a tuple of id, name, open and last_update
@@ -2086,6 +2090,7 @@ class StepChainTests(EmulatedUnitTestCase):
 
         # same function as in WMBSHelper, otherwise we cannot know which fileset name is
         maskString = ",".join(["%s=%s" % (x, myMask[x]) for x in sorted(myMask)])
+        maskString = encodeUnicodeToBytesConditional(maskString, condition=PY3)
         topFilesetName = 'TestWorkload-GENSIM-%s' % md5(maskString).hexdigest()
         expFsets.append(topFilesetName)
         # returns a tuple of id, name, open and last_update
diff --git a/test/python/WMCore_t/WMSpec_t/StdSpecs_t/TaskChain_t.py b/test/python/WMCore_t/WMSpec_t/StdSpecs_t/TaskChain_t.py
index 414c74863..81a494a13 100644
--- a/test/python/WMCore_t/WMSpec_t/StdSpecs_t/TaskChain_t.py
+++ b/test/python/WMCore_t/WMSpec_t/StdSpecs_t/TaskChain_t.py
@@ -21,6 +21,7 @@ from copy import deepcopy
 from hashlib import md5
 
 from Utils.PythonVersion import PY3
+from Utils.Utilities import encodeUnicodeToBytesConditional
 
 from WMCore.DAOFactory import DAOFactory
 from WMCore.DataStructs.Mask import Mask
@@ -1748,6 +1749,7 @@ class TaskChainTests(EmulatedUnitTestCase):
 
         # same function as in WMBSHelper, otherwise we cannot know which fileset name is
         maskString = ",".join(["%s=%s" % (x, myMask[x]) for x in sorted(myMask)])
+        maskString = encodeUnicodeToBytesConditional(maskString, condition=PY3)
         topFilesetName = 'TestWorkload-GenSim-%s' % md5(maskString).hexdigest()
         expFsets[0] = topFilesetName
         # returns a tuple of id, name, open and last_update
@@ -1770,6 +1772,7 @@ class TaskChainTests(EmulatedUnitTestCase):
 
         # same function as in WMBSHelper, otherwise we cannot know which fileset name is
         maskString = ",".join(["%s=%s" % (x, myMask[x]) for x in sorted(myMask)])
+        maskString = encodeUnicodeToBytesConditional(maskString, condition=PY3)
         topFilesetName = 'TestWorkload-GenSim-%s' % md5(maskString).hexdigest()
         expFsets.append(topFilesetName)
         # returns a tuple of id, name, open and last_update