gclient_utils: Make FileRead always return a Unicode string.

Previously, it returned bytes if the file did not contain valid Unicode data,
and a Unicode string otherwise.
It is confusing to reason about such a function, and no current caller needs
bytes data AFAICT, so make FileRead always return Unicode strings.

Bug: 1009814
Change-Id: I89dd1935e5d4fcaf9af71585b85bda6c47695950
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1880013Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

gclient_utils.py

@@ -164,19 +164,16 @@ class PrintableObject(object):
     return output
 
 
-def FileRead(filename, mode='rU'):
+def FileRead(filename, mode='rbU'):
+  # Always decodes output to a Unicode string.
   # On Python 3 newlines are converted to '\n' by default and 'U' is deprecated.
-  if mode == 'rU' and sys.version_info.major == 3:
-    mode = 'r'
+  if mode == 'rbU' and sys.version_info.major == 3:
+    mode = 'rb'
   with open(filename, mode=mode) as f:
-    # codecs.open() has different behavior than open() on python 2.6 so use
-    # open() and decode manually.
     s = f.read()
-    try:
-      return s.decode('utf-8')
-    # AttributeError is for Py3 compatibility
-    except (UnicodeDecodeError, AttributeError):
-      return s
+    if isinstance(s, bytes):
+      return s.decode('utf-8', 'replace')
+    return s
 
 
 def FileWrite(filename, content, mode='w'):

tests/gclient_utils_test.py

@@ -10,6 +10,7 @@ from __future__ import unicode_literals
 import io
 import os
 import sys
+import tempfile
 import time
 import unittest
 
@@ -334,6 +335,26 @@ class GClientUtilsTest(trial_dir.TestCase):
       self.assertEqual(
           expected, gclient_utils.ParseCodereviewSettingsContent(content))
 
+  def testFileRead_Bytes(self):
+    with tempfile.NamedTemporaryFile(delete=False) as tmp:
+      tmp.write(b'foo \xe2\x9c bar')
+      # NamedTemporaryFiles must be closed on Windows before being opened again.
+      tmp.close()
+      try:
+        self.assertEqual('foo \ufffd bar', gclient_utils.FileRead(tmp.name))
+      finally:
+        os.remove(tmp.name)
+
+  def testFileRead_Unicode(self):
+    with tempfile.NamedTemporaryFile(delete=False) as tmp:
+      tmp.write(b'foo \xe2\x9c\x94 bar')
+      # NamedTemporaryFiles must be closed on Windows before being opened again.
+      tmp.close()
+      try:
+        self.assertEqual('foo ✔ bar', gclient_utils.FileRead(tmp.name))
+      finally:
+        os.remove(tmp.name)
+
 
 if __name__ == '__main__':
   unittest.main()