fiber icon indicating copy to clipboard operation
fiber copied to clipboard

🤗 [Question]: Why delete "Connection header" at proxy.Do()?

Open HelloWorld0707 opened this issue 2 years ago • 3 comments

Question Description

Hello! Im using [email protected] and i found "HeaderConnection" is being deleted at doAction() in proxy.go

Let me know why. its have someting issue?

I need this header. To add reverse proxy handler for my server

Code Snippet (optional)

func doAction(
	c *fiber.Ctx,
	addr string,
	action func(cli *fasthttp.Client, req *fasthttp.Request, resp *fasthttp.Response) error,
	clients ...*fasthttp.Client,
) error {
	var cli *fasthttp.Client

	// set local or global client
	if len(clients) != 0 {
		cli = clients[0]
	} else {
		lock.RLock()
		cli = client
		lock.RUnlock()
	}

	req := c.Request()
	res := c.Response()
	originalURL := utils.CopyString(c.OriginalURL())
	defer req.SetRequestURI(originalURL)

	copiedURL := utils.CopyString(addr)
	req.SetRequestURI(copiedURL)
	// NOTE: if req.isTLS is true, SetRequestURI keeps the scheme as https.
	// Reference: https://github.com/gofiber/fiber/issues/1762
	if scheme := getScheme(utils.UnsafeBytes(copiedURL)); len(scheme) > 0 {
		req.URI().SetSchemeBytes(scheme)
	}

	req.Header.Del(fiber.HeaderConnection)
	if err := action(cli, req, res); err != nil {
		return err
	}
	res.Header.Del(fiber.HeaderConnection)
	return nil
}

Checklist:

  • [X] I agree to follow Fiber's Code of Conduct.
  • [X] I have checked for existing issues that describe my questions prior to opening this one.
  • [X] I understand that improperly formatted questions may be closed without explanation.

HelloWorld0707 avatar Nov 02 '23 09:11 HelloWorld0707

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

welcome[bot] avatar Nov 02 '23 09:11 welcome[bot]

@HelloWorld0707 Those headers are usually used for services that offer keepalive, is that your intent. We could add a config option to make this configurable.

gaby avatar Nov 04 '23 11:11 gaby

@HelloWorld0707 what do you think?

unfortunately we do not know the exact cause as it is a very old change and there is no information on it anywhere

however, when proxying requests, we should probably leave the connection header to the server itself, as i understand it but we can also find a solution that works for everyone and try to make the whole thing configurable somehow but i wouldn't necessarily delete the line, because it leads to a breaking change due to behavior adjustment

ReneWerner87 avatar Nov 05 '23 10:11 ReneWerner87