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

Interoperability issues with reverse proxy (Transfer-Encoding: identity) #1909

Closed
pflorczyk opened this issue Nov 29, 2024 · 3 comments · Fixed by #1919
Closed

Interoperability issues with reverse proxy (Transfer-Encoding: identity) #1909

pflorczyk opened this issue Nov 29, 2024 · 3 comments · Fixed by #1919

Comments

@pflorczyk
Copy link

Recently we switched our reverse proxy servers to haproxy. Our users started reporting that they see increase in http 502 responses. After a bit of digging I was able to narrow it down to this on haproxy side:

<0>2024-11-29T15:51:19.584121+01:00 [12|h1|0|mux_h1.c:1900] Unknown transfer-encoding : [B,RUN] [MSG_DONE, MSG_DONE]
<0>2024-11-29T15:51:19.584132+01:00 [12|h1|0|mux_h1.c:2133] parsing or not-implemented error : [B,RUN] [MSG_DONE, MSG_DONE]

This happens when client uses HEAD method, and there is no content-length set. fasthttp inserts Transfer-Encoding: identity into responses. I think the problem lies in ReadBody() function.

It looks like "identity" has been deprecated for quite a long time (2008?):
https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.1
https://datatracker.ietf.org/doc/html/rfc9112#name-transfer-encoding

And here is related issue on haproxy side (they will not revert that change): haproxy/haproxy#2623

@erikdubbelboer
Copy link
Collaborator

I would be open to a pull request that stops fasthttp from returning an Identity encoding.

@ksw2000
Copy link
Contributor

ksw2000 commented Dec 4, 2024

@pflorczyk Thank you for reporting this issue. To help investigate and address the problem more efficiently, could you please provide a minimal, reproducible code example?

@pflorczyk
Copy link
Author

pflorczyk commented Dec 9, 2024

Sorry for late response.

package main

import (
	"fmt"
	"log"

	"github.com/valyala/fasthttp"
)

func proxyHandler(ctx *fasthttp.RequestCtx) {
	targetURL := "http://httpbin.org/stream/0"

	req := fasthttp.AcquireRequest()
	defer fasthttp.ReleaseRequest(req)

	req.Header.SetMethod(string(ctx.Method()))
	req.SetRequestURI(targetURL)
	req.Header.Set("User-Agent", string(ctx.Request.Header.UserAgent()))

	resp := fasthttp.AcquireResponse()
	defer fasthttp.ReleaseResponse(resp)

	err := fasthttp.Do(req, resp)
        if err != nil {
            ctx.Error("Failed to proxy request", fasthttp.StatusInternalServerError)
            return
        }
	fmt.Println("Response Headers from target server:")
	resp.Header.VisitAll(func(key, value []byte) {
                ctx.Response.Header.Set(string(key), string(value))
		fmt.Printf("%s: %s\n", key, value)
	})
	ctx.Write(resp.Body())
}

func main() {
	if err := fasthttp.ListenAndServe(":8080", proxyHandler); err != nil {
		log.Fatalf("Error in ListenAndServe: %s", err)
	}
}

output from curl (headers copied from resp to ctx):

curl -I http://127.0.0.1:8080      
HTTP/1.1 200 OK
Server: gunicorn/19.9.0
Date: Mon, 09 Dec 2024 16:42:57 GMT
Content-Type: application/json
Connection: keep-alive
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Connection: close

output from console (headers from resp):

Response Headers from target server:
Content-Type: application/json
Server: gunicorn/19.9.0
Date: Mon, 09 Dec 2024 16:42:57 GMT
Connection: keep-alive
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Transfer-Encoding: identity
Connection: close

This is my example code, not the one that triggers this issue. I guess the other team returns resp directly (to avoid copy).
Anyway I find it strange that Transfer-Encoding: identity is injected into resp but then stripped from ctx.

ksw2000 added a commit to ksw2000/fasthttp that referenced this issue Dec 13, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
erikdubbelboer pushed a commit that referenced this issue Dec 15, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants