Feature: Request header keys should case-insensitive

[alnemo]

Expected Behaviour

By standard HTTP header keys should not be case sensitive, and in many cases get always converted to lowercase automatically.
For example see Requests library, it defines a case-insensitive dictionary to hold the headers:
from requests.structures import CaseInsensitiveDict
Likewise Chalice uses the same structure for the headers in the request.

Current Behaviour

When using AWS Lambda Powertools the request headers are coming in a standard case-sensitive Python dictionary.
As the result of that retrieving the header by key is case-sensitive.

Code snippet

from aws_lambda_powertools.event_handler import APIGatewayRestResolver, Response
from dataclasses import dataclass
import json

app = APIGatewayRestResolver()
@app.route('/test', method=['GET'])
def test():
  return Response(
        body=json.dumps({
            "Result": app.current_event.headers.get('testheader', 'Headers are case-sensitive')
        }),
        status_code=200
    )

def lambda_handler(event: dict, context) -> dict:
    response =  app.resolve(event, context)
    return response

def lambda_context():
    @dataclass
    class LambdaContext:
        function_name: str = "testfunction"
        memory_limit_in_mb: int = 128
        invoked_function_arn: str = "arn:aws:lambda:us-east-1:123456789012:function:testfunction"
        aws_request_id: str = "d195721c-d4d8-4b10-9c4c-ef4768865b92"

    return LambdaContext()

test_event = {
            "path": "/test",
            "httpMethod": "GET",
            "requestContext": {"requestId": "d195721c-d4d8-4b10-9c4c-ef4768865b92"},  
            "headers": {'testHeader':'Headers are case-insensitive'}
}
print(json.dumps(lambda_handler(test_event, lambda_context())))

Possible Solution

No response

Steps to Reproduce

In lambda code attempt to retrieve request header capitalized differently than it is on the incoming request.

Powertools for AWS Lambda (Python) version

latest

AWS Lambda function runtime

3.11

Packaging format used

Lambda Layers

Debugging logs

No response

[boring-cyborg]

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

[leandrodamascena]

Looking at this now.

[leandrodamascena]

Hello @alnemo! Thank you for opening this issue. Yes, you are right, HTTP header names are case-insensitive, according to RFC 2616.

We convert the APIGateway event into a BaseProxyEvent and we get the headers of the event and we return it to the customer, we do not convert it to lowercase, which generates this bug when using version Payload format version v1.0.

Do you want to send a PR to fix this? It would be amazing to have your contribution here. Let me know if you want to send this PR or I can send it 🚀

[aradyaron]

Hello @alnemo! Thank you for opening this issue. Yes, you are right, HTTP header names are case-insensitive, according to RFC 2616.

We convert the APIGateway event into a BaseProxyEvent and we get the headers of the event and we return it to the customer, we do not convert it to lowercase, which generates this bug when using version Payload format version v1.0.

Do you want to send a PR to fix this? It would be amazing to have your contribution here. Let me know if you want to send this PR or I can send it 🚀

Seems like the requests library implemented a case insensitive dict for these cases
https://github.com/psf/requests/blob/ee93fac6b2f715151f1aa9a1a06ddba9f7dcc59a/src/requests/structures.py#L13

[leandrodamascena]

I updated the title and labels because this isn't actually a bug, it's more of a feature request.

[pgrzesik]

Hey @leandrodamascena - do I understand correctly that the expected behavior here would be to always lowercase keys of BaseProxyEvent.headers? Wouldn't it be a potential breaking change if users already depend on the uppercased version or am I missing something here? I feel like we should perform case-insensitive lookup there

If the author won't be interested in submitting a PR, I'd love to take a stab at it 🙌

[heitorlessa]

I feel like we should perform case-insensitive lookup there

@pgrzesik that's correct ^. Later we can decide in an upcoming version whether we want to make the breaking change for API Gateway Rest API payload (only one that uses non-lower case).

If we don't hear from @alnemo by Monday, please Piotr, it'd be awesome to have a PR from you on this so we can include in the next release already.

[aradyaron]

Hey guys, if needed I’ll gladly help to review or help developing it further

[heitorlessa]

Certainly appreciate the review @aradyaron :)

Future reference: I'm working (as bandwidth permits) on a revamped contributing guide, integrated into our docs. It'll feature a more detailed coding style guide for contributions, reviews, pathways, and before/after conventions that we follow to make this process easier for everyone. Including the biggest part on... how to write good documentation for various IT personas.

[leandrodamascena]

Hey @leandrodamascena - do I understand correctly that the expected behavior here would be to always lowercase keys of BaseProxyEvent.headers? Wouldn't it be a potential breaking change if users already depend on the uppercased version or am I missing something here? I feel like we should perform case-insensitive lookup there

If the author won't be interested in submitting a PR, I'd love to take a stab at it 🙌

Hi @pgrzesik! Yes, you're right! Forgot to update the issue after my last comment.

It will be amazing to merge your code into Powertools. Please let us know if you need any help.

[pgrzesik]

Hey 👋 As the original author didn't respond, I'll take a stab at implementing a PR for this 🙌 Just to make sure we're on the same page - do you feel like having something similar to Chalice's CaseInsensitiveMapping would be okay here (https://github.com/aws/chalice/blob/79838b02dc330cfe549823899d2662ded0538015/chalice/app.py#L203) for headers instead of regular dict?

[leandrodamascena]

This implementation makes sense to me. I Just want to see what the customer experience is like.
Can you create an example of how to use this?! You can do this here or when submitting the PR 😃.

Thank you for your prompt response.

[pgrzesik]

Hey @leandrodamascena - thanks for the question about the customer experience. I looked into the implementation deeper and I think maybe the best way to improve the current situation is not to change the underlying implementation of BaseProxyEvent.headers, but rather be more explicit in the documentation that headers accessed via event.headers will be case-sensitive? There's already a method BaseProxyEvent.get_header_value, which by default does case-insensitive lookup, however, that fact is not mentioned in the documentation here: https://docs.powertools.aws.dev/lambda/python/latest/core/event_handler/api_gateway/#headers

Using a case-insensitive mapping to hold headers is also a potential solution, but it's one that has the potential to be a breaking change if some users actually depend on headers being a dict in their current implementations. Maybe it's better suited for a release that can include such a change. What do you think?

PS. Sorry for back and forth on this seemingly simple issue, but I want to make sure I won't break anything for existing users 🙌

[leandrodamascena]

Hey @leandrodamascena - thanks for the question about the customer experience. I looked into the implementation deeper and I think maybe the best way to improve the current situation is not to change the underlying implementation of BaseProxyEvent.headers, but rather be more explicit in the documentation that headers accessed via event.headers will be case-sensitive? There's already a method BaseProxyEvent.get_header_value, which by default does case-insensitive lookup, however, that fact is not mentioned in the documentation here: docs.powertools.aws.dev/lambda/python/latest/core/event_handler/api_gateway/#headers

I agree! Thinking from this point of view, I think we can improve our documentation instead of changing the code. Do you have any ideas in mind to change and improve the documentation?

Maybe it's better suited for a release that can include such a change. What do you think?

You mean, add this breaking change in the next major release?

PS. Sorry for back and forth on this seemingly simple issue, but I want to make sure I won't break anything for existing users 🙌

No problem man. We love discussing the best solution with our customers.

[pgrzesik]

Hey @leandrodamascena, I've published a small PR that highlights the option for case-insensitive lookup in docs: https://github.com/aws-powertools/powertools-lambda-python/pull/3183/files

You mean, add this breaking change in the next major release?

Correct 👍 Or do you feel like the change there has a low risk of breaking users' existing workflows?

[leandrodamascena]

Hello @alnemo! We've updated our documentation to describe that using get header value is the best solution for this case.
Thanks a lot for reporting this. We appreciate when we can make our documentation even better.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.