Refactor handling header filets for HTTP repository
- Dominant language
- No language data
- Stars
- 126
- Forks
- 26
- PR merge metrics
- No merged PRs in 30d
Description
### Version used
* knot.x: since 1.2.1
### Context of the issue/feature
Basically, a HTTP Repository verticle has configuration `allowedRequestHeaders` where a list of headers need to be defined. These headers are to be passed to HTTP repository.
If not set properly, a POST method (that fetches the template from the repository too) might fail with a strange error - it's because the Knot.x fetches the Template using GET method and uses headers from initial request.
Header handling on HTTP repository connector need to be rethinked/refactored. Few thoughts:
- Call to repo should always be GET method
- We shouldn't do any header filtering instead pass through all of them - it will work only for GET requests
- In case of other request methods, such as POST, we should carefully prepare request to the repo. It means, the repo connector should have coded rules saying what headers are valid for GET methods, and get from the incoming request only the one that makes sense.
E.g based on https://en.wikipedia.org/wiki/List_of_HTTP_header_fields#Request_fields, when doing GET to repo, get rid following headers:
- Content-Type
- X-Http-Method-Override
- Content-Length
- Content-MD5
- Other option would be to create `denyRequestHeaders` parameter instead of **allowed** so the configuration will be more seamless - just block unnecessary headers,
- Or even implement a SPI that allows to create a repo customization (project specific) how headers should be handled - possibly to allow to inject my own custom headers, replace incomming headers, etc.
### Bug reproducer
N/A/
### Steps to reproduce
1. Configure `allowedRequestHeaders=["*"]`
2. Open 'http://localhost:8092/content/remote/multiple-forms.html'
3. Post one and then second form
You will get the error in logs saying that connection is broken, possible HTTP/0.9 protocol.
## So the ticket is rather form of questionare, which option you prefer:
1. Get rid of `allowedRequestHeaders` and hardcode logic what headers should be removed
2. Change into `denyRequestHeaders` so all are allowed, and user might decided which one to ignore
3. Create HTTP Repo SPI that allows to deliver custom implementation how to handle headers
Contributor guide
Research direction
Start by reviewing the HTTP repository connector and its allowedRequestHeaders configuration, then reproduce the two-form request sequence described in the issue. Compare the three proposed header-handling options and confirm the connection error behavior. Done requires an agreed approach and corresponding connector behavior, but the issue does not name files or tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api, backend, backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100