diff --git a/src/azure-cli-core/azure/cli/core/commands/tests/test_validators.py b/src/azure-cli-core/azure/cli/core/commands/tests/test_validators.py new file mode 100644 index 00000000000..5f9178008df --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/commands/tests/test_validators.py @@ -0,0 +1,55 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import unittest +import os + +from azure.cli.core.commands.validators import validate_file_or_dict, JSON_RECOMMENDATION_MESSAGE +from azure.cli.core.azclierror import InvalidArgumentValueError + + +class TestValidators(unittest.TestCase): + + def test_validate_file_or_dict(self): + json_str = '{"name": "my-resource"}' + curr_dir = os.path.dirname(os.path.realpath(__file__)) + temp_file = os.path.join(curr_dir, 'temp.json') + + # Parse JSON file + with open(temp_file, 'w') as f: + f.write(json_str) + json_read = validate_file_or_dict(temp_file) + assert json_read['name'] == 'my-resource' + os.remove(temp_file) + + # Parse in-line JSON string + json_read = validate_file_or_dict(json_str) + assert json_read['name'] == 'my-resource' + + # Test error 1: Parse JSON file with error + with open(temp_file, 'w') as f: + f.write(json_str) + f.write('error!') + with self.assertRaisesRegex(InvalidArgumentValueError, 'Failed to parse file') as ex: + validate_file_or_dict(temp_file) + assert ex.exception.recommendations[0] == JSON_RECOMMENDATION_MESSAGE + assert len(ex.exception.recommendations) == 1 + os.remove(temp_file) + + # Test error 2: A non-existing file, but it looks like a JSON file + with self.assertRaisesRegex(InvalidArgumentValueError, 'JSON file does not exist') as ex: + validate_file_or_dict("not-exist.json") + assert ex.exception.recommendations[0] == JSON_RECOMMENDATION_MESSAGE + assert len(ex.exception.recommendations) == 1 + + # Test error 3: A non-existing file, or invalid JSON string + with self.assertRaisesRegex(InvalidArgumentValueError, 'Failed to parse string as JSON') as ex: + validate_file_or_dict("invalid-string") + assert ex.exception.recommendations[0] == JSON_RECOMMENDATION_MESSAGE + assert 'The provided JSON string may have been parsed by the shell.' in ex.exception.recommendations[1] + + +if __name__ == '__main__': + unittest.main() diff --git a/src/azure-cli-core/azure/cli/core/commands/validators.py b/src/azure-cli-core/azure/cli/core/commands/validators.py index 1a0296f5928..466fb284bfe 100644 --- a/src/azure-cli-core/azure/cli/core/commands/validators.py +++ b/src/azure-cli-core/azure/cli/core/commands/validators.py @@ -10,8 +10,11 @@ from azure.cli.core.profiles import ResourceType from knack.log import get_logger +from knack.util import CLIError from knack.validators import DefaultStr, DefaultInt # pylint: disable=unused-import +JSON_RECOMMENDATION_MESSAGE = "Please provide a valid JSON file path or JSON string." + logger = get_logger(__name__) @@ -82,14 +85,32 @@ def get_default_location_from_resource_group(cmd, namespace): def validate_file_or_dict(string): + """Parse string as a JSON file or in-line JSON string.""" import os string = os.path.expanduser(string) - if os.path.exists(string): - from azure.cli.core.util import get_file_json - return get_file_json(string) - - from azure.cli.core.util import shell_safe_json_parse - return shell_safe_json_parse(string) + try: + if os.path.exists(string): + from azure.cli.core.util import get_file_json + # Error 1: 'string' is an existing file path, but the file contains invalid JSON string + # ex has no recommendation + return get_file_json(string) + + # Error 2: If string ends with '.json', it can't be a JSON string, since a JSON string must ends with + # ], }, or ", so it must be JSON file, and we don't allow parsing it as in-line string + if string.endswith('.json'): + raise CLIError("JSON file does not exist: '{}'".format(string)) + + from azure.cli.core.util import shell_safe_json_parse + # Error 3: string is a non-existing file path or invalid JSON string + # ex has recommendations for shell interpretation + return shell_safe_json_parse(string) + except CLIError as ex: + from azure.cli.core.azclierror import InvalidArgumentValueError + new_ex = InvalidArgumentValueError(ex, recommendation=JSON_RECOMMENDATION_MESSAGE) + # Preserve the recommendation + if hasattr(ex, "recommendations"): + new_ex.set_recommendation(ex.recommendations) + raise new_ex from ex def validate_parameter_set(namespace, required, forbidden, dest_to_options=None, description=None): @@ -100,8 +121,6 @@ def validate_parameter_set(namespace, required, forbidden, dest_to_options=None, included_forbidden = [x for x in forbidden if getattr(namespace, x) and not hasattr(getattr(namespace, x), 'is_default')] if missing_required or included_forbidden: - from knack.util import CLIError - def _dest_to_option(dest): try: return dest_to_options[dest] diff --git a/src/azure-cli-core/azure/cli/core/util.py b/src/azure-cli-core/azure/cli/core/util.py index 94cce31ae92..c95528f47a4 100644 --- a/src/azure-cli-core/azure/cli/core/util.py +++ b/src/azure-cli-core/azure/cli/core/util.py @@ -523,7 +523,8 @@ def get_file_json(file_path, throw_on_empty=True, preserve_order=False): try: return shell_safe_json_parse(content, preserve_order) except CLIError as ex: - raise CLIError("Failed to parse {} with exception:\n {}".format(file_path, ex)) + # Reading file bypasses shell interpretation, so we discard the recommendation for shell quoting. + raise CLIError("Failed to parse file '{}' with exception:\n{}".format(file_path, ex)) def read_file_content(file_path, allow_binary=False): @@ -563,12 +564,13 @@ def shell_safe_json_parse(json_or_dict_string, preserve_order=False, strict=True except Exception as ex: logger.debug(ex) # log the exception which could be a python dict parsing error. - # Echo the JSON received by CLI - msg = "Failed to parse JSON: {}\nError detail: {}".format(json_or_dict_string, json_ex) + # Echo the string received by CLI. Because the user may intend to provide a file path, we don't decisively + # say it is a JSON string. + msg = "Failed to parse string as JSON:\n{}\nError detail: {}".format(json_or_dict_string, json_ex) # Recommendation for all shells from azure.cli.core.azclierror import InvalidArgumentValueError - recommendation = "The JSON may have been parsed by the shell. See " \ + recommendation = "The provided JSON string may have been parsed by the shell. See " \ "https://docs.microsoft.com/cli/azure/use-cli-effectively#use-quotation-marks-in-arguments" # Recommendation especially for PowerShell