upload/editor: fix bytes/string confusion
The upload module tries to turn the strings into bytes before passing
to EditString, but it combines bytes & strings causing an error. The
return value might be bytes or string, but the caller only expects a
string. Lets simplify this by sticking to strings everywhere and have
EditString take care of converting to/from bytes when reading/writing
the underlying files. This also avoids possible locale confusion when
reading the file by forcing UTF-8 everywhere.
Bug: https://crbug.com/gerrit/11929
Change-Id: I07b146170c5e8b5b0500a2c79e4213cd12140a96
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/245621
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
diff --git a/editor.py b/editor.py
index 19b96c3..fcf1638 100644
--- a/editor.py
+++ b/editor.py
@@ -68,11 +68,14 @@
def EditString(cls, data):
"""Opens an editor to edit the given content.
- Args:
- data : the text to edit
+ Args:
+ data: The text to edit.
- Returns:
- new value of edited text; None if editing did not succeed
+ Returns:
+ New value of edited text.
+
+ Raises:
+ EditorError: The editor failed to run.
"""
editor = cls._GetEditor()
if editor == ':':
@@ -80,7 +83,7 @@
fd, path = tempfile.mkstemp()
try:
- os.write(fd, data)
+ os.write(fd, data.encode('utf-8'))
os.close(fd)
fd = None
@@ -106,8 +109,8 @@
raise EditorError('editor failed with exit status %d: %s %s'
% (rc, editor, path))
- with open(path) as fd2:
- return fd2.read()
+ with open(path, mode='rb') as fd2:
+ return fd2.read().decode('utf-8')
finally:
if fd:
os.close(fd)
diff --git a/subcmds/upload.py b/subcmds/upload.py
index d0dd383..5c12aae 100644
--- a/subcmds/upload.py
+++ b/subcmds/upload.py
@@ -271,11 +271,6 @@
branches[project.name] = b
script.append('')
- script = [ x.encode('utf-8')
- if issubclass(type(x), unicode)
- else x
- for x in script ]
-
script = Editor.EditString("\n".join(script)).split("\n")
project_re = re.compile(r'^#?\s*project\s*([^\s]+)/:$')
diff --git a/tests/test_editor.py b/tests/test_editor.py
new file mode 100644
index 0000000..fbcfcdb
--- /dev/null
+++ b/tests/test_editor.py
@@ -0,0 +1,60 @@
+# -*- coding:utf-8 -*-
+#
+# Copyright (C) 2019 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Unittests for the editor.py module."""
+
+from __future__ import print_function
+
+import unittest
+
+from editor import Editor
+
+
+class EditorTestCase(unittest.TestCase):
+ """Take care of resetting Editor state across tests."""
+
+ def setUp(self):
+ self.setEditor(None)
+
+ def tearDown(self):
+ self.setEditor(None)
+
+ @staticmethod
+ def setEditor(editor):
+ Editor._editor = editor
+
+
+class GetEditor(EditorTestCase):
+ """Check GetEditor behavior."""
+
+ def test_basic(self):
+ """Basic checking of _GetEditor."""
+ self.setEditor(':')
+ self.assertEqual(':', Editor._GetEditor())
+
+
+class EditString(EditorTestCase):
+ """Check EditString behavior."""
+
+ def test_no_editor(self):
+ """Check behavior when no editor is available."""
+ self.setEditor(':')
+ self.assertEqual('foo', Editor.EditString('foo'))
+
+ def test_cat_editor(self):
+ """Check behavior when editor is `cat`."""
+ self.setEditor('cat')
+ self.assertEqual('foo', Editor.EditString('foo'))
