Python code for checking request headers I am trying to validate a HTTP REST API request in my python code. It checks for some request headers (X-Foo and X-Bar) and two keys in the json body/payload (someKey and someNestedKey). I have written this code with if/else conditions as well as a simple try-catch. 

headers = event["headers"]
if "X-Foo" not in headers or "X-Bar" not in headers:
    return {
        'statusCode': 401,
        'body': '{"message":"Some error message"}'
    }

# Do something with request headers here
payload = json.loads(event["body"])

if "someKey" not in payload:
    some_flag = False
else:
    if "someNestedKey" not in payload["someKey"]:
        some_flag = False
    else:
        some_flag = True

if not some_flag:
    return {
        'statusCode': 400,
        'body': '{"message":"Some other error message"}'
    }

# if we get this far, then its ok
foo_bar = payload["someKey"]["someNestedKey"]


vs

try:
    headers = event["headers"]
    # Do something with request headers here
    payload = json.loads(event["body"])
    foo_bar = payload["someKey"]["someNestedKey"]
except KeyError:
    return {
            'statusCode': 401,
            'body': '{"message":"Some error message"}'
        }


I'd like a review on all aspects of this code.
I am not sure this question can still be salvaged as some relevant details are missing.

One opening remark though: HTTP header fields are case-insensitive, as stipulated by several RFCs, including RFC 7230 (See 3.2.  Header Fields).


  Each header field consists of a case-insensitive field name followed
  by a colon (":"), optional leading whitespace, the field value, and
  optional trailing whitespace.


If you are using the requests module to collect your headers with key values you would have an object of type requests.structures.CaseInsensitiveDict, and you don't have to take care of this aspect. Therefore, "X-Foo" in req.headers and "x-foo" in req.headers will both match the same.

But it is good to keep this in mind as some clients may take some liberty and decide on a different casing like all-lowercase - after all they are still RFC-compliant, and so should you.

This aspect is also discussed in the doc for the requests module: Response Headers 

So you are checking the existence of two headers but aren't you interested in their values ? What is the point of using those two headers ? Is the code slightly incomplete or did I miss something ?



Exception handling: your whole procedure should be covered by a comprehensive exception handling routine.
For specific tasks like decoding the JSON payload, watch out for JSONDecodeError exception. It will be triggered if an invalid JSON string is sent to your script (could be an attacker). See below.

An unhandled exception could in certain circumstances defeat or bypass the various checks you make. Bottom line: identify the sensitive parts of your code (in particular those processing external input), always handle exceptions, never ignore them.



The code can be simplified, for example:

if "someKey" not in payload:
    some_flag = False
else:
    if "someNestedKey" not in payload["someKey"]:
        some_flag = False
    else:
        some_flag = True

if not some_flag:
    return {
        'statusCode': 400,
        'body': '{"message":"Some other error message"}'
    }


If the goal is to check for the existence of a key in a JSON string and extract the corresponding value you could wrap the functionality in small functions to declutter the code.

In the example below I am using a simplistic JSON string like {"AuthToken": "abcdef", "UserID": 1234}. Since you've mentioned nested keys you'll have to adapt the code a little bit but you get the idea.

def parse_json(json_string):
    try:
        # return dict - maybe (see later)
        return json.loads(json_string)
    except json.JSONDecodeError:
        # stop immediately or alternatively return an empty dict
        print('JSONDecodeError triggered, do something')
        #logger.error('JSONDecodeError triggered', exc_info=True)
        #sys.exit(1)

def get_value_from_key(payload, key_to_check):
    if key_to_check in payload.keys():
        # key found, get the matching value
        return payload[key_to_check]
    else:
        return None


Testing the functions:

json_string = """{"AuthToken": "abcdef", "UserID": 1234}"""
payload = parse_json(json_string)

invalid_json_string = """{"AuthToken": "UserID": 1234}"""
# this will trigger an handled exception, and you should not proceed any further
invalid_payload = parse_json(invalid_json_string)

key_to_check = 'AuthToken'
# value will be None if the key was not found, then you could raise your 401 error or whatever
print(get_value_from_key(payload, key_to_check))


If you don't want to choke on the exception but proceed as casually as possible you could return an empty dict but at least you've handled the invalid input.

It seems to me that the way you are handling KeyError in your code will therefore not suffice to handle all the possible bad situations. The best is of course to test your script in adverse conditions, feed it garbage and see how it reacts.

NB: unlike the headers collection routine get_value_from_key is case-sensitive. So if I want to retain the ability of doing case-insensitive searching on the keys I would probably do this, reusing structures already available to us:

from requests.structures import CaseInsensitiveDict

if key_to_check in CaseInsensitiveDict(payload):
    ...


If your code is already using requests you are not adding extra dependencies

But wait. Actually, json.loads will not always return a dict, than depends.


  Deserialize s (a str, bytes or bytearray instance containing a JSON
  document) to a Python object using this conversion
  table.


Source: json — JSON encoder and decoder

So I would advise you to check that the return value is actually of a suitable type like dict or list. It's always important to read the doc so that there is  no ambiguity as to the data type returned by a given function, because there may be several possibilities.

The proposed code is for demonstration purposes but not comprehensive, hopefully it can help you avoid a few pitfalls.