diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 5ac87cfee2..edd1b0d079 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -11,9 +11,6 @@ import urllib from collections import OrderedDict from dateutil import rrule -# PyYAML -import yaml - # Django from django.conf import settings from django.contrib.auth import authenticate @@ -28,7 +25,7 @@ from django.utils.timezone import now from django.utils.functional import cached_property # Django REST Framework -from rest_framework.exceptions import ValidationError, PermissionDenied +from rest_framework.exceptions import ValidationError, PermissionDenied, ParseError from rest_framework import fields from rest_framework import serializers from rest_framework import validators @@ -1312,6 +1309,9 @@ class HostSerializer(BaseSerializerWithVariables): raise serializers.ValidationError({"detail": _("Cannot create Host for Smart Inventory")}) return value + def validate_variables(self, value): + return vars_validate_or_raise(value) + def validate(self, attrs): name = force_text(attrs.get('name', self.instance and self.instance.name or '')) host, port = self._get_host_port_from_name(name) @@ -1319,19 +1319,9 @@ class HostSerializer(BaseSerializerWithVariables): if port: attrs['name'] = host variables = force_text(attrs.get('variables', self.instance and self.instance.variables or '')) - try: - vars_dict = json.loads(variables.strip() or '{}') - vars_dict['ansible_ssh_port'] = port - attrs['variables'] = json.dumps(vars_dict) - except (ValueError, TypeError): - try: - vars_dict = yaml.safe_load(variables) - if vars_dict is None: - vars_dict = {} - vars_dict['ansible_ssh_port'] = port - attrs['variables'] = yaml.dump(vars_dict) - except (yaml.YAMLError, TypeError): - raise serializers.ValidationError({'variables': _('Must be valid JSON or YAML.')}) + vars_dict = parse_yaml_or_json(variables) + vars_dict['ansible_ssh_port'] = port + attrs['variables'] = json.dumps(vars_dict) return super(HostSerializer, self).validate(attrs) @@ -3326,18 +3316,11 @@ class JobLaunchSerializer(BaseSerializer): extra_vars = attrs.get('extra_vars', {}) - if isinstance(extra_vars, basestring): - try: - extra_vars = json.loads(extra_vars) - except (ValueError, TypeError): - try: - extra_vars = yaml.safe_load(extra_vars) - assert isinstance(extra_vars, dict) - except (yaml.YAMLError, TypeError, AttributeError, AssertionError): - errors['extra_vars'] = _('Must be valid JSON or YAML.') - - if not isinstance(extra_vars, dict): - extra_vars = {} + try: + extra_vars = parse_yaml_or_json(extra_vars, silent_failure=False) + except ParseError as e: + # Catch known user variable formatting errors + errors['extra_vars'] = str(e) if self.get_survey_enabled(obj): validation_errors = obj.survey_variable_validation(extra_vars) @@ -3411,18 +3394,11 @@ class WorkflowJobLaunchSerializer(BaseSerializer): extra_vars = attrs.get('extra_vars', {}) - if isinstance(extra_vars, basestring): - try: - extra_vars = json.loads(extra_vars) - except (ValueError, TypeError): - try: - extra_vars = yaml.safe_load(extra_vars) - assert isinstance(extra_vars, dict) - except (yaml.YAMLError, TypeError, AttributeError, AssertionError): - errors['extra_vars'] = _('Must be valid JSON or YAML.') - - if not isinstance(extra_vars, dict): - extra_vars = {} + try: + extra_vars = parse_yaml_or_json(extra_vars, silent_failure=False) + except ParseError as e: + # Catch known user variable formatting errors + errors['extra_vars'] = str(e) if self.get_survey_enabled(obj): validation_errors = obj.survey_variable_validation(extra_vars) diff --git a/awx/main/models/base.py b/awx/main/models/base.py index 6c038dad74..f41f7a94e4 100644 --- a/awx/main/models/base.py +++ b/awx/main/models/base.py @@ -1,13 +1,6 @@ # Copyright (c) 2015 Ansible, Inc. # All Rights Reserved. -# Python -import json -import shlex - -# PyYAML -import yaml - # Django from django.db import models from django.core.exceptions import ValidationError, ObjectDoesNotExist @@ -21,7 +14,7 @@ from taggit.managers import TaggableManager from crum import get_current_user # AWX -from awx.main.utils import encrypt_field +from awx.main.utils import encrypt_field, parse_yaml_or_json __all__ = ['prevent_search', 'VarsDictProperty', 'BaseModel', 'CreatedModifiedModel', 'PasswordFieldsModel', 'PrimordialModel', 'CommonModel', @@ -80,26 +73,7 @@ class VarsDictProperty(object): if hasattr(v, 'items'): return v v = v.encode('utf-8') - d = None - try: - d = json.loads(v.strip() or '{}') - except ValueError: - pass - if d is None: - try: - d = yaml.safe_load(v) - # This can happen if the whole file is commented out - if d is None: - d = {} - except yaml.YAMLError: - pass - if d is None and self.key_value: - d = {} - for kv in [x.decode('utf-8') for x in shlex.split(v, posix=True)]: - if '=' in kv: - k, v = kv.split('=', 1) - d[k] = v - return d if hasattr(d, 'items') else {} + return parse_yaml_or_json(v) def __set__(self, obj, value): raise AttributeError('readonly property') diff --git a/awx/main/tests/functional/api/test_job_runtime_params.py b/awx/main/tests/functional/api/test_job_runtime_params.py index b197ac2520..be6f0b56fa 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -199,7 +199,7 @@ def test_job_reject_invalid_prompted_extra_vars(runtime_data, job_template_promp dict(extra_vars='{"unbalanced brackets":'), admin_user, expect=400) assert 'extra_vars' in response.data - assert 'valid JSON or YAML' in str(response.data['extra_vars'][0]) + assert 'Cannot parse as' in str(response.data['extra_vars'][0]) @pytest.mark.django_db diff --git a/awx/main/tests/unit/utils/test_common.py b/awx/main/tests/unit/utils/test_common.py index 41f9012040..d31f2b7472 100644 --- a/awx/main/tests/unit/utils/test_common.py +++ b/awx/main/tests/unit/utils/test_common.py @@ -5,9 +5,13 @@ import os import pytest from uuid import uuid4 +import json +import yaml from django.core.cache import cache +from rest_framework.exceptions import ParseError + from awx.main.utils import common from awx.main.models import ( @@ -38,6 +42,42 @@ def test_parse_yaml_or_json(input_, output): assert common.parse_yaml_or_json(input_) == output +class TestParserExceptions: + + @staticmethod + def json_error(data): + try: + json.loads(data) + return None + except Exception as e: + return str(e) + + @staticmethod + def yaml_error(data): + try: + yaml.load(data) + return None + except Exception as e: + return str(e) + + def test_invalid_JSON_and_YAML(self): + data = "{key:val" + with pytest.raises(ParseError) as exc: + common.parse_yaml_or_json(data, silent_failure=False) + message = str(exc.value) + assert "Cannot parse as" in message + assert self.json_error(data) in message + assert self.yaml_error(data) in message + + def test_invalid_vars_type(self): + data = "[1, 2, 3]" + with pytest.raises(ParseError) as exc: + common.parse_yaml_or_json(data, silent_failure=False) + message = str(exc.value) + assert "Cannot parse as" in message + assert "Input type `list` is not a dictionary" in message + + def test_set_environ(): key = str(uuid4()) old_environ = os.environ.copy() diff --git a/awx/main/utils/common.py b/awx/main/utils/common.py index c11e576e15..902621ca62 100644 --- a/awx/main/utils/common.py +++ b/awx/main/utils/common.py @@ -582,21 +582,48 @@ def cache_list_capabilities(page, prefetch_list, model, user): obj.capabilities_cache[display_method] = True -def parse_yaml_or_json(vars_str): +def validate_vars_type(vars_obj): + if not isinstance(vars_obj, dict): + vars_type = type(vars_obj) + if hasattr(vars_type, '__name__'): + data_type = vars_type.__name__ + else: + data_type = str(vars_type) + raise AssertionError( + _('Input type `{data_type}` is not a dictionary').format( + data_type=data_type) + ) + + +def parse_yaml_or_json(vars_str, silent_failure=True): ''' - Attempt to parse a string with variables, and if attempt fails, - return an empty dictionary. + Attempt to parse a string of variables. + First, with JSON parser, if that fails, then with PyYAML. + If both attempts fail, return an empty dictionary if `silent_failure` + is True, re-raise combination error if `silent_failure` if False. ''' if isinstance(vars_str, dict): return vars_str + elif isinstance(vars_str, basestring) and vars_str == '""': + return {} + try: vars_dict = json.loads(vars_str) - except (ValueError, TypeError): + validate_vars_type(vars_dict) + except (ValueError, TypeError, AssertionError) as json_err: try: vars_dict = yaml.safe_load(vars_str) - assert isinstance(vars_dict, dict) - except (yaml.YAMLError, TypeError, AttributeError, AssertionError): - vars_dict = {} + # Can be None if '---' + if vars_dict is None: + return {} + validate_vars_type(vars_dict) + except (yaml.YAMLError, TypeError, AttributeError, AssertionError) as yaml_err: + if silent_failure: + return {} + raise ParseError(_( + 'Cannot parse as JSON (error: {json_error}) or ' + 'YAML (error: {yaml_error}).').format( + json_error=str(json_err), yaml_error=str(yaml_err))) return vars_dict diff --git a/awx/main/validators.py b/awx/main/validators.py index e5b1568464..5af078a861 100644 --- a/awx/main/validators.py +++ b/awx/main/validators.py @@ -4,8 +4,6 @@ # Python import base64 import re -import yaml -import json # Django from django.utils.translation import ugettext_lazy as _ @@ -13,6 +11,10 @@ from django.core.exceptions import ValidationError # REST framework from rest_framework.serializers import ValidationError as RestValidationError +from rest_framework.exceptions import ParseError + +# AWX +from awx.main.utils import parse_yaml_or_json def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): @@ -179,21 +181,9 @@ def vars_validate_or_raise(vars_str): job templates, inventories, or hosts are either an acceptable blank string, or are valid JSON or YAML dict """ - if isinstance(vars_str, dict) or (isinstance(vars_str, basestring) and vars_str == '""'): + try: + parse_yaml_or_json(vars_str, silent_failure=False) return vars_str - - try: - r = json.loads((vars_str or '').strip() or '{}') - if isinstance(r, dict): - return vars_str - except ValueError: - pass - try: - r = yaml.safe_load(vars_str) - # Can be None if '---' - if isinstance(r, dict) or r is None: - return vars_str - except yaml.YAMLError: - pass - raise RestValidationError(_('Must be valid JSON or YAML.')) + except ParseError as e: + raise RestValidationError(str(e))