Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpApi: wrong capitalisation of properties in the headers object - not asking for headers to be case sensitive #3083

Closed
mastazi opened this issue Jul 20, 2021 · 11 comments
Labels

Comments

@mastazi
Copy link

mastazi commented Jul 20, 2021

Description:

Edit 25/07/2021 I am not suggesting that headers be case sensitive, I understand this can't be done, I'm asking for the headers object properties to be lowercase so it matches API Gateway

I have this function:

fakeFunction:
    Type: AWS::Serverless::Function
    Properties:
      Handler: src/handlers/handlers.fake
      Description: A Lambda function.
      Events:
        Api:
          Type: HttpApi
          Properties:
            Path: /my-path/
            Method: get

if I send a header called “Myheader” in the request, then when running sam local start-api, the function will be able to access its value in event.headers.Myheader - note that Myheader is capitalised. But after deployment, API Gateway will store the value in event.headers.myheader. So, in order to have my code work both in SAM and in the cloud, I have to do something like (assuming the function is in Node.js):

let myvar = '';
if (process.env.AWS_SAM_LOCAL) {
    myvar  = event.headers.Myheader;
} else {
    myvar = event.headers.myheader;
}

Steps to reproduce:

  1. Create a function with an HttpApi Path.
  2. In the response, return event.headers, see code snippet below.
  3. Build and then start local api with sam local start-api
  4. Call the function from an http client, and send some arbitrarily named headers with the request, e.g. Foo: bar, Baz: bar.
    const response = {
        statusCode: 200,
        body: JSON.stringify(event.headers),
    };
    return response;

Observed result:

In the response, you will see those headers will be capitalised:

{
  "Foo": "bar",
  "Baz": "bar"
}

Expected result:

The expected result is that the headers should be not capitalised, which is the same behaviour that AWS Api Gateway has in production:

{
  "foo": "bar",
  "baz": "bar"
}

Additional environment details (Ex: Windows, Mac, Amazon Linux etc)

  1. OS: Windows 10
  2. sam --version: SAM CLI, version 1.23.0
  3. AWS region: ap-southeast-2
@FieryCod
Copy link

This is true for Java rapid runtime as well, which makes the development pain, unfortunately.

@jfuss
Copy link
Contributor

jfuss commented Jul 22, 2021

Please see previous issue about this: #1860 (comment)

This is known difference and something we are not looking to address. Headers are case insensitive and should be handled in the function code. This is also being caused by Flask (what we are using locally to build the service). #1860 (comment) has more info and links to other issues explain it deeper.

Closing as won't fix.

@jfuss jfuss closed this as completed Jul 22, 2021
@jfuss jfuss added the wontfix label Jul 22, 2021
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues 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.

@mastazi
Copy link
Author

mastazi commented Jul 25, 2021

Hi @jfuss thank you for your comment and sorry for inadvertently opening a duplicate (I had searched for it but couldn't find #1860 the other day).

Just one note though:

here you said:

Do note that headers are case-insensitive so handling this in your function code is a reasonable expectation.

I have to disagree on this one, if the SAM docs tell me that headers are case insensitive, my initial instinct when reading that, is that I'm not going to worry about normalising capitalisation (which is what I ended up doing), because "headers are case-insensitive" sounds like the simulator and the production env would normalise headers to a consistent case, which is not what's happening (SAM headers are always uppercase, API Gateway headers are always lowercase).

So, if this inconsistency is not easily fixable, I think the documentation should be updated to let users know that while headers are case-insensitive, they have different case in SAM compared to API Gateway. Is it possible to file an issue related to the AWS SAM docs? Do you know if there is a repo for that? (Edit: there is one it is here https://github.com/awsdocs/aws-sam-developer-guide)

@mastazi
Copy link
Author

mastazi commented Jul 25, 2021

One more thing @jfuss I want to clarify: reading your comments in #1860 it seems that you are under the impression that users were asking for the headers dictionary to be case-sensitive. This is not what people are asking.

You see, API Gateway also makes headers case-insensitive.

In API Gateway, both raw headers Myheader and myheader will result in headers.myheader. They are always included in the dictionary as lowercase properties.

But in SAM, both of those raw headers would result in headers.Myheader.

What people are asking is for the dictionary to have lowercase properties, so it matches API Gateway.

We are not asking for the headers to be case sensitive.

The fix would just be to take the headers object and make all properties lowercase e.g. in Python

new_headers = dict((k.lower(), v) for k, v in old_headers.items())

@mastazi mastazi changed the title HttpApi: wrong capitalisation of properties in the headers object HttpApi: wrong capitalisation of properties in the headers object - not asking for headers to be case sensitive Jul 25, 2021
@mastazi
Copy link
Author

mastazi commented Jul 25, 2021

Update: submitted PR #3114

@FieryCod
Copy link

This will hardly ever be accepted do to the breakage it brings to the users.

I'm really sorry your situation, cause I was in the same. You would have to lowercase all the headers from event in your code unfortunately.

@mastazi
Copy link
Author

mastazi commented Jul 25, 2021

@FieryCod Yes it will bring disruption to some users, however I expect most users are jus converting to lowercase and in that instance, nothing will break. The only instance in which it will break is if users are using if-else check, and retrieve capitalised headers if they are in localhost.

@mastazi
Copy link
Author

mastazi commented Jul 26, 2021

updated: closed previous pr and reopened as #3117 due to previous PR having issues related to my linter.

@jfuss
Copy link
Contributor

jfuss commented Jul 27, 2021

@mastazi We cannot just lower() all headers and API Gateway does not just lower all headers. See: #1860 (comment), the concrete example is Authorization which remains that same casing.

API Gateway does not document how they determine casing, so whatever we do is going to be heuristics. Headers are case-insensitive and therefore you cannot guarantee how API Gateway will process them.

As @FieryCod mentioned, this change will break customers. A function that runs today, will break existing customers. We are willing to accept changes like this but they will need to match API Gateway. Just lowering all headers doesn't match API Gateway and will break customers. If your willing to go down a huge testing flow and add all the heuristics to make this SAM match API Gateway, we can look at that. However, we have done this in the past, and Flask does not have a way to get the raw requests. More details: #1860 (comment)

I will comment the same on the PR but currently, I don't think we can accept just .lower() on all headers.

@mastazi
Copy link
Author

mastazi commented Jul 29, 2021

Thank you @jfuss I missed that comment about Authorization, now I understand what you meant in your comment in #1860 where you said that API Gateway does not document how they do it.

Thanks for correcting my wrong assumption, closing #3117

If your willing to go down a huge testing flow and add all the heuristics to make this SAM match API Gateway, we can look at that.

I'm still getting used to the codebase, if I get to a point where I feel ready for that, I will try.

However, we have done this in the past, and Flask does not have a way to get the raw requests.

Yes, I see what you mean, however there is a (remote) possibility that the rules that API Gateway is using, could be re-implemented without accessing raw headers, however we don't know that yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants