luraproject / lura

Ultra performant API Gateway with middlewares. A project hosted at The Linux Foundation
https://luraproject.org
Other
6.35k stars 564 forks source link

Content-Length is not set for backend requests if configured with lowercase typing #510

Closed Yengas closed 3 years ago

Yengas commented 3 years ago

Describe the bug Some backend systems require Content-Length header to be set on proxied requests to process request body. To fix this, we can configure the KrakenD endpoints with headers_to_pass property and make KrakenD send Content-Length to the backend.

However, if the headers_to_pass is configured as headers_to_pass: ["content-length", "content-type"] the proxy code makes a case-sensitive check against the Content-Length header and can not configure the request properly. This behaviour is not documented anywhere and causes a confusion.

To Reproduce

  1. Have a backend that requires "Content-Length" header to be present for request body to be processed
  2. Configure an endpoint with headers_to_pass: ["content-length", "content-type"] and make sure you are making a request to the backend directly, without any load-balancer/reverse-proxy
  3. Observe that requests fail
  4. Change the configuration to headers_to_pass: ["Content-Length", "Content-Type"]
  5. Observe that the requests succeed

Expected behavior I would expect the request to be made with correct configurations.

Additional context If needed I can invest some time into create a repository for reproducing the bug. I locally debugged the KrakenD source code and am suspecting this line of code in the proxy code.

taik0 commented 3 years ago

Hi @Yengas It's possible that you are using a plugin in this tests?

My test: Config:

{
    "version": 2,
    "port": 8080,
    "endpoints": [{
            "endpoint": "/foo",
            "headers_to_pass": [  "content-length", "content-type" ],
            "backend": [ {
                "url_pattern": "/__debug/"
            } ]
        }]
}

curl request:

curl -i -H"content-type: application/json" http://localhost:8080/foo
HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8
X-Krakend: Version 1.4.1
X-Krakend-Completed: true
Date: Thu, 15 Jul 2021 08:37:00 GMT
Content-Length: 18

{"message":"pong"}

KrakenD logs:

2021/07/15 10:37:00  DEBUG: [Method: GET]
2021/07/15 10:37:00  DEBUG: [URL: /__debug/]
2021/07/15 10:37:00  DEBUG: [Query: map[]]
2021/07/15 10:37:00  DEBUG: [Params: [{param /}]]
2021/07/15 10:37:00  DEBUG: [Headers: map[Accept-Encoding:[gzip] Content-Type:[application/json] User-Agent:[KrakenD Version 1.4.1] X-Forwarded-For:[::1] X-Forwarded-Host:[localhost:8080]]]
2021/07/15 10:37:00  DEBUG: [Body: ]
[GIN] 2021/07/15 - 10:37:00 | 200 |       368.1µs |             ::1 | GET      "/__debug/"
[GIN] 2021/07/15 - 10:37:00 | 200 |    2.434902ms |             ::1 | GET      "/foo"

As you can see, the request header is not in lowercase anymore.

Lura takes http.Request and transform them into proxy.Request, in this step the headers are normalized using CanonicalMIMEHeaderKey. https://github.com/luraproject/lura/blob/master/router/gin/endpoint.go#L102-L112

Yengas commented 3 years ago

Hey @taik0, thanks for the really fast response. In your logs I see only the response header of Content-Length from the debug endpoint. But I am actually talking about the "request" made to the backend itself.

In the KrakenD logs you've sent, the backend does not seem to receive the Content-Length header. For some backends, this causes the backend to not read the request body. So the request is seen as a POST/PUT request without a JSON body and usually ends up with Bad request.

P.S. I've made sure to disable all plugins when I was debugging this. The configuration was just a single endpoint without any extra plugin/extra config.

Yengas commented 3 years ago

@taik0 Also regarding the code you've sent. There we have the request that was made to the KrakenD instance, and the proxy request that will be sent to the backend. That "if block" checks the request made to the KrakenD instance to make sure the header is present there, and if present, sets the proxy request header.

However proxy request header key is set to whatever the headersToSend slice contains. So the proxy request headers may be in fact lower case.

taik0 commented 3 years ago

In your logs I see only the response header of Content-Length from the debug endpoint. But I am actually talking about the "request" made to the backend itself. The debug endpoint is a valid backend. You are right, the backend doesn't receive the Content-Length header if it's not canonical. Because the map used in the proxy.Request doesn't have canonical headers and later checks in the pipeline (link this one) expects the key to be canonical.

I will make a patch for this, normalize the header map to avoid mistakes in the configuration and problems checking headers in plugins. In the mean time, I recommend to always set the header names in the configuration already in canonical format.

taik0 commented 3 years ago

Fixed in #512

Yengas commented 3 years ago

I've pulled the latest master branch and verified that my configuration currently works as expected as well. Thank you for your quick response and fix. Kudos 🙏

github-actions[bot] commented 2 years ago

This issue was marked as resolved a long time ago and now has been automatically locked as there has not been any recent activity after it. You can still open a new issue and reference this link.