fix: Revert fields using x-http-method-override

[owodunni]

X-HTTP-Method-Override breaks the relationship and upload field on AWS cloudfront. As well as in any network stack that filters X-HTTP-Method-Override headers. This commit addresses the discussion: #7248

X-HTTP-Method-Override was added as part of PR#6487. The X-HTTP-Method-Override can be security sensitive in some cases therefore some networks stacks remove the header. In my case the header is removed before the payload service ingress (Nginx) is called.

This commit reverts part of #6487, that issue might need to be opened gain if this is merged. I find it hard to understand under what scenarios the relations field and upload filed would break due to too many relations. Maybe there is a different workaround that doesn't resort to X-HTTP-Method-Override?

Description

• [x] I have read and understand the CONTRIBUTING.md document in this repository.

Type of change

• [ ] Chore (non-breaking change which does not add functionality)
• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Change to the templates directory (does not affect core functionality)
• [ ] Change to the examples directory (does not affect core functionality)
• [ ] This change requires a documentation update

Checklist:

I didn't add a test, instead I removed the old test. I figured a discussion was in order before I put more time into this.

• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] Existing test suite passes locally with my changes
• [ ] I have made corresponding changes to the documentation

[andershermansen]

If this is merged it will reopen issue #6486 The test is there for that reason.

If anything there should be a length check on the query so it uses regular GET for short query strings and the X-HTTP-Method-Override header on longer queries when it's needed.

But that will cover up the compatibility issue with proxies which do not forward the X-HTTP-Method-Override header and you will get in to same trouble as described in #6486 when you have a lot of relationships.

[owodunni]

@andershermansen I started looking at adding a utility function for switching between GET and POST. I didn't know where to put it. Could you give me pointers?

Also max length URL should be 2083 characters if I googled correct. https://saturncloud.io/blog/what-is-the-maximum-length-of-a-url-in-different-browsers/

[BarrySaikSoundry]

One way to do it would be to just use a normal request when there are less than 20 relationships. (Or pick a relatively low number). I think most people have lower numbers of relations and 10 seems like a safe number to use where it’s unlikely to break the url length. Then for more than 20 use the X-HTTP-Method-Override.

What is the use case for needing 100’s of relationships?

[owodunni]

@andershermansen @BarrySaikSoundry now I have added a utility function that checks if the url is bellow 2083 characters and changes the method used depending on that. For my needs that is good enough. I reverted my removal of the tests.

Is there anything else you need me to add?

[paulpopus]

My concern here is that this doesn't solve the underlying issue, which is that the headers get stripped by your AWS Cloudfront configuration.

This feature is used by the new hasMany option on uploads as well.

Would this approach not be suitable? https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/distribution-web-values-specify.html#DownloadDistValuesForwardHeaders

[owodunni]

@paulpopus in my case I'm not using AWS I'm deploying behind a corporate network firewall/load balancer that I have no control over. I don't think I'm the only one.

I think its good if 'X-HTTP-Method-Override' is not used by default to make it easier to deploy in different environments.

[andershermansen]

An alternative might be to actually introduce a new endpoint that you can POST to for find.

GET to /api/{collection-slug} is connected to find POST to /api/{collection-slug} is connected to new POST to /api/{collection-slug} with X-HTTP-Method-Override set to GET is connected to find

Instead of using the header to "rewire" the POST to the find method we can use a new endpoint instead

POST to /api/{collection-slug}/find connected to find

This will be a breaking change if anybody has created a custom endpoint with name find and method POST

@owodunni Note that I am not a payload developer and do not make these decision. Just giving my input and suggestions.

[smhdale]

I've also come across issues with this header when deploying Payload 3 beta to AWS Lambda with OpenNext - API Gateway seems to "honour" this header by actually modifying the HTTP method passed to the Lambda function. A POST request with the X-HTTP-Method-Override header set to GET causes Lambda to see a GET request instead, which in turn breaks how Payload interprets the request.

I was able to work around this by renaming the header to something non-standard in a CloudFront function (I used X-Payload-Method-Override), then renaming it back in a NextJS middleware. Maybe the ability to configure or just fully disable this behaviour in the Payload config file would be handy?

[marvinengelmann]

Just want to add, you cannot use the Arcjet shield with this header because it triggers suspicious activity.

[github-actions[bot]]

This PR is stale due to lack of activity.

To keep the PR open, please indicate that it is still relevant in a comment below.

[andershermansen]

This is maybe not needed anymore as #12571 has done a proper fix.