Repository: Azure/azure-cli

Issue:
"@file support": should handle the BOM on file loading
Otherwise, using `@<parameter-file>` will return an invalid json string for the command to use. Since json file is the main file type, so the core layer should handle it.
CLI already have [the logic](https://github.com/Azure/azure-cli/blob/master/src/azure-cli-core/azure/cli/core/_util.py#L91) to handle BOM, so we just need to incorporate the same mechanism to [here](https://github.com/Azure/azure-cli/blob/master/src/azure-cli-core/azure/cli/core/application.py#L244) 


Discussion:

Patch:
diff --git a/src/azure-cli-core/azure/cli/core/_util.py b/src/azure-cli-core/azure/cli/core/_util.py
index c194599bc..379152887 100644
--- a/src/azure-cli-core/azure/cli/core/_util.py
+++ b/src/azure-cli-core/azure/cli/core/_util.py
@@ -94,21 +94,27 @@ def get_json_object(json_string):
 
 
 def get_file_json(file_path, throw_on_empty=True):
+    content = read_file_content(file_path)
+    if not content and not throw_on_empty:
+        return None
+    return json.loads(content)
+
+
+def read_file_content(file_path, allow_binary=False):
     from codecs import open as codecs_open
-    # always try 'utf-8-sig' first, so that BOM in WinOS won't cause trouble.
-    for encoding in ('utf-8-sig', 'utf-8', 'utf-16', 'utf-16le', 'utf-16be'):
+    # Note, always put 'utf-8-sig' first, so that BOM in WinOS won't cause trouble.
+    for encoding in ['utf-8-sig', 'utf-8', 'utf-16', 'utf-16le', 'utf-16be']:
         try:
             with codecs_open(file_path, encoding=encoding) as f:
-                text = f.read()
-
-            if not text and not throw_on_empty:
-                return None
-
-            return json.loads(text)
+                return f.read()
+        except UnicodeDecodeError:
+            if allow_binary:
+                with open(file_path, 'rb') as input_file:
+                    return input_file.read()
+            else:
+                raise
         except UnicodeError:
             pass
-        except Exception as ex:
-            raise CLIError("File '{}' contains error: {}".format(file_path, str(ex)))
 
     raise CLIError('Failed to decode file {} - unknown decoding'.format(file_path))
 
diff --git a/src/azure-cli-core/azure/cli/core/application.py b/src/azure-cli-core/azure/cli/core/application.py
index e651a2c8a..72b282d16 100644
--- a/src/azure-cli-core/azure/cli/core/application.py
+++ b/src/azure-cli-core/azure/cli/core/application.py
@@ -13,7 +13,7 @@ from azure.cli.core._output import CommandResultItem
 import azure.cli.core.extensions
 import azure.cli.core._help as _help
 import azure.cli.core.azlogging as azlogging
-from azure.cli.core._util import todict, truncate_text, CLIError
+from azure.cli.core._util import todict, truncate_text, CLIError, read_file_content
 from azure.cli.core._config import az_config
 
 import azure.cli.core.telemetry as telemetry
@@ -237,19 +237,13 @@ class Application(object):
 
     @staticmethod
     def _load_file(path):
-        try:
-            if path == '-':
-                content = sys.stdin.read()
-            else:
-                try:
-                    with open(os.path.expanduser(path), 'r') as input_file:
-                        content = input_file.read()
-                except UnicodeDecodeError:
-                    with open(os.path.expanduser(path), 'rb') as input_file:
-                        content = input_file.read()
-            return content[0:-1] if content[-1] == '\n' else content
-        except:
-            raise CLIError('Failed to open file {}'.format(path))
+        if path == '-':
+            content = sys.stdin.read()
+        else:
+            content = read_file_content(os.path.expanduser(path),
+                                        allow_binary=True)
+
+        return content[0:-1] if content and content[-1] == '\n' else content
 
     def _handle_builtin_arguments(self, **kwargs):
         args = kwargs['args']


Test patch:
diff --git a/src/azure-cli-core/azure/cli/core/tests/test_application.py b/src/azure-cli-core/azure/cli/core/tests/test_application.py
index 42f09f10f..2142b0515 100644
--- a/src/azure-cli-core/azure/cli/core/tests/test_application.py
+++ b/src/azure-cli-core/azure/cli/core/tests/test_application.py
@@ -87,13 +87,21 @@ class TestApplication(unittest.TestCase):
         f = tempfile.NamedTemporaryFile(delete=False)
         f.close()
 
+        f_with_bom = tempfile.NamedTemporaryFile(delete=False)
+        f_with_bom.close()
+
         with open(f.name, 'w+') as stream:
             stream.write('foo')
 
+        from codecs import open as codecs_open
+        with codecs_open(f_with_bom.name, encoding='utf-8-sig', mode='w+') as stream:
+            stream.write('foo')
+
         cases = [
             [['bar=baz'], ['bar=baz']],
             [['bar', 'baz'], ['bar', 'baz']],
             [['bar=@{}'.format(f.name)], ['bar=foo']],
+            [['bar=@{}'.format(f_with_bom.name)], ['bar=foo']],
             [['bar', '@{}'.format(f.name)], ['bar', 'foo']],
             [['bar', f.name], ['bar', f.name]],
             [['bar=name@company.com'], ['bar=name@company.com']],