Knotx / Knotx/knotx

Refactor handling header filets for HTTP repository

Open
#401 0 comments 0 reactions 0 assignees View on GitHub
discussion
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.