Merge pull request #81 from DMTF/Invalid-JSON-Handling

Added methods to handle invalid JSON in responses

redfish_protocol_validator/accounts.py

@@ -20,7 +20,7 @@ def get_user_names(sut: SystemUnderTest, session,
     users = set()
     response = sut.get_response('GET', sut.accounts_uri)
     if response.status_code == requests.codes.OK:
-        data = response.json()
+        data = utils.get_response_json(response)
         uris = [m.get('@odata.id') for m in data.get('Members', []) if
                 m.get('@odata.id')]
         responses = sut.get_responses_by_method(
@@ -34,7 +34,7 @@ def get_user_names(sut: SystemUnderTest, session,
                                  resource_type=ResourceType.MANAGER_ACCOUNT,
                                  request_type=request_type)
             if response.status_code == requests.codes.OK:
-                data = response.json()
+                data = utils.get_response_json(response)
                 user = data.get('UserName')
                 if user:
                     users.add(user)
@@ -47,7 +47,7 @@ def get_available_roles(sut: SystemUnderTest, session,
     roles = set()
     response = sut.get_response('GET', sut.roles_uri)
     if response.status_code == requests.codes.OK:
-        data = response.json()
+        data = utils.get_response_json(response)
         uris = [m.get('@odata.id') for m in data.get('Members', []) if
                 m.get('@odata.id')]
         responses = sut.get_responses_by_method(
@@ -61,7 +61,7 @@ def get_available_roles(sut: SystemUnderTest, session,
                                  resource_type=ResourceType.ROLE,
                                  request_type=request_type)
             if response.status_code == requests.codes.OK:
-                data = response.json()
+                data = utils.get_response_json(response)
                 role = data.get('Id')
                 if role:
                     roles.add(role)
@@ -96,7 +96,7 @@ def new_password(sut: SystemUnderTest, length=16, upper=1, lower=1,
     response = sut.get_response('GET', sut.account_service_uri)
     try:
         if response.ok:
-            data = response.json()
+            data = utils.get_response_json(response)
             if 'MinPasswordLength' in data and length < data['MinPasswordLength']:
                 length = data['MinPasswordLength']
             elif 'MaxPasswordLength' in data and length > data['MaxPasswordLength']:
@@ -120,7 +120,7 @@ def new_password(sut: SystemUnderTest, length=16, upper=1, lower=1,
 def find_empty_account_slot(sut: SystemUnderTest, session,
                             request_type=RequestType.NORMAL):
     response = sut.get_response('GET', sut.accounts_uri)
-    data = response.json()
+    data = utils.get_response_json(response)
     uris = [m.get('@odata.id') for m in data.get('Members', []) if
             m.get('@odata.id')]
     responses = sut.get_responses_by_method(
@@ -137,7 +137,7 @@ def find_empty_account_slot(sut: SystemUnderTest, session,
                              resource_type=ResourceType.MANAGER_ACCOUNT,
                              request_type=request_type)
         if response.status_code == requests.codes.OK:
-            data = response.json()
+            data = utils.get_response_json(response)
             if data.get('UserName') == '' and not data.get('Enabled', True):
                 return uri
     return None
@@ -162,7 +162,7 @@ def add_account_via_patch(sut: SystemUnderTest, session, user, role, password,
         response = session.get(sut.rhost + uri)
         if response.ok:
             # Enable the account if it not already enabled
-            data = response.json()
+            data = utils.get_response_json(response)
             if 'Enabled' in data and data['Enabled'] is False:
                 headers = utils.get_etag_header(sut, session, uri)
                 payload = {'Enabled': True}
@@ -208,7 +208,7 @@ def add_account(sut: SystemUnderTest, session,
         if location:
             new_acct_uri = urlparse(location).path
         else:
-            new_acct_uri = response.json().get('@odata.id')
+            new_acct_uri = utils.get_response_json(response).get('@odata.id')
         response = session.get(sut.rhost + new_acct_uri)
         sut.add_response(new_acct_uri, response,
                          resource_type=ResourceType.MANAGER_ACCOUNT,
@@ -286,7 +286,7 @@ def delete_account_via_patch(sut: SystemUnderTest, session, user, acct_uri,
                              request_type=RequestType.NORMAL):
     response = sut.get_response('GET', acct_uri)
     if response.status_code == requests.codes.OK:
-        data = response.json()
+        data = utils.get_response_json(response)
         if data and data.get('UserName') == user:
             payload = {'UserName': ''}
             if data.get('Enabled', False):

redfish_protocol_validator/protocol_details.py

@@ -262,7 +262,7 @@ def test_valid_etag(sut: SystemUnderTest, uri, response):
         source = 'header'
         if (etag is None and utils.get_response_media_type(response)
                 == 'application/json'):
-            data = response.json()
+            data = utils.get_response_json(response)
             if '@odata.etag' in data:
                 source = 'property'
                 etag = data.get('@odata.etag')

redfish_protocol_validator/resources.py

@@ -38,13 +38,13 @@ def find_certificates(sut: SystemUnderTest, data):
         r = sut.session.get(sut.rhost + uri)
         yield {'uri': uri, 'response': r}
         if r.ok:
-            d = r.json()
+            d = utils.get_response_json(r)
             if 'HTTPS' in d and 'Certificates' in d['HTTPS']:
                 coll_uri = d['HTTPS']['Certificates']['@odata.id']
                 r = sut.session.get(sut.rhost + coll_uri)
                 yield {'uri': coll_uri, 'response': r}
                 if r.ok:
-                    d = r.json()
+                    d = utils.get_response_json(r)
                     if 'Members' in d and len(d['Members']):
                         for m in d['Members']:
                             uri = m['@odata.id']
@@ -85,7 +85,7 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
     # do GET on the service root
     r = sut.session.get(sut.rhost + uri)
     yield {'uri': uri, 'response': r}
-    root = r.json() if r.status_code == requests.codes.OK else {}
+    root = utils.get_response_json(r) if r.status_code == requests.codes.OK else {}
 
     sut.set_version(root.get('RedfishVersion', '1.0.0'))
     sut.set_product(root.get('Product', 'N/A'))
@@ -99,7 +99,7 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
             r = sut.session.get(sut.rhost + uri)
             yield {'uri': uri, 'response': r}
             if r.ok:
-                data = r.json()
+                data = utils.get_response_json(r)
                 if 'Members' in data and len(data['Members']):
                     uri = data['Members'][0]['@odata.id']
                     r = sut.session.get(sut.rhost + uri)
@@ -111,14 +111,14 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
         r = sut.session.get(sut.rhost + uri)
         yield {'uri': uri, 'response': r}
         if r.ok:
-            data = r.json()
+            data = utils.get_response_json(r)
             if 'Members' in data and len(data['Members']):
                 for m in data['Members']:
                     uri = m['@odata.id']
                     r = sut.session.get(sut.rhost + uri)
                     yield {'uri': uri, 'response': r}
                     if r.ok:
-                        d = r.json()
+                        d = utils.get_response_json(r)
                         set_mfr_model_fw(sut, d)
                         set_mgr_net_proto_uri(sut, d)
                         for c in find_certificates(sut, d):
@@ -130,7 +130,7 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
         r = sut.session.get(sut.rhost + uri)
         yield {'uri': uri, 'response': r}
         if r.ok:
-            data = r.json()
+            data = utils.get_response_json(r)
             if 'PrivilegeMap' in data:
                 uri = data['PrivilegeMap']['@odata.id']
                 sut.set_nav_prop_uri('PrivilegeMap', uri)
@@ -141,7 +141,7 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
                     yield {'uri': uri, 'response': r}
                     sut.set_nav_prop_uri(prop, uri)
                     if r.ok:
-                        d = r.json()
+                        d = utils.get_response_json(r)
                         if 'Members' in d and len(d['Members']):
                             if prop == 'Accounts':
                                 resource_type = ResourceType.MANAGER_ACCOUNT
@@ -152,8 +152,8 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
                                     yield {'uri': uri, 'response': r,
                                            'resource_type': resource_type}
                                     if r.ok:
-                                        sut.add_user(r.json())
-                                        if (r.json().get('UserName')
+                                        sut.add_user(utils.get_response_json(r))
+                                        if (utils.get_response_json(r).get('UserName')
                                                 == sut.username):
                                             break
 
@@ -166,20 +166,20 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
                                     yield {'uri': uri, 'response': r,
                                            'resource_type': resource_type}
                                     if r.ok:
-                                        sut.add_role(r.json())
+                                        sut.add_role(utils.get_response_json(r))
 
     if 'SessionService' in root:
         uri = root['SessionService']['@odata.id']
         r = sut.session.get(sut.rhost + uri)
         yield {'uri': uri, 'response': r}
         if r.ok:
-            data = r.json()
+            data = utils.get_response_json(r)
             if 'Sessions' in data:
                 uri = data['Sessions']['@odata.id']
                 r = sut.session.get(sut.rhost + uri)
                 yield {'uri': uri, 'response': r}
                 if r.ok:
-                    data = r.json()
+                    data = utils.get_response_json(r)
                     if 'Members' in data and len(data['Members']):
                         uri = data['Members'][0]['@odata.id']
                         r = sut.session.get(sut.rhost + uri)
@@ -191,7 +191,7 @@ def get_default_resources(sut: SystemUnderTest, uri='/redfish/v1/',
         r = sut.session.get(sut.rhost + uri)
         yield {'uri': uri, 'response': r}
         if r.ok:
-            data = r.json()
+            data = utils.get_response_json(r)
             if 'Subscriptions' in data:
                 sut.set_nav_prop_uri(
                     'Subscriptions', data['Subscriptions']['@odata.id'])
@@ -290,7 +290,7 @@ def data_modification_requests(sut: SystemUnderTest):
             sut.add_response(new_uri, response)
             if response.ok:
                 etag = utils.get_response_etag(response)
-                data = response.json()
+                data = utils.get_response_json(response)
                 if 'PasswordChangeRequired' in data:
                     acct.password_change_required(sut, sut.session, new_user,
                                                   new_pwd, new_uri, data, etag)

redfish_protocol_validator/security_details.py

@@ -76,7 +76,7 @@ def test_default_cert_replacement(sut: SystemUnderTest):
     if sut.certificate_service_uri is not None:
         response = sut.get_response('GET', sut.certificate_service_uri)
         if response.ok:
-            data = response.json()
+            data = utils.get_response_json(response)
             if ('Actions' in data and '#CertificateService.ReplaceCertificate'
                     in data['Actions']):
                 sut.log(Result.PASS, 'GET', response.status_code,
@@ -450,14 +450,14 @@ def test_sessions_uri_location(sut: SystemUnderTest):
     sessions_uri1, sessions_uri2 = None, None
     response1 = sut.get_response('GET', '/redfish/v1/')
     if response1.ok:
-        data = response1.json()
+        data = utils.get_response_json(response1)
         if 'Links' in data and 'Sessions' in data['Links']:
             sessions_uri1 = data['Links']['Sessions']['@odata.id']
         if 'SessionService' in data:
             uri = data['SessionService']['@odata.id']
             response2 = sut.get_response('GET', uri)
             if response2.ok:
-                data = response2.json()
+                data = utils.get_response_json(response2)
                 if 'Sessions' in data:
                     sessions_uri2 = data['Sessions']['@odata.id']
         if sessions_uri1 and sessions_uri2:
@@ -496,7 +496,7 @@ def test_session_post_response(sut: SystemUnderTest):
                         sut.sessions_uri, Assertion.SEC_SESSION_POST_RESPONSE,
                         msg)
         if response.status_code == requests.codes.CREATED:
-            data = response.json()
+            data = utils.get_response_json(response)
             missing_props = []
             for prop in ['@odata.id', '@odata.type', 'Id', 'Name', 'UserName']:
                 if prop not in data:
@@ -717,7 +717,7 @@ def test_password_change_required(sut: SystemUnderTest):
                 Assertion.SEC_PWD_CHANGE_REQ_ALLOW_SESSION_LOGIN, msg)
     else:
         if response.ok:
-            data = response.json()
+            data = utils.get_response_json(response)
             keys = utils.get_extended_info_message_keys(data)
             if 'PasswordChangeRequired' in keys:
                 sut.log(Result.PASS, 'POST', response.status_code,
@@ -775,7 +775,7 @@ def test_password_change_required(sut: SystemUnderTest):
                 Assertion.SEC_PWD_CHANGE_REQ_DISALLOW_ALL_OTHERS, msg)
     else:
         if response.status_code == requests.codes.FORBIDDEN:
-            data = response.json()
+            data = utils.get_response_json(response)
             keys = utils.get_extended_info_message_keys(data)
             if 'PasswordChangeRequired' in keys:
                 sut.log(Result.PASS, 'GET', response.status_code,
@@ -808,8 +808,8 @@ def test_password_change_required(sut: SystemUnderTest):
             sut.log(Result.NOT_TESTED, 'PATCH', '', account_uri,
                     Assertion.SEC_PWD_CHANGE_REQ_ALLOW_PATCH_PASSWORD, msg)
         else:
-            if response.ok:
-                data = response.json()
+            if response.status_code == requests.codes.OK:
+                data = utils.get_response_json(response)
                 if ('PasswordChangeRequired' in data and
                         data['PasswordChangeRequired'] is False):
                     sut.log(Result.PASS, 'PATCH', response.status_code,
@@ -825,6 +825,13 @@ def test_password_change_required(sut: SystemUnderTest):
                             account_uri,
                             Assertion.SEC_PWD_CHANGE_REQ_ALLOW_PATCH_PASSWORD,
                             msg)
+            elif response.ok:
+                # No response body; treat as a pass; may want to consider
+                # adding a follow-up GET to perform an additional check
+                sut.log(Result.PASS, 'PATCH', response.status_code,
+                        account_uri,
+                        Assertion.SEC_PWD_CHANGE_REQ_ALLOW_PATCH_PASSWORD,
+                        'Test passed')
             else:
                 msg = ('Password change to %s failed with status %s; '
                        'expected it to succeed' %
@@ -845,7 +852,7 @@ def test_priv_one_role_per_user(sut: SystemUnderTest):
     for uri, response in sut.get_responses_by_method(
             'GET', resource_type=ResourceType.MANAGER_ACCOUNT).items():
         if response.ok:
-            data = response.json()
+            data = utils.get_response_json(response)
             role = data.get('RoleId')
             if role is not None:
                 sut.log(Result.PASS, 'GET', response.status_code, uri,
@@ -877,7 +884,7 @@ def test_priv_support_predefined_roles(sut: SystemUnderTest):
     for uri, response in sut.get_responses_by_method(
             'GET', resource_type=ResourceType.ROLE).items():
         if response.ok:
-            data = response.json()
+            data = utils.get_response_json(response)
             role = data.get('Id')
             if role in role_map:
                 role_map[role]['found'] = True
@@ -911,7 +918,7 @@ def test_priv_predefined_roles_not_modifiable(sut: SystemUnderTest):
     for uri, response in sut.get_responses_by_method(
             'GET', resource_type=ResourceType.ROLE).items():
         if response.ok:
-            data = response.json()
+            data = utils.get_response_json(response)
             if data.get('Id') == role:
                 read_only_found = True
                 break
@@ -959,7 +966,7 @@ def test_priv_roles_assigned_at_account_create(sut: SystemUnderTest):
     for uri, response in sut.get_responses_by_method(
             'GET', resource_type=ResourceType.MANAGER_ACCOUNT).items():
         if response.ok:
-            data = response.json()
+            data = utils.get_response_json(response)
             username = data.get('UserName', '')
             if username.startswith('rfpv'):
                 role = data.get('RoleId')

redfish_protocol_validator/service_details.py

@@ -119,7 +119,7 @@ def pre_ssdp(sut: SystemUnderTest):
         r = sut.session.get(sut.rhost + sut.mgr_net_proto_uri)
         if r.ok:
             sut.add_response(sut.mgr_net_proto_uri, r)
-            d = r.json()
+            d = utils.get_response_json(r)
             if 'SSDP' in d and 'ProtocolEnabled' in d['SSDP']:
                 enabled = d['SSDP']['ProtocolEnabled']
                 sut.set_ssdp_enabled(enabled)
@@ -532,7 +532,7 @@ def test_sse_unsuccessful_response(sut: SystemUnderTest):
                 failed = True
             else:
                 errors = ''
-                data = response.json()
+                data = utils.get_response_json(response)
                 if 'error' in data and isinstance(data['error'], dict):
                     if 'code' not in data['error']:
                         errors += (' Property "code" missing from "error" '
@@ -737,7 +737,7 @@ def test_sse_event_dest_context_opaque_str(sut: SystemUnderTest, event_dest):
 
     r = sut.session.get(sut.rhost + event_dest)
     if r.status_code == requests.codes.OK:
-        data = r.json()
+        data = utils.get_response_json(r)
         if 'Context' in data:
             context = data.get('Context')
             if isinstance(context, str):