Consolidation of variables parsing throughout codebase

* Remove attempted support of key=value pattern, because
  it is not actually allowed in practice
* Have variables validator defer to the utils variables parser
* Prune serializers of a handful of cases that previous
  attempts at cleanup have missed

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
@@ -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 = parse_yaml_or_json(variables)
-                vars_dict = json.loads(variables.strip() or '{}')
+            vars_dict['ansible_ssh_port'] = port
-                vars_dict['ansible_ssh_port'] = port
+            attrs['variables'] = json.dumps(vars_dict)
-                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.')})
 
         return super(HostSerializer, self).validate(attrs)
 
@@ -3286,18 +3276,12 @@ class JobLaunchSerializer(BaseSerializer):
 
         extra_vars = attrs.get('extra_vars', {})
 
-        if isinstance(extra_vars, basestring):
+        try:
-            try:
+            extra_vars = parse_yaml_or_json(extra_vars, silent_failure=False)
-                extra_vars = json.loads(extra_vars)
+        except:
-            except (ValueError, TypeError):
+            # Log exception, in case of yet-unknown parsing failures
-                try:
+            logger.exception('extra_vars parsing error on Job Template launch.')
-                    extra_vars = yaml.safe_load(extra_vars)
+            errors['extra_vars'] = _('Must be valid JSON or YAML.')
-                    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 = {}
 
         if self.get_survey_enabled(obj):
             validation_errors = obj.survey_variable_validation(extra_vars)
@@ -3371,18 +3355,12 @@ class WorkflowJobLaunchSerializer(BaseSerializer):
 
         extra_vars = attrs.get('extra_vars', {})
 
-        if isinstance(extra_vars, basestring):
+        try:
-            try:
+            extra_vars = parse_yaml_or_json(extra_vars, silent_failure=False)
-                extra_vars = json.loads(extra_vars)
+        except:
-            except (ValueError, TypeError):
+            # Log exception, in case of yet-unknown parsing failures
-                try:
+            logger.exception('extra_vars parsing error on Workflow JT launch.')
-                    extra_vars = yaml.safe_load(extra_vars)
+            errors['extra_vars'] = _('Must be valid JSON or YAML.')
-                    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 = {}
 
         if self.get_survey_enabled(obj):
             validation_errors = obj.survey_variable_validation(extra_vars)

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
+        return parse_yaml_or_json(v)
-        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 {}
 
     def __set__(self, obj, value):
         raise AttributeError('readonly property')

awx/main/utils/common.py

@@ -582,21 +582,32 @@ def cache_list_capabilities(page, prefetch_list, model, user):
                 obj.capabilities_cache[display_method] = True
 
 
-def parse_yaml_or_json(vars_str):
+def parse_yaml_or_json(vars_str, silent_failure=True):
     '''
-    Attempt to parse a string with variables, and if attempt fails,
+    Attempt to parse a string of variables.
-    return an empty dictionary.
+    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 the last 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):
+        assert isinstance(vars_dict, dict)
+    except (ValueError, TypeError, AssertionError):
         try:
             vars_dict = yaml.safe_load(vars_str)
+            # Can be None if '---'
+            if vars_dict is None:
+                return {}
             assert isinstance(vars_dict, dict)
         except (yaml.YAMLError, TypeError, AttributeError, AssertionError):
-            vars_dict = {}
+            if silent_failure:
+                return {}
+            raise
     return vars_dict
 
 

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 _
@@ -14,6 +12,9 @@ from django.core.exceptions import ValidationError
 # REST framework
 from rest_framework.serializers import ValidationError as RestValidationError
 
+# 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 +180,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
-
+    except:
-    try:
+        raise RestValidationError(_('Must be valid JSON or YAML.'))
-        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.'))